Skip to content

Prevent double beep or 'cap' announcement with delayed character descriptions#14271

Merged
seanbudd merged 5 commits into
masterfrom
fixup-14239
Oct 28, 2022
Merged

Prevent double beep or 'cap' announcement with delayed character descriptions#14271
seanbudd merged 5 commits into
masterfrom
fixup-14239

Conversation

@seanbudd

@seanbudd seanbudd commented Oct 19, 2022

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #14239

Summary of the issue:

There are 3 settings for notifying a user of a capital letter. There is "beep", announce 'cap', and/or shift pitch for capitals.

When navigating by character, there is already a beep or "cap" announcement.
When the delayed character description occurs, another beep or "cap" announcement occurs.
There is no need for a secondary beep or "cap" announcement when announcing the the delayed character description.

The cap pitch change may be useful, as a pitch change may be harder to notice, and continuing the shifted pitch is more intuitive.

Description of user facing changes

When beep or announce 'cap' for capitals is enabled with delayed character descriptions, the descriptions do not beep or announce 'cap' twice.

Description of development approach

Disable beeping and announcing 'cap' for delayed character descriptions.

Testing strategy:

Tested the STR in #14239 with combinations of the 4 settings involved (beep, pitch, 'cap' and delayed character descriptions).

Known issues with pull request:

None

Change log entries:

Bug fixes

- When beep or announce 'cap' for capitals is enabled with delayed character descriptions, the descriptions do not beep or announce 'cap' twice. (#14239)

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.

@seanbudd seanbudd requested a review from a team as a code owner October 19, 2022 04:21
@seanbudd seanbudd requested a review from feerrenrut October 19, 2022 04:21

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why make this method configurable?
If it isn't supporting a use case, it's introducing needless complexity.

Comment thread source/speech/speech.py Outdated
@seanbudd seanbudd marked this pull request as draft October 25, 2022 05:14
@seanbudd seanbudd marked this pull request as ready for review October 27, 2022 02:48
@seanbudd

Copy link
Copy Markdown
Member Author

Why make this method configurable?
If it isn't supporting a use case, it's introducing needless complexity.

The way the function is named implies it is more generic than it should be. In a follow up PR I think we should consider moving the 2 character description functions to a new file in speech, and renaming them.

@seanbudd

Copy link
Copy Markdown
Member Author

I have made the approach simpler

@seanbudd seanbudd requested a review from feerrenrut October 27, 2022 02:50
@seanbudd seanbudd merged commit b79a82d into master Oct 28, 2022
@seanbudd seanbudd deleted the fixup-14239 branch October 28, 2022 01:13
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Oct 28, 2022
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.

Using delayed character descriptions, on each uppercase letter, a double beep is heard.

3 participants