MuseScore icon indicating copy to clipboard operation
MuseScore copied to clipboard

Fix #22062: Add preferences for missing engraving UI colors

Open iwoithe opened this issue 10 months ago • 3 comments

Resolves: #22062 Resolves: #13678

  • [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)

iwoithe avatar Apr 12 '24 13:04 iwoithe

@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:

  • Voice 1 color
  • Voice 2 color
  • Voice 3 color
  • Voice 4 color
  • All voices color
  • Anchor color
  • Desynchronized color
  • Formatting color

image

avvvvve avatar Apr 22 '24 19:04 avvvvve

(For reference) The current ordering:

image

@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.

iwoithe avatar Apr 23 '24 04:04 iwoithe

@iwoithe @Eism Sorry for the delay here! Thanks for explaining how the ordering currently works. Since we currently only have a handful of options, that system worked fine, but as this list is growing we're going to need more control over the order. This is my first time seeing those new XML preferences too, so we have to take those into account.

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:

  • Palette scale
  • Smooth panning
  • All voices color*
  • Voice 1 color
  • Voice 2 color
  • Voice 3 color
  • Voice 4 color
  • Anchor color
  • Desynchronized color
  • Formatting color
  • Export invisible elements to MusicXML
  • Limit MusicXML export for compatibility with MuseScore 3

*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 :)

avvvvve avatar May 02 '24 21:05 avvvvve

@iwoithe @Eism can I please request a status update on this one?

bkunda avatar Jul 01 '24 14:07 bkunda

@bkunda

  1. ~I need to resolve the merge conflicts (I'll do that tonight)~ Edit: Merge conflicts now resolved
  2. Still not sure how to resolve @avvvvve's comment code wise, but I'll potentially take a look into it as well tonight (although I wonder whether that should be done in a separate PR? I'm happy to work on it in any case 🙂)

iwoithe avatar Jul 02 '24 00:07 iwoithe

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. (It just looks at all settings keys registered by different parts of the app, and filters those that have a tag that indicates that they should appear in Advanced preferences, and sorts them alphabetically without knowledge of what these settings are semantically related to. That makes it impossible to apply any specific semantic ordering. (To fix that, we'd need to create a central manually sorted list of options that should appear in advanced preferences, but that has the disadvantage that the keys of these settings will somehow need to be exposed by the corresponding modules to prevent typing them literally (with the risk of mistyping them), and that gets messy quite soon.))

@avvvvve Is that okay?

cbjeukendrup avatar Jul 02 '24 21:07 cbjeukendrup

@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 😵‍💫)

avvvvve avatar Jul 03 '24 15:07 avvvvve

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.

cbjeukendrup avatar Jul 03 '24 15:07 cbjeukendrup

Cool! New issue is here: https://github.com/musescore/MuseScore/issues/23455

avvvvve avatar Jul 03 '24 15:07 avvvvve

FYI @cbjeukendrup I've resolved your review comment

iwoithe avatar Jul 11 '24 01:07 iwoithe

@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 avatar Jul 19 '24 02:07 cbjeukendrup

@cbjeukendrup reviewed and approved!

Found a very minor UI bug in the process: https://github.com/musescore/MuseScore/issues/23754

avvvvve avatar Jul 24 '24 15:07 avvvvve