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

Removed definition lists from user guide since NVDA does not expose them correctly #15902

Closed
wants to merge 3 commits into from

Conversation

Adriani90
Copy link
Collaborator

Link to issue number:

Fixes #14240

Summary of the issue:

The user guide contains a definition list but as per #3858 NVDA cannot expose correctly definition lists.

Description of user facing changes

No definition list, but a simple paragraph saying this setting is x by default. This is consistent with other settings throughout the user guide.

Description of development approach

None

Testing strategy:

Tested that the paragraphs are shown correctly in the user guide.

Known issues with pull request:

None

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.

@Adriani90 Adriani90 requested a review from a team as a code owner December 11, 2023 00:51
@seanbudd
Copy link
Member

Hi, as per the comment #14240 (comment), please replace the definition lists with tables to ensure the same information is captured in a structured manner.

@Adriani90
Copy link
Collaborator Author

Before doing this, I would like to understand why we should document the same information twice?
The available options for a setting are already documented in normal lists. As I understand the comments by Cyrille and Brian this should suffice. The information should meet the purpose of informing the user already.

@CyrilleB79
Copy link
Collaborator

@Adriani90, could you clarify, quoting them to which comments you are referring to?

Could you give an example with a specific parameter on how the same information would be documented twice? I am not sure to get this.

@Adriani90
Copy link
Collaborator Author

@CyrilleB79 in the issue #14240 the expected behavior is to have a table or a normal list. Also Brian comments in that issue that a normal list is a good solution. The settings contain already bullet points which list the available options in an usual way.

With the definition list we have currently documented the options for a setting twice. One in the definition list itself and a second time in the text below having the options as bullet points which is an usual way of listing.

Why should we document the available options 1. in a separate list and 2. in the text below the list as bullet points?

Or is the expected behavior to restructure the bullet points containing the options under each setting as a table? If yes, then I would do this in a separate PR and this should not prevent this PR from being merged.

@CyrilleB79
Copy link
Collaborator

@Adriani90, as asked before, could you please give an example with the paragraph of a specific setting? It will be clearer. Note that there are differences between the structure and the content of all the paragraphs, so working on the most representative example will help.

Anyway, my understanding of #14240 (comment) is that definition lists should be converted to table and, if needed, that the written paragraph below the table should be reworded. E.g.: if the paragraph only indicates the available options in a list without giving a further explanation, there is no need to provide a second time the same list.

The goal of keeping tables or definition list at the top of a setting paragraph is to have a standard way to document the available and default choices for each parameter. Writing a list in the middle of the text is not standard, and thus may be considered harder to find. Also standardization may allow automatic processing of the documentation if such use is considered in the future. E.g.: Check that documentation is in line with NVDA's code.

@Adriani90
Copy link
Collaborator Author

@CyrilleB79 here are two examples. Note that definition lists have never been added to all settings, so there was no consistency anyway from this point of view.

Example 1 - Paragraph describing a setting with a checkbox which can be checked or not checked:
> ==== Delayed descriptions for characters on cursor movement ====[delayedCharacterDescriptions]
: Default
  Disabled
: Options
  Enabled, Disabled
:
When this setting is checked, NVDA will say the character description when you move by characters.

Problem: enabled or disabled is not the option displayed on the screen, so this does not match with the GUI. The setting is a checkbox which can be checked or not checked. When you read the text below the definition list, you already understand that the setting can be either enabled (checked) or disabled (unchecked).
I added the sentence "the setting is disabled by default" I could also say "the setting is unchecked by default".
So the definition list already listed double the things that have been written in the text below. What is the purpose of this?

Example 2: a paragraph describing a setting with multiple options

> ==== Paragraph Style ====[ParagraphStyle]
: Default
  Handled by application
: Options
  Default (Handled by application), Handled by application, Single line break, Multi line break
:
This combo box allows you to select the paragraph style to be used when navigating by paragraphs with control+upArrow and control+downArrow. The available paragraph styles are:
• Handled by application: NVDA will let the application determine the previous or next paragraph, and NVDA will read the new paragraph when navigating. This style works best when the application supports paragraph navigation natively, and is the default.
• Single line break: NVDA will attempt to determine the previous or next paragraph using a single line break as the paragraph indicator. This style works best when reading documents in an application which does not natively support paragraph navigation, and paragraphs in the document are marked by a single press of the enter key.
• Multi line break: NVDA will attempt to determine the previous or next paragraph using at least one blank line (two presses of the enter key) as the paragraph indicator. This style works best when working with documents which use block paragraphs. Note that this paragraph style cannot be used in Microsoft Word or Microsoft Outlook, unless you are using UIA to access Microsoft Word controls.

As you see, things are double displayed and for an user not really useful. Also in this case I just added a sentence saying that "handled by application" is the default option which is enough to inform the user about this. We don't need an additional definition list.

The goal of keeping tables or definition list at the top of a setting paragraph is to have a standard way to document the available and default choices for each parameter.

The current approach is not implemented consistently over all settings, so if this is a goal we should open a discussion on what exactly the purpose of the standard is and how this should look like. In the end whis would mean a rewrite of a lot of paragraphs in order to avoid double information being displayed.

Writing a list in the middle of the text is not standard, and thus may be considered harder to find.

Actually it is standard. If the list of bullet points complement the context that I describe, then i add them below the description and not above. Let's say I describe a combo box and its values, then the values follow the description of the combo box's purpose. Having them above the descriptive text does not make sense from a logical reading perspective.
And the heading is already an orientation point to find the relevant setting along with the table of contents.

standardization may allow automatic processing of the documentation if such use is considered in the future. E.g.: Check that documentation is in line with NVDA's code.

Agree with you, but we don't have this standardization yet, nor we have a clear purpose we build the standardization on. If there is a clear purpose for it (i.e. building a settings reference overview or something like that), then i could of course try to propose an approach for this. But i would like to do it in a separate PR starting from scratch.
This PR already solves an issue.

@CyrilleB79
Copy link
Collaborator

Thanks for explanations.

Note that before this PR, in the majority of the cases the information about default value is not duplicated; it was the choice in this PR to move the information from the definition list to plain text. Only the possible values were generally duplicated.

To clarify more generally:

  • Confusing description of options in user guide #14240 was more about reformatting the existing information since the container (definition) used to provide them was not well supported by NVDA; it
  • The current PR tries to address a broader issu and implements a proposal : reorganize the information presented in a paragraph and decide how this information should be presented.

The reason why all the paragraphs are not formatted the same way is because this way to do is quite new. The idea is to not to update all the documentation but to use the new pattern when writing new paragraphs, or modifying existing ones.

Since the way to document settings is described in userGuideStandards.md, maybe it would have been interesting to discuss its change before a PR with NV Access. But now that your PR is here, I'll let @seanbudd decide if we can continue discussing here or if it's better to clarify it in a dedicated issue. The opinion of other people of the community would also be interesting; let's wait Sean's feedback before CCing other people though.

Anyway, I understand now your point of view, and I share your concerns to a certain extent. Seeing the available options and then a detailed description is quite common and useful in computer science technical documentation (e.g. descriptions of a Linux command, description of a function with its parameters, etc.). But I acknowledge that it's not so common elsewhere and that it may seem less easy to read for some people.
Moreover, given that the paragraphs describing each option are generally small, I am not sure that it is worth having such a summary at the beginning of the paragraph.

In the existing documentation there is also an ambiguity on "Default" used in the definition list:

  • For simple options (e.g. checkboxes), "default" indicates the value by default in the config, i.e. in NVDA's factory settings.
  • For feature flags, "Default" indicates the recommended value by NV Access.

For an example with "Use WASAPI for audio output":

: Default
  Enabled
: Options
  Default (Enabled), Disabled, Enabled
:

The value indicated in the "Default" field is "Enabled", which is recommanded by NV Access. But the factory config for this parameter is not "Enabled"; it is "Default (Enabled)".

@seanbudd seanbudd marked this pull request as draft December 12, 2023 01:42
@XLTechie
Copy link
Collaborator

XLTechie commented Dec 12, 2023 via email

@Adriani90 Adriani90 marked this pull request as ready for review December 16, 2023 20:52
@seanbudd
Copy link
Member

seanbudd commented Dec 18, 2023

Given the additional feedback we have discussed this further internally and would like to continue to keep structured information like this in the user guide.
Please update this PR to use tables as an alternative to definition lists as suggested previously.

I would also ask that the userGuideStandards.md be expanded to the following format.
This is so the behaviour of options can also be documented in a well structured manner.

|| Options | Default (Enabled), Enabled, Disabled |
|| Default | Enabled |
|| Toggle command | ``nvda+shift+e`` |

|| Option || Behaviour ||
|| Enabled | behaviour of enabled |
|| Disabled | behaviour of enabled |

@seanbudd seanbudd marked this pull request as draft December 18, 2023 06:04
@seanbudd
Copy link
Member

Please also rebase these changes to beta - otherwise there will be significant conflicts when attempting to merge in the future, as we will be converting t2t files to markdown

@CyrilleB79
Copy link
Collaborator

@seanbudd it seems to me that the feedback expressed here regarding the usage of structured content (table or definition list) is rather negative, given such structured content is more suitable for a dev doc than a user guide (see messages above for details).

During internal discussions at NV Access, you have reached a conclusion and decided to present the parameters available for options and the default in a structured table though. Could you share with us the reasons why you have made this decision? And why the concerns about the familiarity of basic users with such structured documentation has not been retained?

Also, while at it, could you clarify what is expected in the default field?

  • Value recommended by NV Access (only applicable for feature flags)
  • or factory default value (applicable for all settings)

Thanks.

@Adriani90
Copy link
Collaborator Author

I don't rebase this since I don't expect it to be merged into Beta. This definitely needs more discussion. Sean I can address your points via a separate PR, we have to make sure that the user guide is readable for the user first. Please also take into account our arguments above and provide more insights into this decisoin also from a technical point of view. What benefits should this bring to the user?

@Adriani90
Copy link
Collaborator Author

cc: @josephsl

@Adriani90
Copy link
Collaborator Author

I am also happy to close this PR and we can re take the discussoin after the documentation is converted to markdown if we don't find a common understanding here for now.

@Adriani90
Copy link
Collaborator Author

@Qchristensen your opinion on this is also very welcome.

@seanbudd
Copy link
Member

Value recommended by NV Access (only applicable for feature flags)
or factory default value (applicable for all settings)

The expectation (as per implementation in current docs) is to use "Enabled" for "Default (Enabled)" feature flags, and "Enabled" where that is the factory default value. The idea is to inform the user what the factory default behaviour is.

@CyrilleB79
Copy link
Collaborator

Value recommended by NV Access (only applicable for feature flags)
or factory default value (applicable for all settings)

The expectation (as per implementation in current docs) is to use "Enabled" for "Default (Enabled)" feature flags, and "Enabled" where that is the factory default value. The idea is to inform the user what the factory default behaviour is.

I find it inconsistesince the factory default for a feature flag is "Default (Enabled)", not only just "Enabled".

@Adriani90
Copy link
Collaborator Author

I think it makes sense to describe the settings values and their default values according to the GUI. However, the default (enabled) value for feature flag is in my view confusing. Why are there not only two values in the combo box saying "enabled" or "disabled"? The same applies for "yes" and "no" and so on. Why is there an additional default value at all duplicating an already existing value in the combo box of settings in the advanced settings pannel?

In my view it is still more readable to have a sentence saying "x is the default setting" rather than making the user guide less readable with kind of structured data that is still not consistent.

We can document this in a structured way i.e. in an annex at the end of the user guide if needed, but there should be a clear purpose for this and the GUI needs to be adjusted first to not disorientate users.
Having said that, I think this PR is still a good interim solution, documenting the default setting in a readable way until we find a common understanding of the consistency between GUI and user guide.

@Adriani90
Copy link
Collaborator Author

In light of #15945 I think you plan to convert the definition lists to tables which is also ok in my opinion. However, having the tables in every paragraph at the top will not make the user guide more readable. If that will be done, I propose to do it at the end of the user guide as annex called "settings reference" or someting like that.
Are you planning to close this in favor of the new PR which will convert definition lists into tables?

@seanbudd
Copy link
Member

I was planning to leave this PR open as an avenue for discussing the documentation standards.

However, if you'd like to close this, I would recommend proposing changes to how we document setting by opening a separate GitHub discussion or issue, before opening a PR to amend userGuideStandards.md and change the documentation.
I think documenting them towards the end of each section rather than the start is a good idea.

As mentioned before, there is inherit value to keeping structured information for readability purposes and easy reference.
This is common across reference materials.
It also enables the creation of a settings reference guide, like key commands.

I think there has been some misunderstanding over the purpose of the user guide - it is primarily a reference material, not a guide. We have the "quick start" guide (in the user guide) and training materials for more goal-oriented guides.

Documentation theory suggests 4 kinds of documentation:

  • learning-oriented tutorials.
  • goal-oriented how-to guides.
  • understanding-oriented discussions.
  • information-oriented reference material.

The user guide contains some tutorials and very limited how-to guides, it is primarily a reference material. You can tell this by what the current content is - it documents the functionality of NVDA, rather than provide task oriented guides. Ideally it would be good to separate these concepts out, which was somewhat the point of the quick start guide.

Why is there an additional default value at all duplicating an already existing value in the combo box of settings in the advanced settings pannel?

Please read our docs on feature flags

Further reading if you're interested.

@Adriani90
Copy link
Collaborator Author

Thank you very much for the nice explanation and the additional information. I understand now the reason behind the default value in feature flags.

I think it is a good compromise to have the tables at the end of a paragraph. Instead of a new discussion, I would open a new PR with all settings in section 12 organized in line with #15950. Would that be acceptable?
Thanks for clarifying the purpose of the user guide, from this point of view I understand the reason for having such structured data in place.

@Adriani90 Adriani90 closed this Dec 22, 2023
@seanbudd
Copy link
Member

@Adriani90 - that would be appreciated if you did that

seanbudd added a commit that referenced this pull request Jun 7, 2024
…6667)

Summary of the issue:
As discussed in #16490 and #15902 (comment) that feature descriptions should come before settings tables for features in the user guide.
This is because a basic description is more common than detailed tables in reference manuals

Description of user facing changes
Fixes up order for feature descriptions in the user guide standards
seanbudd added a commit that referenced this pull request Jun 7, 2024
Summary of the issue:
As discussed in #16490 and #15902 (comment) that feature descriptions should come before settings tables for features in the user guide.
This is because a basic description is more common than detailed tables in reference manuals

Description of user facing changes
Fixes up order for feature descriptions in the user guide standards
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing description of options in user guide
4 participants