Skip to content

Add missing panel descriptions in the settings dialogs. #17701

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

Conversation

heiko-folkerts-msg-david
Copy link

@heiko-folkerts-msg-david heiko-folkerts-msg-david commented Feb 15, 2025

Link to issue number:

Fixes #13568

Summary of the issue:

More config dialogs in NVDA settings should have descriptive text

Description of user facing changes

Added description for all classes derived from SettingsPanel so that all settings categories now have descriptions when tabbing into them.

Description of development approach

Copying the pattern description found for the object presentation category

Testing strategy:

Tested that the descriptions are spoken when running NVDA from source.

Known issues with pull request:

None. Did my best to write good english but it is not my mothers tongue.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

@seanbudd seanbudd marked this pull request as draft February 16, 2025 22:54
@seanbudd
Copy link
Member

@heiko-folkerts-msg-david - have you seen #17160? There's some text there that might be good to use. Particularly for IME and review cursor

Copy link
Member

@Qchristensen Qchristensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, I added a couple of suggestions as well.

Copy link
Collaborator

@CyrilleB79 CyrilleB79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have put some comments.

But my main feedback would be the following:
IMO, the description is not useful if it only lists the options available in the panel. It is useful if it synthesizes why the options available in one panel are there and not in another one.

  • For the user, it should help understand the logic of the organization of the options in the various panels
  • In addition, for developers, it also helps understand the organization of the options, especially, when they have to add a new option.

panelDescription = _(
# Translators: This is a label appearing on the braille settings panel.
"Configures various settings for braille in and output."
" You can set which braille table to be used and other options.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have you chosen to mention table explicitly and other options implicitly?

@SaschaCowley SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Feb 17, 2025
@heiko-folkerts-msg-david
Copy link
Author

heiko-folkerts-msg-david commented Feb 19, 2025 via email

@seanbudd seanbudd marked this pull request as ready for review February 20, 2025 04:07
seanbudd
seanbudd previously approved these changes Feb 20, 2025
@AppVeyorBot
Copy link

  • PASS: Translation comments check.
  • PASS: License check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • PASS: System tests (tags: installer NVDA).
  • Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 8b90e08c8e

@heiko-folkerts-msg-david
Copy link
Author

heiko-folkerts-msg-david commented Feb 20, 2025 via email

Qchristensen
Qchristensen previously approved these changes Feb 21, 2025
Copy link
Member

@Qchristensen Qchristensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reads well.

@@ -1644,6 +1657,10 @@ class VoiceSettingsPanel(AutoSettingsMixin, SettingsPanel):
# Translators: This is the label for the voice settings panel.
title = _("Voice")
helpId = "SpeechSettings"
panelDescription = _(
# Translators: This is a label appearing on the voice settings panel.
"Configures the voice settings for the selected speech synthesizer.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this information really show up somewhere? I cannot see it nor hear it.

Copy link
Collaborator

@CyrilleB79 CyrilleB79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I am not convinced by this work.

First, I have noted the following issues:

  • some descriptions can be heard but are not visible (visually); please sighted people, double check, I can have missed something...
  • also adding a descriptions at the beginning of each panel makes the navigation more verbose, e.g.:
    • when you press Tab from the category list to enter the panel, you need to hear the full panel description before hearing the first option
    • when you press control+tab to switch panel, you hear the panel description before hearing the first option

I think that before accepting such a work to be begun, we have forgotten to think why it would be needed. After re-reading this PR and testing it, my opinion is that:

  • the descriptions should be short or not present at all
  • they should be present only if they really add an extra information, e.g. adding a description to the Audio panel is not needed because we already know which type of options are in this panel and what they are about...

To summarize, in addition to technical issues, the most important is that we should re-discuss when and why a description is needed or not.

@@ -2494,6 +2530,10 @@ class BrowseModePanel(SettingsPanel):
# Translators: This is the label for the browse mode settings panel.
title = _("Browse Mode")
helpId = "BrowseModeSettings"
panelDescription = _(
# Translators: This is a label appearing on the browse mode settings panel.
"Configure how NVDA behaves when reading complex documents, such as web pages and emails.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Configure how NVDA behaves when reading complex documents, such as web pages and emails.",
"Configure how NVDA behaves when reading complex documents, such as web pages and e-mails.",

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But actually, are e-mails in Thunderbird in browse mode?

@@ -2995,6 +3035,10 @@ class DocumentNavigationPanel(SettingsPanel):
# Translators: This is the label for the document navigation settings panel.
title = _("Document Navigation")
helpId = "DocumentNavigation"
panelDescription = _(
# Translators: This is a label appearing on the document navigation settings panel.
"Configures options impacting how you navigate in a document with the cursor. ",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Configures options impacting how you navigate in a document with the cursor. ",
"Configures options controlling how you navigate in a document with the cursor. ",

@@ -2995,6 +3035,10 @@ class DocumentNavigationPanel(SettingsPanel):
# Translators: This is the label for the document navigation settings panel.
title = _("Document Navigation")
helpId = "DocumentNavigation"
panelDescription = _(
# Translators: This is a label appearing on the document navigation settings panel.
"Configures options impacting how you navigate in a document with the cursor. ",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text does not seem to be visible (visually).

@@ -3028,6 +3072,10 @@ class AudioPanel(SettingsPanel):
# Translators: This is the label for the audio settings panel.
title = _("Audio")
helpId = "AudioSettings"
panelDescription = _(
# Translators: This is a label appearing on the audio settings panel.
"Configures the audio settings like volume and sound splitting.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this description is not needed. It only adds more verbosity for no more useful information.

@@ -3240,6 +3288,10 @@ class AddonStorePanel(SettingsPanel):
# Translators: This is the label for the addon navigation settings panel.
title = _("Add-on Store")
helpId = "AddonStoreSettings"
panelDescription = _(
# Translators: This is a label appearing on the addon store settings panel.
"Configures the Add-on Store, including how to handle updates. ",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This information is not visually visible. Moreover, is it really useful? It only adds verbosity.

@heiko-folkerts-msg-david
Copy link
Author

heiko-folkerts-msg-david commented Feb 24, 2025 via email

@CyrilleB79
Copy link
Collaborator

@heiko-folkerts-msg-david, I understand your frustration and I am sorry for that.

First, please take into account that I am not a member of NV Access, so keep in mind that my opinion may differ from NV Access' one.

Regarding my feedback differing from what was written / agreed in the initial issue: Sometimes, it may happen that issue seem easy to fix, and when you try to fix it, something unexpected pops up and makes it more difficult. It also sometimes happens that implementing a solution allow people to better imagine te consequence of a design choice: personally, I had not realized before the impact of the excess of verbosity caused by long panel descriptions. It's just the life of software development.

Apart from this, IMO, it's not a good idea to add descriptions that are not visually visible, even if the main user base of NVDA do not use the visual channel. I have double checked and only the descriptions you have added are not visible; the pre-existing ones (e.g. Document formatting settings, Vision) are visible. I have not investigated why this difference.
So you have various options:

  1. If you can, fix in this PR the issue preventing the description to be visible.
  2. If you can't, for whatever reason, open a new GitHub issue to report this issue and put this PR in blocked state, waiting for the issue to be fixed. Maybe a sighted help is required for this one and, again, it was not expected.

You also write:

  1. Even after many years of daily usage of NVDA I had no clear knowledge of every category. Ofcourse I know that “audio” may be related to something like volume, but if one doesn’t know about the possibility of sound splitting how will he or she find the option? So IMO the descriptions are needed to help users get started and find options they might need.

Your feedback, as the one from other people, is interesting. Hence my question, which should have been asked before: What do we expect from these panel descriptions? And as a follow-up, other questions:

  • Are we able to provide a synthetic description of the content of each panel?
  • How do we make a balance between useful information provided by the description and verbosity overhead caused by it?
  • Do people have difficulty to find the options in the settings dialog, and if yes, do such description help them?
  • Should the description provide a summary of all the options present in a panel?
  • Or should the description rather focus on a common specificity of these options? E.g. the warning in Advanced settings, or in Object Presentation, "These options apply to focus reporting and NVDA object navigation, but not when reading text content e.g. web content with browse mode."

I'd ask NV Access (@seanbudd, @Qchristensen, @SaschaCowley) to think again to this and to report their opinion about this.

At last, @heiko-folkerts-msg-david, do not feel discouraged with this. It may take more time and work than you had anticipated, but it's normal and it will be for you the opportunity to learn more things.

@seanbudd seanbudd added blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. and removed conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. labels Feb 25, 2025
@heiko-folkerts-msg-david
Copy link
Author

heiko-folkerts-msg-david commented Feb 25, 2025 via email

@seanbudd seanbudd dismissed stale reviews from Qchristensen and themself February 25, 2025 05:16

stale

@seanbudd
Copy link
Member

Hi @heiko-folkerts-msg-david,

Given that only the descriptions you have added are not visible, we would generally expect this PR to add visible descriptions like the existing ones, rather than it be handled in a separate issue.

We'll get back to you and Cyrille soon regarding verbosity and content of these descriptions.

@seanbudd seanbudd added this to the 2025.2 milestone Apr 4, 2025
@seanbudd
Copy link
Member

We are in favour of only adding panel descriptions for settings categories which are not intuitive - e.g. keep Review Cursor, Input Composition, Browse Mode and remove Speech, Mouse, etc

@seanbudd seanbudd marked this pull request as draft April 16, 2025 00:19
@heiko-folkerts-msg-david
Copy link
Author

@seanbudd
Copy link
Member

@heiko-folkerts-msg-david - do you still intend on doing this?

@seanbudd seanbudd removed blocked blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. labels Jun 2, 2025
@seanbudd
Copy link
Member

We will close this as abandoned if there's no response in 2 weeks

@heiko-folkerts-msg-david
Copy link
Author

@seanbudd
Copy link
Member

No problem, thanks for your efforts. I will close this and someone else can consider picking up the work.

@seanbudd seanbudd closed this Jun 11, 2025
@seanbudd seanbudd added the Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. label Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Abandoned requested reports or updates are missing since more than 1 year, author or users are not available.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More config dialogs in NVDA settings should have descriptive text
6 participants