Skip to content

Add spelling for double presses of some NVDA commands#15504

Merged
seanbudd merged 5 commits into
nvaccess:masterfrom
CyrilleB79:spellSelection
Sep 26, 2023
Merged

Add spelling for double presses of some NVDA commands#15504
seanbudd merged 5 commits into
nvaccess:masterfrom
CyrilleB79:spellSelection

Conversation

@CyrilleB79

@CyrilleB79 CyrilleB79 commented Sep 23, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Link to issue number:

Closes #15449

Summary of the issue:

Various NVDA commands which report information spells it on second press. This was not the case for the two following commands:

  • Report current selection
  • Report clipboard text

Moreover, the command to report focus (NVDA+Tab) support spelling the information on second press, but not the triple press version spelling it using character description.

Description of user facing changes

When pressing two times NVDA+C or NVDA+shift+upArrow, the reported information will be spelt. A third press spells the information using character description. As for other similar scripts, if the selection or the clipboard text contains too many characters, the information is not spelt and the number of characters is reported instead.

Similarly, the command to report focus (NVDA+Tab) has also been extended to support spelling the information using character description on third press.

Description of development approach

Same logic as other similar scripts.

Testing strategy:

Manual test:

Tested report selection script with 1, 2 or more presses for:

  • Nothing selected
  • Few characters selected, e.g. one word
  • Many characters selected (more than 2000)

Tested report clipboard script with 1, 2 or more presses and in the following cases:

  • The clipboard does not contain text ; file copied from Windows Explorer
  • The clipboard contains a text with few characters, e.g. one word
  • The clipboard contains a lot of text, e.g. a page of more than 2000 characters.

Tested NVDA+Tab with 1, 2 or 3 presses

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.

@CyrilleB79
Add spelling for double presses of some NVDA commands:
b9455f8 
- Report current selection
- Report clipboard text
@AppVeyorBot

AppVeyorBot commented Sep 23, 2023

Copy link
Copy Markdown

See test results for failed build of commit c5f0c4cb97

@mzanm

mzanm commented Sep 24, 2023

Copy link
Copy Markdown
Contributor

Hello
Thank you for this. Could you please add on 3 presses to spell phonetically like other scripts? Thanks.

@CyrilleB79

CyrilleB79 commented Sep 24, 2023

Copy link
Copy Markdown
Contributor Author

There are various commands reporting something that support double-press to spell the reported information. Among these, the majority of them actually support triple press:

  • either to spell the information in international code (alpha, bravo, Charlie); that's the case for read line, read review word and read review line.
  • or to copy the information to the clipboard. That's the case for read window title, read status bar and report current nav object
    The only exception that does not support triple press is report focused object (NVDA+tab).

Of course, for the scripts of this PR (read current selection and report clipboard text), a triple press to add the information to the clipboard has no sense, either because the clipboard already contains this information or, in the case of selection, because control+C is the native script to copy the selection.

Thus supporting triple press in international code makes sense for the two commands modified in this PR. Should I also add it for report focused object?

@seanbudd, NV Access' opinion about adding the triple press for these 3 scripts?

@seanbudd

seanbudd commented Sep 24, 2023

Copy link
Copy Markdown
Member

Yes, it makes sense to be able to spell phonetically in this case

@CyrilleB79 CyrilleB79 marked this pull request as ready for review September 25, 2023 07:38
@CyrilleB79 CyrilleB79 requested review from a team as code owners September 25, 2023 07:38
@AppVeyorBot

AppVeyorBot commented Sep 25, 2023

Copy link
Copy Markdown

See test results for failed build of commit 2508c058ed

@seanbudd seanbudd added this to the 2024.1 milestone Sep 26, 2023
seanbudd
seanbudd reviewed Sep 26, 2023
View reviewed changes

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @CyrilleB79 generally looks good

Comment thread user_docs/en/changes.t2t Outdated
Qchristensen
Qchristensen approved these changes Sep 26, 2023
View reviewed changes

@Qchristensen Qchristensen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

User guide change reads well.

@seanbudd seanbudd merged commit 26d1fb5 into nvaccess:master Sep 26, 2023
@CyrilleB79 CyrilleB79 deleted the spellSelection branch September 26, 2023 07:16
@CyrilleB79 CyrilleB79 mentioned this pull request Oct 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanbudd seanbudd seanbudd approved these changes

@Qchristensen Qchristensen Qchristensen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2024.1

Development

Successfully merging this pull request may close these issues.

Spell selection when NVDA+shift+upArrow is pressed a second time

5 participants

@CyrilleB79 @AppVeyorBot @mzanm @seanbudd @Qchristensen