surge-synthesizer / surge

Synthesizer plug-in (previously released as Vember Audio Surge)
https://surge-synthesizer.github.io/
GNU General Public License v3.0
3.13k stars 400 forks source link

Patch Change Confirmation Dialog Not Receiving Keyboard Focus for Screen Reader Users #7089

Closed ironcross32 closed 11 months ago

ironcross32 commented 1 year ago

Bug Description: A clear and concise description of what the bug is.

When attempting to change patchesr using keybinds after a patch has been modified, there is no indication that the dialog has appeared. Of course, it has, and it is usable, but it no longer jumps keyboard focus as it once did.

Surge XT Version This information is found on the About screen, which you get to from the bottom right menu.

Surge XT 1.3.nightly.5628124

64-bit

Reproduction Steps: Steps to reproduce the behavior:

  1. Modify an existing patch
  2. Try to change patch categories or patch in category using the keybinds
  3. Note that the keyboard focus does not jump to the dialog asking if it's OK to overwrite

Expected Behavior: A clear and concise description of what you expected to happen.

Keyboard focus should jump to the dialog

Screenshots: If applicable, add screenshots/GIF/videos to help explain your problem.

Computer Information (please complete the following!):

Windows 11

6.80

Additional Information: Add any other context about the problem here.

From Discord:

Oh we changed all those dialogs. I wonder if I’m doing so we didn’t port the keyboard focus behavior of the alert box @EvilDragon

mkruselj commented 1 year ago

Happens with all dialogs now. We have redesigned them but forgot to add a11y to them. Apologies.

baconpaul commented 1 year ago

From Discord

This is the reverse order of how it normally appears, but here's the entire dialog as I see it.

OK  button
Cancel  button
Don't ask me again  check box  not checked  read only
MPE  check box  not checked  Configure
Surge Synth Controls  grouping
Don't ask me again  check box  not checked  read only
The currently loaded patch has unsaved changes.
Loading a new patch will discard any such changes.

Do you want to proceed?  The currently loaded patch has unsaved changes.
Loading a new patch will discard any such changes.

Do you want to proceed?
Confirm Patch Loading
The currently loaded patch has unsaved changes.
Loading a new patch will discard any such changes.

Do you want to proceed?  The currently loaded patch has unsaved changes.
Loading a new patch will discard any such changes.

Do you want to proceed?
OK  button
bscross — Today at 3:37 PM
Ah OK, so it really isn't behaving at all like a modal dialog for accessibility purposes.
baconpaul — Today at 3:41 PM
Well you can if you navigate out with the keyboard
Run it with the focus debugger and you can find your way to the back items
The mouse events are eaten but the keyboard aren’t
I know how to fix that. It’s not fun but possible
We should reopen issue and put this in there
So it is mouse modal but not keyboard modal
Accessibility is hard
We will get it fixed though
Basically the modal overlays need a custom keyboard nav handled

Roughly the fake modal dialogs need a custom tab handler to forbid escape with the keyboard. So re-opening issue. That's a wee bit annoying but not impossible.