-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix #22062: Add preferences for missing engraving UI colors #22361
Fix #22062: Add preferences for missing engraving UI colors #22361
Conversation
d6bab73
to
9b47e36
Compare
@iwoithe Thanks for jumping on this! One oversight on my part from the last design I posted on the original issue—the ordering of the items just needs to be slightly tweaked. It should be as follows:
|
9b47e36
to
308c709
Compare
(For reference) The current ordering: @Eism Please correct me if I'm wrong on any of this @avvvvve From my understanding, the order of the items in the advanced settings is automatically sorted by alphabetical order, first by module name and then the setting name. What this results in is obviously the voice colours are pushed after all the colours added in this PR. Also, the anchor colour is never actually used in the engraving module, hence why I placed it inside of the notation module, and because of the alphabetical module sorting, why it appears at the bottom and not with the other colours. |
308c709
to
1b97678
Compare
@iwoithe @Eism We should prioritize organizing these options in a way that's most sensible for a user scanning them over adhering to the current technical implementation. Plus, it just looks like a bug when the colors aren't grouped together. Would it be possible to specify a manual order instead? Also, could someone provide a list of the modules and which preferences are contained in each? Maybe we can keep the structure of ordering Modules first and then ordering the individual preferences in each of them (though if the Anchor color is in a different module than the rest of the colors, this might not work). Here's how I'd recommend ordering that full list:
*Wanted to point out that the feature the "All voices color" supports is in development in #22662 in case there is duplicate code to consider :) |
|
1b97678
to
b33bd94
Compare
Perhaps changing the list ordering should indeed be done in a next PR. It would require some thought, because specifying the order of the items is not really possible, given the way that advanced preferences are currently implemented. @avvvvve Is that okay? |
@cbjeukendrup That's fine! Let me log a separate issue now for that. Do you think there is time to implement a temporary solve for this for 4.4 that we can revisit at some point later? (sorry, you lost me at keys and modules 😵💫) |
I think a temporary solution is doable, but technically it will be somewhat ugly and perhaps not very sustainable. But we can certainly do that on the 4.4 branch when that is created and do something nicer on the master branch. |
Cool! New issue is here: #23455 |
b33bd94
to
29b4d46
Compare
FYI @cbjeukendrup I've resolved your review comment |
@avvvvve Perhaps it's a good idea if you test it once more, and then we can hopefully merge it just in time for 4.4! |
@cbjeukendrup reviewed and approved! Found a very minor UI bug in the process: #23754 |
Resolves: #22062
Resolves: #13678