Added descriptive text to more of the settings panels #17160
Conversation
|
@josephsl, if you can, please verify the description of Input Composition. |
There was a problem hiding this comment.
Thanks for making a start on this @XLTechie. There's still a bit of work to go with regard to making the descriptions visible in the GUI, and I've also provided some thoughts on the wording.
source/gui/settingsDialogs.py
Outdated
| "Adjust certain aspects of the Add-on Store, including whether you want to be notified " | ||
| "when your add-ons have updates available.", |
There was a problem hiding this comment.
This feels fairly self-explanatory to me.
source/gui/settingsDialogs.py
Outdated
| @@ -3246,6 +3269,10 @@ def onSave(self): | |||
| class UwpOcrPanel(SettingsPanel): | |||
| # Translators: The title of the Windows OCR panel. | |||
| title = _("Windows OCR") | |||
| panelDescription = _( | |||
| # Translators: This is descriptive text shown at the top of the Windows OCR settings panel. | |||
| "Configure some elements of the Windows optical character recognition feature which NVDA makes available.", | |||
There was a problem hiding this comment.
This wording feels a little clunky, but I'm not sure I can come up with much better. How about:
| "Configure some elements of the Windows optical character recognition feature which NVDA makes available.", | |
| "Configure NVDA's Windows optical character recognition support.", |
There was a problem hiding this comment.
Yeah, most of what I came up with was clunky too. I'd apply this, but GH won't let me for unknown reasons (the diff is up to date). I'll change it manually.
There was a problem hiding this comment.
The panel descriptions are not visible in the UI. Take a look at ObjectPresentationPanel.makeSettings to see how they're being added visually. Though given similar code is increasingly going to be repeated all over the place, I wonder if it's worth refactoring SettingsPanel to visually add panelDescription itself to keep things DRY.
There was a problem hiding this comment.
@SaschaCowley
That would be a heck of a refactor, given that makeSettings is abstract.
I hadn't looked at that code in a while, and forgot how it worked.
It would break a lot of add-ons to change the way this is handled, although I suppose now is the time.
I thought of trying to add it to the sizer using the metaclass, and have it be dependent on whether makeSettings does anything with the intro text, but inspect.getsource() is not going to be able to read makeSettings for one of the gui.SettingsPanel subclasses. At least, as far as I know.
Not sure of any other "gentle" way of doing things.
It's either break API here and refactor the whole thing; or duplicate the code.
Unless you can think of an alternative.
There was a problem hiding this comment.
Yes, you're right... Two thoughts come to mind, though I haven't thought through these at great length:
- Add the panel description in
SettingsPanel._buildGui. Add a class attribute,automaticallyAddPanelDescription(or similar) to override this behaviour if not desired. This could be set toFalseto maintain backwards compatibility, orTrueto make things "just work". This will be needed, at the very least, for the advanced settings panel, which has a warning banner and descriptive text that are added separately, but concatenated to form itspanelDescription. - Add a helper function to
SettingsPanelthat will handle adding the description frompanelDescriptionappropriately. Manually call the helper function whenever we want a visible description. This way, we aren't repeating all of the layout logic, but aren't changing the API in any backwards-incompatible way. Theoretically, I think this would only need to be passed the box sizer helper as it will be able to get everything else from instance attributes.
My inclination is to go the automatic route, as SettingsPanelAccessible automatically adds panelDescription from the screen-reader user's point of view, so why shouldn't it be there visually? I'm not sure how many things this would break though.
Co-authored-by: Sascha Cowley <[email protected]>
This reverts commit e994f06.
There was a problem hiding this comment.
Thanks @XLTechie.
Only some minor suggestions.
Also, I wonder if it wouldn't be useful to add a description in all panels for consistency?
Co-authored-by: Cyrille Bougot <[email protected]>
Co-authored-by: Cyrille Bougot <[email protected]>
seanbudd
added
the
conceptApproved
Link to issue number:
Fixes #13568
Summary of the issue:
Several settings panels were missing friendly introductory text.
While some have names that are so obvious they do not need introductory text (Speech, Braille, Keyboard, Mouse), others do need it (Input Composition, Browse Mode).
Description of user facing changes
The following settings panels now have some introductory text.
Description of development approach
Wrote, or mostly copied from the user guide with minor alterations, descriptions for the above listed settings panels.
Testing strategy:
Built, made sure the modified panels spoke correctly.
Some with vision may want to check that they show up right.
Known issues with pull request:
None, unless you count that i didn't label the more obvious panels.
Code Review Checklist:
@coderabbitai summary