GSoC '24 - Dynamics Popup - Part 1

[ketgg]

This is the first part of the main pull request: GSoC '24 - Dynamics Popup, which adds a dynamic popup with the functionality to change dynamics and add hairpins through the popup.

• [x] I signed the CLA
• [x] The title of the PR describes the problem it addresses
• [x] Each commit's message describes its purpose and effects, and references the issue it resolves
• [x] If changes are extensive, there is a sequence of easily reviewable commits
• [x] The code in the PR follows the coding rules
• [x] There are no unnecessary changes
• [x] The code compiles and runs on my machine, preferably after each commit individually
• [x] I created a unit test or vtest to verify the changes I made (if applicable)

[cbjeukendrup]

@avvvvve This is definitively ready for testing! We decided to merge the work in multiple steps, and this is the first one. Opening the popup and clicking options in the popup are implemented in this PR.

[avvvvve]

@re1san @cbjeukendrup Awesome! Could you give me a quick summary of what is left to implement so I know what to test? Looking forward to this one!

[cbjeukendrup]

So this PR is the first step, about opening/closing the popup and the basic interaction of clicking options.

The second PR will implement the 'drag handle to create hairpin' interaction.

A third PR will implement the keyboard shortcuts.

These three PRs will just copy code from #23038, but in separate clean steps to make reviewing easier. This is going to happen within the next few days.

Then all that's left should be the 'hover to preview' functionality. @re1san is planning to open a fourth PR for this, but implementing this functionality properly (in a way that doesn't cause problems with undo, for example) has proven to be difficult, and will need more thought from all of us. So this will probably be finished in the further future.

[avvvvve]

@re1san First off, it's so nice to see this working! Wrote some suggestions for refinements that would make this feel really seamless. Let me know if you have any questions!

Refinements (highest to lowest priority)

1. Users must be able to navigate to/within the popup via keyboard navigation, i.e. with a dynamic selected, "Tab" should move focus to the left arrow, then arrow keys can be used to navigate. Pressing enter/spacebar acts as clicking. See the "Capo" and "Harp pedal diagram" elements for examples of how this is already implemented!

2. Certain keyboard interactions on dynamics in the score no longer work with the popup open. Left/right nudging (with and without shift held) is not possible. Here's how I would expect this to work:

   • Left/right/up/down: When you use an arrow key, the dynamic should move and the popup should be dismissed
   • Shift: When shift is held down, the time grid (pictured below) should appear on the staff and the popup should be dismissed. Then, the user should be able to use left/right to nudge it to time positions. This was being developed for 4.4 concurrently with your project, so no worries if you didn't see this behavior before.

3. 'Esc' is not working correctly. This is in part due to a regression to 4.3.2 not caused by this PR, but even once that is fixed we should include this additional behavior: When a dynamic is selected and the popup is open, one 'esc' should dismiss the popup but leave the dynamic selected, and a second 'esc' should deselect the dynamic.

4. If I click on the dynamic again after selecting it, the popup should be dismissed. Clicking it again should open it again. See the sound flag popup for an example of how this is already implemented. Example of why you'd want this: the popup is dismissed when scrolled (expected behavior), but you have to deselect and reselect the dynamic if you want to see the popup again.

https://github.com/user-attachments/assets/83db0024-a6f2-4eee-ab02-37d4087ebc62

5. When I click a dynamic in the score that isn't on the first page of the popup, it should call up the page it's on. Current behavior (the first page shows up every time): Expected behavior:

6. The order of the "ppppp pppp ppp" and the crescendo/decrescendo pages should be swapped, so when you move left from the first page, you get to the "ppppp ..." page. Left again would take you to cres/dec. (Oversight on my part, updated figma with the correct order)

7. When clicking the left arrow to cycle through pages, the rightmost dynamic in the popup has the darker hover state, but it shouldn't

https://github.com/user-attachments/assets/693cd95e-48e1-4537-8520-38090761b770

These two are optional

(Video for both 8 and 9 below)

8. If a dynamic is selected that is immediately followed by a crescendo/decrescendo that is snapped to the dynamic, using the cresc/dec buttons in the popup currently add additional hairpin objects. Perhaps they should replace the existing one when clicked (or do nothing if a crescendo exists and a crescendo is clicked).

9. The popup shifts around a little bit when different dynamics are selected. It would be nice if the popup remained in the same place while clicking different dynamics. The position could then update the next time the popup is opened.

https://github.com/user-attachments/assets/03c769d7-d06b-4d42-97ec-475fd80fa726

[cbjeukendrup]

@re1san Some accompanying technical advice:

1. (Keyboard navigation) You have already created a NavigationPanel, just like CapoPopup has for example, and it looks like you only need to hook up the buttons to this panel, using their navigation.… properties.
2. (Keyboard interactions while popup is open) What you need for this, is that the popup doesn't grab keyboard focus when it is opened. In fact, this is approximately the same problem as in https://github.com/musescore/MuseScore/pull/18436#issuecomment-2231043699. @Eism you commented on that PR that this should be possible, could you tell us how?
3. (Esc) That almost looks like the opposite of 2, actually. If the popup has no keyboard focus, then it won't receive the Esc key press event. But you can probably get around this by checking if a popup is open and if so closing it in NotationActionController::resetState (called when pressing Esc with the notation view open).
4. –
5. @avvvvve, when the user selects a custom dynamic that is not on any page of the popup, presumably the first page should be shown?
6. –
7. I'm uncertain whether this is something we can do anything about, or that it is a Qt oddity. I've seen such things before namely in the existing parts of the UI.
8. –
9. I think Ben's idea of simply not updating the position until close/reopen will work here. (In https://github.com/musescore/MuseScore/pull/18436#issuecomment-1808384073, there was a similar request of not making the popup move when clicking options, but in that case simply not updating the position will be a bit more awkward.)

[ketgg]

@avvvvve @cbjeukendrup Thanks for the suggestions and advice. Should I incorporate these changes into the current PR?

[cbjeukendrup]

Perhaps that's the best idea, yes. But if a specific item turns out to be difficult to solve, then we can decide together to save that for later, for a different PR.

[avvvvve]

Re: 5

when the user selects a custom dynamic that is not on any page of the popup, presumably the first page should be shown?

That seems sensible to me!

[cbjeukendrup]

(the latest push was just a rebase, but nothing besides that)

[avvvvve]

@cbjeukendrup Re-reviewed. Here's how I'd prioritize the feedback from my Aug 23 comment:

Must fix before 4.5 launch: 1, 2, 3, 6 Optional refinements: 4, 5, 7, 8, 9

Would it be okay if I filed one issue for each of those groups above (rather than individual issues)? ^

---

There are also some new issues after the rebase, I assume since these two PRs were merged that enable typing in dynamics: #25252 and #25396

Somewhat unsurprisingly, this PR (unpredictably) breaks the ability to type in dynamics. I'm having trouble nailing down clear reproduction steps, but try:

• Triple-clicking a dynamic to select its text, type in "f", click outside to confirm the "forte", then double-click in again and you won't be able to type anything.

The dynamic appears to be in edit mode, but acts as if it is just selected (try using arrow keys and see that it nudges the dynamic rather than moving the entry cursor). You are locked from typing anything. Sometimes single-clicking in the edit field fixes it, but not always. I've gotten to the point where I can't type in a dynamic right after using the new Ctrl/Cmd + D shortcut.

https://github.com/user-attachments/assets/ff0bb5b9-78bc-433f-bc29-156670ef85c7

To see the expected results, try editing dynamics via typing in a nightly build.

[cbjeukendrup]

Would it be okay if I filed one issue for each of those groups above (rather than individual issues)?

Yes, that's fine. Also, maybe the content of those issues could just be a link to your comment above and which numbers should be fixed in which order of priority.

About the new problem, I think that boils down to the same keyboard focus problem as number 2, which Elnur is going to work on, so there's a good chance that will be solvable easily enough.

[avvvvve]

@cbjeukendrup Great! I created those issues (linked above) and assigned both to you and the high-priority one also to @Eism. Feel free to reassign as needed!