Home > Scores: add context menu#33301
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a controller API to remove recent files and implements it, exposes removal and two platform actions (reveal in file browser, view online) via model Q_INVOKABLEs, and extends QML: a new ScoreItemMenuButton component, updated score item delegates and list/grid views to surface remove/reveal/view actions, and ScoresPage handlers that call the model methods. The controller rebuilds the recent-files list without the removed path and updates state when changes occur. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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/scorespagemodel.cpp`:
- Around line 61-80: The viewOnline function currently calls the blocking
downloadScoreInfo(scoreId) on the UI thread; change it to perform the network
fetch off the main thread and only call platformInteractive()->openUrl(...) from
the completion handler. Replace the synchronous call in
ScoresPageModel::viewOnline with an asynchronous pattern (either use the
existing async::Promise/ScoresList pattern if you can make downloadScoreInfo
return a Promise, or wrap the call in QtConcurrent::run and handle the result
via a queued/Qt::AutoConnection callback). Ensure you still validate
muse::RetVal<muse::cloud::ScoreInfo> (or the resolved value) and QUrl validity
before calling platformInteractive()->openUrl, and propagate/log errors from the
background task back to the UI thread.
- Around line 61-80: The viewOnline method currently blocks the UI by calling
the synchronous museScoreComService()->downloadScoreInfo(scoreId) (which
internally uses QEventLoop::exec()); change it to perform the network lookup
asynchronously and only call platformInteractive()->openUrl(...) from the
callback/slot when the response arrives: invoke the async download API or expose
a signal/slot or use QtConcurrent::run to fetch muse::cloud::ScoreInfo off the
main thread, connect a lambda or slot to receive the RetVal, validate
scoreInfo.ret and the QUrl (as currently done) inside that callback, log errors
and return immediately from viewOnline without blocking so the QML/UI thread
remains responsive.
🪄 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: 14b3f301-fcbb-4f95-bef4-318e6a77a547
📒 Files selected for processing (16)
src/framework/cloud/qml/Muse/Cloud/CloudScoresView.qmlsrc/framework/stubs/cloud/qml/Muse/Cloud/CloudScoresView.qmlsrc/project/internal/recentfilescontroller.cppsrc/project/internal/recentfilescontroller.hsrc/project/irecentfilescontroller.hsrc/project/qml/MuseScore/Project/ScoresGridView.qmlsrc/project/qml/MuseScore/Project/ScoresListView.qmlsrc/project/qml/MuseScore/Project/ScoresPage.qmlsrc/project/qml/MuseScore/Project/ScoresView.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/RecentScoresView.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/ScoreGridItem.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/ScoreListItem.qmlsrc/project/qml/MuseScore/Project/internal/ScoresPage/recentscoresmodel.cppsrc/project/qml/MuseScore/Project/internal/ScoresPage/recentscoresmodel.hsrc/project/qml/MuseScore/Project/internal/ScoresPage/scorespagemodel.cppsrc/project/qml/MuseScore/Project/internal/ScoresPage/scorespagemodel.h
| void ScoresPageModel::viewOnline(int scoreId) | ||
| { | ||
| if (scoreId <= 0) { | ||
| return; | ||
| } | ||
|
|
||
| muse::RetVal<muse::cloud::ScoreInfo> scoreInfo = museScoreComService()->downloadScoreInfo(scoreId); | ||
| if (!scoreInfo.ret) { | ||
| LOGE() << scoreInfo.ret.toString(); | ||
| return; | ||
| } | ||
|
|
||
| QUrl scoreUrl = QUrl::fromUserInput(scoreInfo.val.url); | ||
| if (!scoreUrl.isValid() || scoreUrl.isEmpty()) { | ||
| LOGE() << "Invalid score URL for cloud score" << scoreId << ":" << scoreInfo.val.url; | ||
| return; | ||
| } | ||
|
|
||
| platformInteractive()->openUrl(scoreUrl); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the declaration and any implementation of downloadScoreInfo
# Interface declaration
rg -n "downloadScoreInfo" --type=h -C 4
# Implementation bodies
rg -n "downloadScoreInfo" --type=cpp -C 6Repository: musescore/MuseScore
Length of output: 15241
🏁 Script executed:
cat -n src/framework/cloud/musescorecom/musescorecomservice.cpp | sed -n '400,450p'Repository: musescore/MuseScore
Length of output: 2072
🏁 Script executed:
rg -n "downloadScoresList" src/framework/cloud/musescorecom/musescorecomservice.cpp -A 20 | head -60Repository: musescore/MuseScore
Length of output: 1172
Dispatch network fetch off the main thread to avoid freezing the UI.
downloadScoreInfo(int scoreId) appears to return a synchronous RetVal<ScoreInfo>, but internally uses QEventLoop::exec() to block the calling thread until the network request completes. When called from QML (line 67), this blocks the main UI thread, freezing the GUI until the response arrives.
The implementation confirms this: downloadScoreInfo(int scoreId) creates a QEventLoop, initiates an async network request via doDownloadScoreInfo(), then calls loop.exec() to block until the callback invokes loop.quit(). Under slow or unavailable network conditions, this produces a visibly frozen UI.
The codebase already uses a better async pattern elsewhere: downloadScoresList() returns async::Promise<ScoresList> without blocking. Adopt a similar Promise-based approach or use QtConcurrent::run to move the fetch off the main thread and open the URL from the result callback.
🤖 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/scorespagemodel.cpp`
around lines 61 - 80, The viewOnline function currently calls the blocking
downloadScoreInfo(scoreId) on the UI thread; change it to perform the network
fetch off the main thread and only call platformInteractive()->openUrl(...) from
the completion handler. Replace the synchronous call in
ScoresPageModel::viewOnline with an asynchronous pattern (either use the
existing async::Promise/ScoresList pattern if you can make downloadScoreInfo
return a Promise, or wrap the call in QtConcurrent::run and handle the result
via a queued/Qt::AutoConnection callback). Ensure you still validate
muse::RetVal<muse::cloud::ScoreInfo> (or the resolved value) and QUrl validity
before calling platformInteractive()->openUrl, and propagate/log errors from the
background task back to the UI thread.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the declaration and implementation of downloadScoreInfo
# Check the interface
rg -n "downloadScoreInfo" --type=h -C 3
# Check any implementation
rg -n "downloadScoreInfo" --type=cpp -C 5Repository: musescore/MuseScore
Length of output: 12780
🏁 Script executed:
sed -n '400,440p' src/framework/cloud/musescorecom/musescorecomservice.cppRepository: musescore/MuseScore
Length of output: 1304
🏁 Script executed:
rg -n "template.*RetVal|class RetVal|struct RetVal" --type=h -B 2 -A 8 | head -80Repository: musescore/MuseScore
Length of output: 1960
viewOnline blocks the UI thread with a synchronous network call wrapped in a nested event loop.
museScoreComService()->downloadScoreInfo(scoreId) returns muse::RetVal<muse::cloud::ScoreInfo> — a synchronous value-return type. The implementation uses QEventLoop::exec() to block until an asynchronous network request (m_networkManager->get()) completes. When invoked from QML, this blocking call runs on the main UI thread, preventing the calling code from returning until the network response arrives. While QEventLoop::exec() does process events, the viewOnline() function itself remains blocked, making the QML layer unresponsive to further interactions until the request finishes.
The fix is to make this operation truly asynchronous — for example, trigger the network request and open the URL when the result arrives via a signal, or use QtConcurrent::run with a result handler.
🤖 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/scorespagemodel.cpp`
around lines 61 - 80, The viewOnline method currently blocks the UI by calling
the synchronous museScoreComService()->downloadScoreInfo(scoreId) (which
internally uses QEventLoop::exec()); change it to perform the network lookup
asynchronously and only call platformInteractive()->openUrl(...) from the
callback/slot when the response arrives: invoke the async download API or expose
a signal/slot or use QtConcurrent::run to fetch muse::cloud::ScoreInfo off the
main thread, connect a lambda or slot to receive the RetVal, validate
scoreInfo.ret and the QUrl (as currently done) inside that callback, log errors
and return immediately from viewOnline without blocking so the QML/UI thread
remains responsive.
| } | ||
| } | ||
|
|
||
| ContextMenuLoader { |
There was a problem hiding this comment.
It would be great if this menu were a separate component that could be reused in both views
| } | ||
| } | ||
|
|
||
| void RecentFilesController::removeRecentFile(const muse::io::path_t& path) |
There was a problem hiding this comment.
Could you add TRACEFUNC?
|
Nice! Are there any plans to make this context menu accessible and more discoverable? For example, how about a three dot icon that displays it when clicked? A couple of nitpicks (can be addressed in a future PR):
|
I've implemented this locally, but am waiting for two muse_framework PRs to be merged first (that's the disadvantage of multiple repos: need to wait for three code reviews to get a single patch done...). |
cbjeukendrup
force-pushed
the
home-scores-context-menu
branch
from
bc3321e to
033f685
Compare
|
@cbjeukendrup please rebase |
cbjeukendrup
force-pushed
the
home-scores-context-menu
branch
from
033f685 to
6f6b374
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@muse`:
- Line 1: The submodule pointer in this change references an unreachable commit
(5a321d04fb06be9172ffc15fc9a68c59ea5fd774); do not merge until muse_framework
PRs `#6` and `#7` are merged into the default branch and then update the submodule
pointer in the "muse" entry to a commit that is reachable from the
default/release branch (i.e., the merged tip of muse_framework), replacing the
dangling commit hash with the stable branch commit and re-run submodule update
to ensure CI/consumers can fetch it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| @@ -1 +1 @@ | |||
| Subproject commit 4ae9f3ede9e3c41be92a8b220c5c9dd5b2312f8e | |||
| Subproject commit 5a321d04fb06be9172ffc15fc9a68c59ea5fd774 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the submodule commit is reachable on the muse_framework repo's default branch
# and inspect its message/parents to ensure it incorporates the dependent PRs.
set -euo pipefail
COMMIT="5a321d04fb06be9172ffc15fc9a68c59ea5fd774"
echo "== Submodule pointer recorded in this PR =="
git ls-tree HEAD muse || true
echo
echo "== Submodule URL configured in .gitmodules =="
grep -A2 'submodule "muse"' .gitmodules || cat .gitmodules || true
echo
echo "== Commit metadata via GitHub API (assumes musescore/muse_framework) =="
gh api repos/musescore/muse_framework/commits/${COMMIT} \
--jq '{sha, message: .commit.message, author: .commit.author, parents: [.parents[].sha]}' || true
echo
echo "== Branches/refs containing this commit =="
gh api "repos/musescore/muse_framework/commits/${COMMIT}/branches-where-head" || true
gh api "repos/musescore/muse_framework/commits/${COMMIT}/pulls" \
--jq '[.[] | {number, state, merged_at, base: .base.ref, head: .head.ref}]' || trueRepository: musescore/MuseScore
Length of output: 1100
Do not merge until muse_framework PRs #6 and #7 are merged into the default branch.
The submodule pointer references commit 5a321d04fb06be9172ffc15fc9a68c59ea5fd774, which exists in the muse_framework repository but is not reachable from any branch or merged PR. This indicates the commit is either on a detached state or an ephemeral PR branch. Consumers and CI systems would pull a dangling commit reference. Ensure muse_framework PRs #6 and #7 are merged to the default/release branch, then update this pointer to commit that is reachable from a stable branch.
🤖 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 `@muse` at line 1, The submodule pointer in this change references an
unreachable commit (5a321d04fb06be9172ffc15fc9a68c59ea5fd774); do not merge
until muse_framework PRs `#6` and `#7` are merged into the default branch and then
update the submodule pointer in the "muse" entry to a commit that is reachable
from the default/release branch (i.e., the merged tip of muse_framework),
replacing the dangling commit hash with the stable branch commit and re-run
submodule update to ensure CI/consumers can fetch it.
|
What do you think about having a separator line between "Reveal in file browser" and "Remove from recent files list" as "Remove" is a destructive operation without ability to undo? It does not delete the actual file but still some separation might be a good idea. Also IMHO the three dot button is very hard to spot in thumbnail view (see screenshot). 
|
* Open a score * Reveal it in the file browser (local scores) / online (cloud scores) * Remove it from recent files (on the recent files page only) Resolves: musescore#11235
cbjeukendrup
force-pushed
the
home-scores-context-menu
branch
from
6f6b374 to
ca48022
Compare
|
I added a separator, good idea. I agree the '...' button in grid mode is not ideal yet, but I don't immediately know a solution. Let's await design input. |
5 participants
Resolves: #11235
Depends on musescore/muse_framework#6
Depends on musescore/muse_framework#7
Replaces: #16507
Schermopname.2026-05-07.om.01.09.54.mov
This is part 2 of 2 in a stack made with GitButler: