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

Add setting to disable standard hid braille #13180

Merged
merged 3 commits into from
Dec 20, 2021
Merged

Conversation

feerrenrut
Copy link
Contributor

Link to issue number:

None

Summary of the issue:

Due to the late nature of changes to HID braille support, as a precaution add a work around in case users encounter issues.

Description of how this pull request fixes the issue:

Allow users to prevent HID braille from being used.

Testing strategy:

  • Locally modified the config via the settings panel.
  • Testing via RC

Known issues with pull request:

None

Change log entries:

Changes
- The new HID Braille protocol can be disabled via a setting in the advanced settings panel.

Code Review Checklist:

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

@feerrenrut feerrenrut requested a review from a team as a code owner December 20, 2021 07:39
@feerrenrut feerrenrut requested review from michaelDCurran, LeonarddeR and a team and removed request for a team December 20, 2021 07:39
@LeonarddeR
Copy link
Collaborator

The documentation needs to be updated as well, since this is a new advanced setting.

@CyrilleB79
Copy link
Collaborator

As @LeonarddeR writes, I think that the documentation needs to be updated; also do not forget context help. Even if documentation is not translated or if context help does not work for all languages, I think that it is better to have the English documentation up-to-date.

Regarding translation what is your plan:

  1. Launch a new translation freeze allowing complete translation of UI and documentation
  2. Do not allow translation before RC and 2021.3.1 release. If documentation and context help is updated in English anyway this leads to:
    • English NVDA fully translated
    • Non-English NVDA with a piece of UI in English, no documentation at all for this option and context help not working for this option.
  3. Do not allow translation before RC and 2021.3.1 but paste manually the English paragraph in all the non-English documentations: This leads to:
    • English NVDA fully translated
    • Non-English NVDA with a piece of UI in English, a piece of documentation in English and context help working.
    • Risk related to a non-conventional way to update the documentation (merge conflicts in SVN, English parts remaining in 2022.1, etc.)

@feerrenrut
Copy link
Contributor Author

Given this is advanced setting, and only likely to be changed in response to a support request or some technical investigation. Also at this stage, I think it is an unlikely problem, I've not had any confirmed reports of ongoing issues. I'd prefer not to encounter the issues highlighted by @CyrilleB79 for updating the documentation. On balance I think it is better to address the user guide changes in 2022.1.

@CyrilleB79
Copy link
Collaborator

Just to be extra-clear regarding my position (and given @feerrenrut's last comment)

I really think that having an undocumented part of the UI is not a good practice. Indeed, in case of any braille issue, unofficial documentation will naturally appear on the mailing lists consisting by e-mails send by whoever saying "Try to change the HID parameter in the advanced settings panel".

On the contrary I personally prefer having an official documentation, even in English only, which can serve as a reference in case of discussions on the mailing lists. Regarding translations, this documentation will then be naturally translated in 2022.1.

This been said, the final decision is yours, NVAccess.

@feerrenrut
Copy link
Contributor Author

@CyrilleB79 generally I agree with you, in this case there is not clear advice for this setting, we are approaching the limit for how long this point release can delayed. The documentation for this settings panel is, don't touch unless instructed to by support or know what you are doing. In this case it's actually very low risk, and the option is quite self documenting, there aren't many HID braille devices out there.

I'm going to push forward with an RC2

@feerrenrut feerrenrut merged commit 952d28b into rc Dec 20, 2021
@feerrenrut feerrenrut deleted the addFlagForHidBraille branch December 20, 2021 11:40
@nvaccessAuto nvaccessAuto added this to the 2022.1 milestone Dec 20, 2021
@feerrenrut feerrenrut modified the milestones: 2022.1, 2021.3.1 Dec 20, 2021
@CyrilleB79
Copy link
Collaborator

OK no problem. I understand your point.

@LeonarddeR
Copy link
Collaborator

Honestly I'd still rather have the English docs updated as part of 2021.3.1. I don't think there's anything holding us back from that, right?

feerrenrut added a commit that referenced this pull request Dec 22, 2021
* add setting to disable HID Braille
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.

4 participants