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 accessibility #1308

Merged

Conversation

benoit-pierre
Copy link
Member

@benoit-pierre benoit-pierre commented May 2, 2021

Summary of changes

Partially address:

Supersede #1293.

For now, it's only a partial pass on the main windows, and parts of the configuration dialog:

  • Disable tag-key navigation in tables, so focusing a table does not lock global tab-key navigation to it.
  • Remove some container widget, tweak focus rules to avoid extra unnecessary steps when using tab-key navigation (like selecting the dictionaries widget outer frame).
  • In the configuration dialog, automatically set the accessible name / description of each option widget to the option name / description (same as the label / tooltip).
  • Manually set the accessible name / description of some widgets (e.g. the widgets of the machine panel in the main window).

@nvdaes: is this a step in the right direction? Please provide feedback (focusing on those 2 dialogs), and we'll iterate until its satisfactory before doing a full pass on the rest of the application.

Some potential issues I've already identified:

  • The main window output panel is problematic: instead of using a single check box for the output state (the logical and simpler choice), we went with 2 mutually exclusive radio buttons (enabled / disabled), for the sake of appearances. There's no clue when using Orca that you can use the up/down arrow key to switch the output state.
  • Taking the machine serial options widget as an example:
    • we need to link each setting widget to its label, e.g. "Port -> the combo box", so the screen reader automatically read the associated label when focusing the setting widget (using the "Edit Buddies" option in Qt Designer).
    • Avoid unnecessary outer frames and/or using tweak focus settings (proxy / policy) to avoid unnecessary steps during keyboard navigation.
    • Avoid using something like "[timeout combo box] [seconds label]", stick to [label]: [setting].
    • Make liberal use of the accessible name / description settings.

Pull Request Checklist

  • Changes have tests (partial: the dictionaries widget is tested)
  • News fragment added in news.d. See documentation for details

@nvdaes
Copy link
Contributor

nvdaes commented May 2, 2021

@benoit-pierre, this is great!
I¡ve tested the main panel and even have been able to test the dialogs to add new translations. About the radio buttons to enable/disable the output, for me they are easy to use with up and down arrow and this is perfectly read by NVDA screen reader.
Even if this PR was merged like that, it would be an excellent step, much better than the current behavior of Plover gui.
Further improvements may include, if possible:

  • Provide an accessible name for the dictionary table, since when it's focused with tab key NVDA don't read nothing, but, since other elements are read properly, we can know that we are in the table and use arrow keys to check individual dictionaries. Another possibility, not incompatible with the previous one, would be to provide a shortcut to focus the table as a start point.
  • Provide other shortcuts if possible (not strictly needed), for other elements, to focus or activate them.
    • Improve the keymapping table, though now it's much better and I have been able to modify assignments, but I'm not sure how this should be done. I move the cursor, for example, to f1 and the press a letter. Then a combo box is focused and I can use arrows, but this should be documented or something.
      Thanks!

@benoit-pierre
Copy link
Member Author

I¡ve tested the main panel and even have been able to test the dialogs to add new translations. About the radio buttons to enable/disable the output, for me they are easy to use with up and down arrow and this is perfectly read by NVDA screen reader.

OK.

Even if this PR was merged like that, it would be an excellent step, much better than the current behavior of Plover gui.
Further improvements may include, if possible:

* Provide an accessible name for the dictionary table, since when it's focused with tab key NVDA don't read nothing, but, since other elements are read properly, we can know that we are in the table and use arrow keys to check individual dictionaries. Another possibility, not incompatible with the previous one, would be to provide a shortcut to focus the table as a start point.

I already did set an accessible name for the dictionary table, but for some reason, NDVA or Narrator don't read it, unless you move focus inside the table (with the arrow keys). Orca does correctly say the name when focusing the table (after tabbing to it).

* * Improve the keymapping table, though now it's much better and I have been able to modify assignments, but I'm not sure how this should be done. I move the cursor, for example, to f1 and the press a letter. 

I'm not sure I understand your point, what do you mean by "to f1 and the press a letter"?

Then a combo box is focused and I can use arrows, but this should be documented or something.

Isn't that the standard behavior for a combo box?

One thing I've noticed, is that in the Orca settings window, when focusing a combo box after tab-key navigation, the screen reader will read both the label and the combo box current value, but for example in Plover's configuration dialog, after focusing the "Dictionaries display order" combo box, the screen reader will read the associated label and the description, but not the current value. Same issue with NVDA, but Narrator does read the current value too.

Anyway, I've now gone over all widgets, accessibility should be better.

Some things still seem far from optimal. For example, take the "Add translation" dialog: there's no feedback when entering the stroke/translation, and you can't focus the two labels reporting on existing mappings.

@nvdaes
Copy link
Contributor

nvdaes commented May 10, 2021

I'm not sure I understand your point, what do you mean by "to f1 and the press a letter"?

I go to key named f1 and then I press a letter.

Then a combo box is focused and I can use arrows, but this should be documented or something.

In fact, this is the standard behavior of combo boxes, but maybe I'm missing something, since I don't know the right way to use this dialog.

I'll test this pr and may provide more feedback. Also, I may search NVDA issues in its repo in case there's some problem with these widgets.

Thanks

@nvdaes
Copy link
Contributor

nvdaes commented May 10, 2021

for example in Plover's configuration dialog, after focusing the "Dictionaries display order" combo box, the screen reader will read the associated label and the description, but not the current value. Same issue with NVDA, but Narrator does read the current value too.

NVDA 2020.4, on Windows 10, is reading the value for me. this is a copy of feedback in Spanish:

Plover: Configuración diálogo
Orden para visualización de diccionarios: cuadro combinado De arriba abajo Establecer orden para visualización de diccionarios:

  • de arriba abajo: coincidir con el orden de búsqueda; primero el de mayor prioridad
  • de abajo arriba: invertir el orden de búsqueda; primero el de menor prioridad
    Abajo
    De abajo arriba
    De arriba abajo

I note an extra word: "abajo" (down).

In the add translation dialog, I can read the text entered by me for strokes and for translation, but I cannot focus the current mappings.
Hope this helps

@benoit-pierre
Copy link
Member Author

NVDA 2020.4, on Windows 10, is reading the value for me. this is a copy of feedback in Spanish:

You're right, I got it wrong, NVDA works, Orca does not. No idea if it's a configuration issue, or a problem with Qt itself on Linux.

@nvdaes
Copy link
Contributor

nvdaes commented May 11, 2021

OK. About the issue produced when pressing tab when the dictionary table is reached, not read on Windows, I have used NVDA+f1 to read developer info for the table object, and it appears as not focusable. Seems to be presented as a container, in case this helps.
Thanks

@benoit-pierre
Copy link
Member Author

I think the issue with tables (not just the dictionaries table, but with others too) is a Qt bug: https://bugreports.qt.io/browse/QTBUG-91029. It's marked as fixed in 5.15.4, but we're using 5.15.2. Unfortunately, I'm not sure we'll get an update (no more open source updates for the LTS branch, commercial only).

Another issue I've noticed, is that when activating one of the tools, like the lookup dialog, NVDA will announce things twice. It looks like this one is a NVDA bug: nvaccess/nvda#12035.

@nvdaes
Copy link
Contributor

nvdaes commented May 11, 2021

Thanks for this investigation!
Another issue is that the results of the lookup dialog cannot be read with the system cursor and we need to use the review cursor of NVDA, what is not the most comfortable procedure since a readonly edit box is presented, and we expect to move the system cursor to read it.

Anyway your changes make Plover to be more accessible than previously.

Another possibility would be, since Plover and NVDA use Python, to create an NVDA add-on to use WX, invoking Plover methods, in case there is a library to send messages from another application to Plover. Or, in case this is not possible, maybe document how to perform certain tasks?

For example, I use git grep searching in the repo to see if a translation is contained in my system. This is a rudimentary method (make a search in the plugin repo with github grep), and if there is a better method it can be documented.

Cheers

@benoit-pierre
Copy link
Member Author

Another issue is that the results of the lookup dialog cannot be read with the system cursor and we need to use the review cursor of NVDA, what is not the most comfortable procedure since a readonly edit box is presented, and we expect to move the system cursor to read it.

I've rewritten the suggestions widget, which is used both by the suggestions tool and the lookup tool (for displaying the results): it's now a list, so you can navigate the lookup results / suggestions with the arrow key.

I've also tried something; each entry in the list as a custom accessible text with the strokes written differently: each stroke letters are space separated to force the screen reader to spell each stroke (with a verbatim dash for the - character to similarly force spelling). Is that kind of thing useful? Would it make sense to generalize it where applicable? (e.g. in the dictionary editor)

Another possibility would be, since Plover and NVDA use Python, to create an NVDA add-on to use WX, invoking Plover methods, in case there is a library to send messages from another application to Plover.

That would basically mean rewriting the UI to use WX: no thanks. Sorry, but we switched from WX to Qt for a reason.

In the add translation dialog, I can read the text entered by me for strokes and for translation, but I cannot focus the current mappings.

I've changed the dialog to use 2 text edits for the existing mappings, please test.

@benoit-pierre
Copy link
Member Author

What about the paper tape? Would using a list like the new suggestions widget be better?

@nvdaes
Copy link
Contributor

nvdaes commented May 13, 2021

I've also tried something; each entry in the list as a custom accessible text with the strokes written differently: each stroke letters are space separated to force the screen reader to spell each stroke (with a verbatim dash for the - character to similarly force spelling). Is that kind of thing useful? Would it make sense to generalize it where applicable? (e.g. in the dictionary editor)

I think this may not be very useful, since screen reader users can choose if they would like to spell sequences of characters and it takes more space in braille displays, so, depending on the line lenght and the number of cells of the display, maybe required to perform mor scroll pressing the corresponding buttons on the braille device.

Anyway, I can use properly the add translation and lookup dialog. Thanks so much!

Definitely, when applicable, for example in the paper tape dialog, it's better to use list instead of the current readonly edit box.

Thanks

@benoit-pierre
Copy link
Member Author

I've also tried something; each entry in the list as a custom accessible text with the strokes written differently: each stroke letters are space separated to force the screen reader to spell each stroke (with a verbatim dash for the - character to similarly force spelling). Is that kind of thing useful? Would it make sense to generalize it where applicable? (e.g. in the dictionary editor)

I think this may not be very useful, since screen reader users can choose if they would like to spell sequences of characters and it takes more space in braille displays, so, depending on the line length and the number of cells of the display, maybe required to perform more scroll pressing the corresponding buttons on the braille device.

Right, makes sense.

Definitely, when applicable, for example in the paper tape dialog, it's better to use list instead of the current readonly edit box.

OK, what should the accessible text for each entry be: the same string as for the display, or the RTFCRE representation for each stroke? Right now, I've chosen the later.

@nvdaes
Copy link
Contributor

nvdaes commented May 13, 2021

OK, what should the accessible text for each entry be: the same string as for the display, or the RTFCRE representation for each stroke? Right now, I've chosen the later.

Can you explain what's the difference, or provide an example or steps to reproduce?

Thanks

@benoit-pierre
Copy link
Member Author

OK, what should the accessible text for each entry be: the same string as for the display, or the RTFCRE representation for each stroke? Right now, I've chosen the later.

Can you explain what's the difference, or provide an example or steps to reproduce?

The paper tape as 2 display modes, do we want the accessible text to reflect those or not? In raw mode, the accessible text will be the same anyway (the stroke RTFCRE representation), but in "Paper" mode, it might be more useful to still use the RTFCRE representation for the accessible text (which does not mean there's no point in using the "Paper" mode, as it still would be of use when using the save function to ensure the output is formatted a certain way).

@nvdaes
Copy link
Contributor

nvdaes commented May 13, 2021

I understand. Then I think we should use just the RTFCRE representation. I suppose that the display may not have sense for screen readers, or may consume extra spaces. The important thing is that, for example if we need to ask a question on mailing list or in the chat and someone request to provide the contents of the paper tape to give help, we aren't limited and can perform the task.
If the display representation is not needed to receive help and you think it doesn't make sense for screen readers, use RTF/CRE.

@benoit-pierre
Copy link
Member Author

If the display representation is not needed to receive help and you think it doesn't make sense for screen readers, use RTF/CRE.

Agreed.

OK, done, I think this is ready for review.

@nvdaes
Copy link
Contributor

nvdaes commented May 13, 2021

Hello, I cannot focus the paper tape. Also, the dialog to select font is not properly read by NVDA, though it's usable.

Here's developer info about the papertape containing text. I have reached it with object navigation of the screen reader, which is not the ideal way and is intended to reach objects that cannot be read with the system cursor.

name: '#STKPWHRAO*EUFRPBLGTSDZ'
role: ROLE_HEADER
roleText: None
states:
isFocusable: False
hasFocus: False
Python object: <NVDAObjects.UIA.UIA object at 0x068C22B0>
Python class mro: (<class 'NVDAObjects.UIA.UIA'>, <class 'NVDAObjects.window.Window'>, <class 'NVDAObjects.NVDAObject'>, <class 'documentBase.TextContainerObject'>, <class 'baseObject.ScriptableObject'>, <class 'baseObject.AutoPropertyObject'>, <class 'garbageHandler.TrackedObject'>, <class 'object'>)
description: ''
location: RectLTWH(left=572, top=187, width=223, height=22)
value: ''
appModule: <'python' (appName 'python', process ID 15760) at address fc2f50>
appModule.productName: 'Python'
appModule.productVersion: '3.8.0'
TextInfo: <class 'NVDAObjects.NVDAObjectTextInfo'>
windowHandle: 3277586
windowClassName: 'Qt5152QWindowIcon'
windowControlID: 0
windowStyle: -1765015552
extendedWindowStyle: 1280
windowThreadID: 324
windowText: 'Plover: Tira de papel'
displayText: ''
UIAElement: <POINTER(IUIAutomationElement) ptr=0x55ab070 at f54800>
UIA automationID:
UIA frameworkID: Qt
UIA runtimeID: (42, 3277586, 4, -2147483490)
UIA providerDescription: [pid:15760,providerId:0x0 Main(parent link):Unidentified Provider (unmanaged:qwindows.dll)]
UIA className:
UIA patterns available: ValuePattern, LegacyIAccessiblePattern

@benoit-pierre
Copy link
Member Author

benoit-pierre commented May 13, 2021

Hello, I cannot focus the paper tape.

Why not? I have no such issue.

Here's developer info about the papertape containing text. I have reached it with object navigation of the screen reader, which is not the ideal way and is intended to reach objects that cannot be read with the system cursor.

That's the list header, which you can ignore, and is not supposed to be focusable. It's also only shown when the style is set to "paper tape". Previously it was a separate label, but I've switched to use the list header to improve the alignment with the list entries. The list entries themselves are focusable, and navigable using the arrow keys on my side (both with Orca and NVDA).

Also, the dialog to select font is not properly read by NVDA, though it's usable.

It does behave weirdly. Unfortunately, it's also a standard dialog provided by Qt, so I don't know what we can do about it (I really don't want to have to recreate it).

@nvdaes
Copy link
Contributor

nvdaes commented May 13, 2021

Not sure. I'll restart my computer and try again.

@nvdaes
Copy link
Contributor

nvdaes commented May 13, 2021

I will test later or tomorrow. My computer Windows is being updated. When you merge this PR, if there are widgets that cannot improved here, I may try to create an app module to use Plover with NVDA, for example, trying to read the state of dictionaries, checked or not checked, when pressing space over an item without needing to use arrow keys, and for the select font dialog. App modules can be created to improve access to applications with the screen reader, ideally to fix issues that cannot be addressed in the original program. Really your work here is excellent. Thanks.

@benoit-pierre
Copy link
Member Author

For the dictionaries widget, now that I know how to set the accessible text of a list item, we could just use that.

@benoit-pierre
Copy link
Member Author

Updated on top of #1332: the dictionaries widget now uses a list too.

@nvdaes
Copy link
Contributor

nvdaes commented May 23, 2021

Great, hope these accessibility improvements can be available for the next stable release or before. This is amazing!

@benoit-pierre benoit-pierre merged commit 7130177 into openstenoproject:master Oct 9, 2021
@benoit-pierre benoit-pierre deleted the improve_accessibility branch October 9, 2021 22:05
@nvdaes
Copy link
Contributor

nvdaes commented Oct 9, 2021

Great, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants