Move Home > Scores > Cloud scores components into project module#33333
Conversation
Near all other Home > Scores components
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis pull request introduces three new QML components for browsing cloud (online) scores in MuseScore. CloudScoresView acts as the main container, instantiating CloudScoresModel and selecting between grid, list, empty, not-signed-in, and error UI states. CloudScoresGridView and CloudScoresListView implement the respective browsing layouts, each with viewport-based pagination logic that schedules increases to desired-row-count when insufficient scores remain visible. Both views show a loading indicator during the Loading model state. The module CMakeLists.txt is updated to register the new components and add the muse_cloud_qml dependency. Redundant imports are removed from existing components to complete the integration. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/project/qml/MuseScore/Project/internal/ScoresPage/CloudScoresGridView.qml`:
- Around line 36-38: Component.onCompleted currently calls
prv.updateDesiredRowCount() without guarding for early/invalid geometry (e.g.,
cellHeight <= 0 or columns <= 0), causing unstable pagination; update the
Component.onCompleted handler in CloudScoresGridView.qml (and the other similar
blocks around the 54-56 and 67-87 regions) to check that relevant layout metrics
(columns and cellHeight) are positive and finite before calling
prv.updateDesiredRowCount(), and in the callers inside the
resize/geometry-change logic ensure the same guards are applied so
updateDesiredRowCount() only runs when the grid has valid geometry.
In
`@src/project/qml/MuseScore/Project/internal/ScoresPage/CloudScoresListView.qml`:
- Around line 37-39: Guard calls to prv.updateDesiredRowCount() so the
pagination math runs only after the list geometry (rowHeight and height) is
valid: replace the direct calls from Component.onCompleted and the other spots
(around the blocks calling prv.updateDesiredRowCount at lines noted) with a
check that ListView.rowHeight and ListView.height (or an equivalent geometry
metric) are > 0, and if not, attach a one‑time handler (e.g. by listening to
rowHeightChanged or heightChanged on the ListView) to re-run
prv.updateDesiredRowCount() when those values become valid; ensure the handler
disconnects itself after invoking updateDesiredRowCount so the update is only
retried once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66759ffb-cf59-4c07-bbbd-fccf7770de51
📒 Files selected for processing (8)
src/project/qml/MuseScore/Project/CMakeLists.txtsrc/project/qml/MuseScore/Project/ScoresPage.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/CloudScoresGridView.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/CloudScoresListView.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/CloudScoresView.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/ScoresGridView.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/ScoresListView.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/ScoresView.qml
💤 Files with no reviewable changes (3)
- src/project/qml/MuseScore/Project/internal/ScoresPage/ScoresListView.qml
- src/project/qml/MuseScore/Project/ScoresPage.qml
- src/project/qml/MuseScore/Project/internal/ScoresPage/ScoresGridView.qml
| Component.onCompleted: { | ||
| prv.updateDesiredRowCount() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add defensive guards for early grid geometry values.
Pagination currently assumes valid cellHeight/columns. Guarding these values avoids unstable desired-row updates during early layout.
Proposed patch
Component.onCompleted: {
- prv.updateDesiredRowCount()
+ Qt.callLater(prv.updateDesiredRowCount)
}
Connections {
target: root.model
function onStateChanged() {
if (root.model.state === CloudScoresModel.Fine) {
// After the model has loaded more, check if even more is needed
prv.updateDesiredRowCount();
}
}
}
+ Connections {
+ target: root.view
+ function onCellHeightChanged() {
+ if (root.view.cellHeight > 0 && root.view.columns > 0) {
+ prv.updateDesiredRowCount()
+ }
+ }
+ function onColumnsChanged() {
+ if (root.view.cellHeight > 0 && root.view.columns > 0) {
+ prv.updateDesiredRowCount()
+ }
+ }
+ }
+
QtObject {
id: prv
@@
function updateDesiredRowCount() {
+ if (root.view.cellHeight <= 0 || root.view.columns <= 0) {
+ return
+ }
+
if (updateDesiredRowCountScheduled) {
return
}Also applies to: 54-56, 67-87
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/project/qml/MuseScore/Project/internal/ScoresPage/CloudScoresGridView.qml`
around lines 36 - 38, Component.onCompleted currently calls
prv.updateDesiredRowCount() without guarding for early/invalid geometry (e.g.,
cellHeight <= 0 or columns <= 0), causing unstable pagination; update the
Component.onCompleted handler in CloudScoresGridView.qml (and the other similar
blocks around the 54-56 and 67-87 regions) to check that relevant layout metrics
(columns and cellHeight) are positive and finite before calling
prv.updateDesiredRowCount(), and in the callers inside the
resize/geometry-change logic ensure the same guards are applied so
updateDesiredRowCount() only runs when the grid has valid geometry.
| Component.onCompleted: { | ||
| prv.updateDesiredRowCount() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Guard pagination math until list geometry is valid.
updateDesiredRowCount() can be scheduled before rowHeight is ready, which makes the viewport math unstable. Add a geometry guard and re-trigger when row height becomes valid.
Proposed patch
Component.onCompleted: {
- prv.updateDesiredRowCount()
+ Qt.callLater(prv.updateDesiredRowCount)
}
Connections {
target: root.model
function onStateChanged() {
if (root.model.state === CloudScoresModel.Fine) {
// After the model has loaded more, check if even more is needed
prv.updateDesiredRowCount();
}
}
}
+ Connections {
+ target: root.view
+ function onRowHeightChanged() {
+ if (root.view.rowHeight > 0) {
+ prv.updateDesiredRowCount()
+ }
+ }
+ }
+
QtObject {
id: prv
@@
function updateDesiredRowCount() {
+ if (root.view.rowHeight <= 0) {
+ return
+ }
+
if (updateDesiredRowCountScheduled) {
return
}Also applies to: 54-56, 67-87
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/project/qml/MuseScore/Project/internal/ScoresPage/CloudScoresListView.qml`
around lines 37 - 39, Guard calls to prv.updateDesiredRowCount() so the
pagination math runs only after the list geometry (rowHeight and height) is
valid: replace the direct calls from Component.onCompleted and the other spots
(around the blocks calling prv.updateDesiredRowCount at lines noted) with a
check that ListView.rowHeight and ListView.height (or an equivalent geometry
metric) are > 0, and if not, attach a one‑time handler (e.g. by listening to
rowHeightChanged or heightChanged on the ListView) to re-run
prv.updateDesiredRowCount() when those values become valid; ensure the handler
disconnects itself after invoking updateDesiredRowCount so the update is only
retried once.
|
@cbjeukendrup We are learning and gaining experience on how to work in a new way... In this case, the PR in the MF should probably have been a draft. |
|
If we do that, there is no association between the framework change and this PR. I think it's better to first merge the framework PR, then immediately update the submodule in this PR and merge it. |
|
Anyway, to keep this moving forward... it looks like it's okay to merge this PR first, and merge the framework change later, even if that is semantically questionable. So @igorkorsukov would you mind approving and merging this PR? |
|
Thanks :) |
2 participants
Near all other Home > Scores components
This fixes the master branch, which would not run because the framework was unexpectedly already updated with musescore/muse_framework#6, while that should only have happened as part of this commit (extracted from #33301).
This is part 1 of 2 in a stack made with GitButler: