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

Improve clef hiding logic and create new style menu page #20911

Merged
merged 2 commits into from
Jan 25, 2024

Conversation

miiizen
Copy link
Contributor

@miiizen miiizen commented Jan 9, 2024

Resolves: #20795

  • Improves clef visibility option - if only one clef is desired, this is drawn on the first visible stave, not just the first system
  • Introduces a new option to hide TAB clefs after their first appearance separate to other clef types - this is ON by default, while for other clefs it is OFF.
    Screenshot 2024-01-09 at 16 48 48
  • Moves these options from 'Page' to a new page called 'Clefs, key and time signatures'
    image
    Screenshot 2024-01-09 at 16 50 00

@miiizen miiizen requested a review from bkunda January 9, 2024 16:53
@oktophonie oktophonie added the vtests This PR produces approved changes to vtest results label Jan 10, 2024
Copy link

@bkunda bkunda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly really terrific!

Just a couple of points:

  1. A tad bit of vertical space would be good above the section headings "Key Signatures" and "Time Signatures" in the new Styles dialog page (See the Bends page for reference)
  2. I encountered an issue where key signatures were being repeated on the second system in the main score, where a system break had been entered (despite the Styles setting being set to "Hide all key signatures after the first system where they appear").

Both of these issues are illustrated in this video

I also noticed a variant of issue no. 2 above when reviewing part scores: See this video

Test score used:
clefs-visibility-test-11Jan23.mscz.zip

Comment on lines 3717 to 3719
<property name="spacing">
<number>0</number>
</property>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. A tad bit of vertical space would be good above the section headings "Key Signatures" and "Time Signatures" in the new Styles dialog page (See the Bends page for reference)

Looks like these lines would need to be removed to achieve that

Comment on lines 244 to 246
<property name="currentIndex">
<number>1</number>
</property>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preferably, drop this change

<tabstop>genKeysig</tabstop>
<tabstop>genCourtesyClef</tabstop>
<tabstop>genCourtesyTimesig</tabstop>
<tabstop>genCourtesyKeysig</tabstop>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure any new buttons have a sensible tab order. This can be done as follows:

  1. Click this tiny button to enter tab order edit mode
    Scherm­afbeelding 2024-01-12 om 00 14 41

  2. On the previous page, look what's the last tab stop number: it's 131
    Scherm­afbeelding 2024-01-12 om 00 17 11

  3. Right-click and choose "Tab Order List"

  4. Drag the new buttons from the bottom of the list to just after item 131

    Schermopname.2024-01-12.om.00.19.53_.mov
  5. Right-click the first item on the page and choose "start from here"

  6. Click the items in the correct order. (misclicked? Use "start from here" again)

    Schermopname.2024-01-12.om.00.25.23_.mov

(and yes, it can be hyper frustrating)

Copy link

@bkunda bkunda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on macOS. All looks good from a usability POV.
Thanks @miiizen!

@cbjeukendrup

This comment was marked as resolved.

@Tantacrul
Copy link
Contributor

Great we're getting this one done. Thanks all!

@RomanPudashkin RomanPudashkin merged commit 2a70fdb into musescore:master Jan 25, 2024
10 of 11 checks passed
@Eism Eism mentioned this pull request Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vtests This PR produces approved changes to vtest results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add options to show/hide clefs only on the first system of music they appear
6 participants