Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GSoC '24 - Dynamics Popup - Part 2 #24147

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ketgg
Copy link
Contributor

@ketgg ketgg commented Aug 22, 2024

This is the second part of the main pull request: #23038, which adds drag handles to dynamic to create hairpins and the shift key functionality to change hairpin type while dragging.

PR for part 1 is here: #24125

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

@cbjeukendrup cbjeukendrup added the GSoC PRs created by GSoC participants during coding period label Aug 22, 2024
@ketgg ketgg force-pushed the gsoc_dynamics_popup_part_2 branch from 5b13c6f to b11c56f Compare August 23, 2024 04:09
@avvvvve
Copy link

avvvvve commented Oct 22, 2024

While testing, I noticed a hiccup in how I designed the "press shift to swap hairpin direction" functionality.
There's a conflicting long-existing behavior (at least since 4.0.2) where when you hold shift while dragging the end of a spanner (like a hairpin), you can "lock" the element from resizing while dragging the location of the anchor point.

Screen.Recording.2024-10-22.at.6.57.26.PM.mov

I didn't account for this in the design, thus the shift-to-swap-direction behavior was built as a toggle: press AND release shift to swap direction while dragging.

It strikes me that only allowing swapping the hairpin direction while dragging is pretty limiting. What if I just want to swap the direction without changing its position?

It would be simpler to just have a shortcut for "mirroring" a hairpin when it's selected. We already have the shortcut "Mirror notehead" defined as Shift + X. We could extend this shortcut to apply to hairpins and rename it as such, something like "Mirror element (noteheads; hairpins)". If this would be useful for other element types we could make them work with this shortcut too.

A downside of doing this would mean that if you have a range selection containing noteheads and hairpins, and want to mirror noteheads but not hairpins, you'd have to deselect the hairpins first. Seems like an uncommon scenario though.

@bkunda @oktophonie @cbjeukendrup Let me know what you think of that change!

@cbjeukendrup
Copy link
Contributor

@avvvvve Perhaps we could call this new command "Flip horizontally" and rename the old one to "Flip vertically". That's perhaps a bit more abstract but therefore also a bit more general.

@avvvvve
Copy link

avvvvve commented Nov 20, 2024

@cbjeukendrup Actually we already have both shortcuts, but they do specific things:

  • 'Flip direction' (X) flips the vertical stem direction of a note
  • 'Mirror notehead' (Shift + X) makes the notehead appear on the other side of the stem

You're right that we lose some specificity with "Flip vertically" and "Flip horizontally" but I think it would make more sense to do that than to have several shortcuts for flipping different types of objects.

We'll just have to make sure it's documented properly in the handbook.

@MarcSabatella
Copy link
Contributor

You probably realize this, but just in case: the "flip vertical" command ("X") isn't just for note stems, but also flips text and articulations from above the staff to below or vice versa.

@cbjeukendrup cbjeukendrup force-pushed the gsoc_dynamics_popup_part_2 branch from b11c56f to d4c070e Compare January 9, 2025 15:05
@cbjeukendrup
Copy link
Contributor

Rebased.

@Tantacrul
Copy link
Contributor

This is really coming together! Excellent stuff. What's left to implement before this feature is done? I recall quite a few things we included in the design stage but unsure of the final scope agreed upon for the first release.

For example:

  • Hairpins cleverly know to switch around if going the wrong way (eg: P>F -> P<F)

@Tantacrul
Copy link
Contributor

Oh, another was:

  • When you create a hairpin and release it - where it doesn't attach to a dynamic, the popup appears to let you quickly choose a destination dynamic. That would make this truly brilliant.
image

@ketgg
Copy link
Contributor Author

ketgg commented Jan 9, 2025

@Tantacrul They are implemented in the 3rd part of my pull request: #24152

@Tantacrul
Copy link
Contributor

@Tantacrul They are implemented in the 3rd part of my pull request: #24152

Amazing! Can't wait to try it all out when everything is rebased / built. Thanks so much for this wonderful contribution!

@cbjeukendrup cbjeukendrup force-pushed the gsoc_dynamics_popup_part_2 branch from d4c070e to 55d4420 Compare January 10, 2025 14:53
@cbjeukendrup cbjeukendrup force-pushed the gsoc_dynamics_popup_part_2 branch from 55d4420 to 30d7bc1 Compare January 10, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC PRs created by GSoC participants during coding period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants