fix #302822: breaks are now accessible via keyboard #5919
Conversation
| @@ -641,6 +641,9 @@ Element* Score::nextElement() | |||
| return mb; | |||
| } | |||
| } | |||
| case ElementType::LAYOUT_BREAK: { | |||
| staffId = 0; // otherwise it will equal -1, which breaks the navigation | |||
There was a problem hiding this comment.
Does this do reasonable things if the top staff happens to be invisible or hidden? To be honest, it's not clear what the expectation is here, and I actually tried it a few different ways when implementing navigation through frames or through repeat text, both of which also don't necessarily have a valid staffid of their own.
There was a problem hiding this comment.
I hadn't thought of that possibility. Unfortunately, it does not do reasonable things if the top staff is hidden (if I was a user I would expect the top visible staff to lead me to the break).
Fortunately, it does allow for a more reasonable workaround (the user can alt+up to the invisible top staff, change the break and go down again). I will continue looking into this issue (see if I can make it behave in a smarter manner).
At the very least, this behavior seems consistent with the current behavior of similar elements :^).
There was a problem hiding this comment.
Yes, then to me, that does count as "reasonable" :-). I tried having other measure elements act the way you describe - navigating as if they were attached to the top visible staff - but it turned out to be more trouble to get right than it was worth, in my opinion.
I'm OK with the break still navigating on the top staff even though it isn't visible, because actually, a blind user currently gets no feedback whatsoever about what is visible and what isn't. So they would not normally know or think to not navigate the top staff just because it doesn't happen to be visible on the current system.
Someday it will make sense to have the screen reader announce when you are on a staff that is not visible.
Anyhow, I built this code and it does seem to work well, but I couldn't easily test the various cases I'd have wanted to because on my system right now the Instruments dialog crashes MuseScore - and my whole system - hard. Not sure what's up with that, but it's not connected to your code of course.
There was a problem hiding this comment.
@MarcSabatella have you found a way to fix your machine? Do you have time to test the changes from this PR?
There was a problem hiding this comment.
I got some things working better, but instruments dialog still crashes for me. I will try on my Windows machine, though.
|
I have now been able to test this in a variety of scenarios involving multiple staves, hidden staves, etc, and all is well. I guess a slight improvement possible would be to override and augment the accessibleInfo() for the breaks to say which kind of break they are - system, page, or section. Maybe similar to how other elements implement a userName() function that is then used in their accessibleInfo(). This would add new strings for translation, I guess, although they are duplicates of strings used elsewhere (eg, in menus.cpp). Anyhow, not as important as getting the navigation itself working, so to me it can wait for another PR. |
Resolves: https://musescore.org/en/node/302822
Added the ability to access breaks via the keyboard.