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

Display in braille the character number upon request #14837

Merged
merged 3 commits into from
Apr 19, 2023

Conversation

CyrilleB79
Copy link
Collaborator

@CyrilleB79 CyrilleB79 commented Apr 17, 2023

Link to issue number:

Fixes #14826

Summary of the issue:

When pressing 3 times numpad2, the character number is reported by speech (both in decimal form and hexadecimal form). However, for braille users it would be useful to have this information.

Description of user facing changes

When pressing three times numpad2 to call the script "Report current character in review", the reported information (character number in decimal and hexadecimal) is also reported in braille, not only by speech.
First press and second press of this script remain unchanged since:

  • First press only report the current character which is already available at the review cursor's position on the braille display, provided that the braille follows automatically (default settings) or review.
  • Second press is a feature dedicated to speech users, when they cannot hear correctly the character name.

Description of development approach

Added a braille update as done in ui.message. We do not use directly ui.message since the hex value has to be spelt.

Testing strategy:

Manual testing:
Pressed once, twice and 3 times numpad2; and checked that:

  • For each case, the speech is still reported the same way as it was before
  • Upon first and second press, nothing happens at braille level
  • At third press, the information reported vocally is also reported in braille

Known issues with pull request:

None

Change log entries:

New features
When pressing three times numpad2 to report the numerical value of the character at the position of the review cursor, the information is now also provided in braille. (#14826)

Note: better wording welcome if needed; cc @XLTechie

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
  • Security precautions taken.

@CyrilleB79 CyrilleB79 marked this pull request as ready for review April 18, 2023 08:22
@CyrilleB79 CyrilleB79 requested a review from a team as a code owner April 18, 2023 08:22
@@ -1861,6 +1861,7 @@ def script_review_currentCharacter(self, gesture: inputCore.InputGesture):
if c is not None:
speech.speakMessage("%d," % c)
speech.speakSpelling(hex(c))
braille.handler.message(f"{c}, {hex(c)}")
Copy link
Member

Choose a reason for hiding this comment

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

should this be a translatable message given the use of comma? I know the previous speech example isn't but perhaps should be as well?

@CyrilleB79
Copy link
Collaborator Author

Although specific punctuation exists in some other languages (e.g. "،" = Arabic comma), I think that the comma is present elsewhere in NVDA's UI without being translated. And I do not know of any user complaining of it.

If you are thinking of any user request or any more specific situation, it would be good to know.

Finally, I have no opinion so I'll conform to what you finally request. Just let me know if your comment is just an open question or if it is a change request.

Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Considering its use elsewhere, this is not an issue

@seanbudd seanbudd merged commit 7bca0e8 into nvaccess:master Apr 19, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Apr 19, 2023
@CyrilleB79 CyrilleB79 deleted the brailleCharNum branch April 24, 2023 11:37
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.

displaying the numeric value of the character (in decimal and hexadecimal) in braille
3 participants