-
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
Accessibility for position slider and position label in Play panel #1773
Conversation
62bfc8b
to
464b669
Compare
|
||
PlayPositionSlider::PlayPositionSlider(QWidget *p) : QSlider(p) | ||
{ | ||
setAccessibleDescription(tr("Use Left and Right arrow keys to modify the beat. Use Ctrl + Left and Right arrow keys to modify the measure.")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to move beat by beat ? to move measures by measures?
464b669
to
2d79984
Compare
fix #47276
2d79984
to
3b345e0
Compare
@andreituicu hi. Are you around to rebase the PR onto current master? |
Archiving this, please reopen or create new pull request if there is an update. |
Is this going to be continued, or can I pick it up? |
@lizzyd710 given that it's been over five years, I doubt the original author has any interest in returning to it... |
Do you have experience working on accessibility code? If not, feel free to ask us questions on the Telegram chat. If you do have experience, we might have some questions you could help us with :-) |
I have experience working with Qt Widgets in C++ applications, and I'm very passionate about accessibility and technology. I haven't directly worked with Qt Widget accessibility, but I'm already reading the documentation Qt has for accessible interfaces and classes. I'd be happy to help with any questions if I can! |
Are there any changes to be made besides rebasing? |
I see one yet unadressed open review comment, reg. plural: #1773 (diff) |
I am not really sure but I think the changes to the play panel may have been pretty substantial since this code was written. In theory, sliders should be accessible already, but the units for the playback position slider are not good - it moves in something ridiculous like MIDI ticks (1/480 of a beat). So the code here is to make it respond beat by beat or measure by measure. Nice to have, sure. I'm not 100% there isn't a simpler way to implement that (eg, just changing the step value). But FWIW, the tab order is also not great in this window. |
FWIW, more generally, while there are any number of accessibility improvements yet to be made, the thing that bothers me most is lack of support for VoiceOver on macOS. We have some pretty grandiose plans for how the code can be restructured in a way that will make this happen eventually, but I am convinced there is a simple way to get it to happen just by choosing the right role and accessibility events for our ScoreView class. I have feedback from a coupe of different people with suggestions on how to go about it, but I haven't had luck so far, and I lack a Mac to test on anyhow. But this isn't really the place for general discussions like this, I'll just encourage you again to join the developer chat on Telegram if you haven't already! |
fix #47276 fix #47276
The value of the position label is now read by the screen-reader for the position slider, instead of the current tick as it was before.
Pressing right/left arrow key increments/decrements the beat in the position label.
Pressing Ctrl+right/left arrow key increments/decrements the measure.
When changing the value of the slider either by using the mouse or by using the keys the screen-reader is notified.