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 pronunciation of Mac Option key symbol "⌥" #14682

Merged
merged 3 commits into from Mar 23, 2023

Conversation

dbjorge
Copy link
Contributor

@dbjorge dbjorge commented Feb 28, 2023

Link to issue number:

n/a (minor/trivial change)

Summary of the issue:

NVDA doesn't announce the symbol that macs use to represent the mac "Option" key ("⌥") (see Mac keyboard shortcuts documentation).

Description of user facing changes

Adds a symbol description for this symbol ("mac Option key" announced at "none" level and up, similar to the existing entry for "mac Command key" at the "none" level for symbol "⌘").

Description of development approach

Followed the "Defining Symbol Information" developer documentation, using the existing "mac Command key" entry and similar recent PR #14548 as examples

Testing strategy:

Didn't attempt to adjust any automated tests (assumed they were already present generally for symbol pronunciation). Manually verified reading by:

  1. runnvda.bat from this branch
  2. Browse to Mac keyboard shortcuts docs
  3. Read through the content under the "Mac keyboard shortcuts" H1, particularly the list item with content "Option (or Alt) ⌥"
  4. Verify that the symbol is read out as "mac Option key" as expected
  5. Change NVDA's "Settings > Speech > Punctuation/symbol level" to "none" and repeat above steps

Known issues with pull request:

Mac commonly uses a few other symbols to represent keyboard modifiers (eg, when displaying keyboard shortcut hints), per the Mac keyboard shortcuts documentation. I've left the other cases alone because they either:

  • Already have a comparable entry ("Command ⌘")
  • Are usually represented as an image, not a character symbol ("Fn globe icon")
  • Already have more general symbol names that would be applicable in more contexts ("Shift ⇧", "Caps Lock ⇪", "Control ^")

Change log entries:

New features
- Added pronunciation of Mac Option key symbol "⌥". (#14682)

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.

@dbjorge dbjorge requested a review from a team as a code owner February 28, 2023 22:13
@dbjorge dbjorge marked this pull request as draft February 28, 2023 22:14
@dbjorge
Copy link
Contributor Author

dbjorge commented Mar 16, 2023

Hi @seanbudd ,

Just wanted to give a friendly ping on this and to clarify that this PR is marked as a "Draft" only because that's what the PR template suggests to do - I think it's ready for review. Is there anything further action you'd like me to take to help move this forward?

@XLTechie
Copy link
Contributor

XLTechie commented Mar 16, 2023 via email

@dbjorge dbjorge marked this pull request as ready for review March 16, 2023 19:41
@dbjorge
Copy link
Contributor Author

dbjorge commented Mar 16, 2023

Apologies, I misunderstood the instructions. Thanks for clarifying, @XLTechie!

@seanbudd seanbudd merged commit 4628bb5 into nvaccess:master Mar 23, 2023
1 check was pending
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Mar 23, 2023
@codeofdusk
Copy link
Contributor

Nice to see you contributing here!

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.

None yet

5 participants