Skip to content

If command for report the Word comment pressed twice, presents the information in browse mode#15987

Closed
tseykovets wants to merge 4 commits into
nvaccess:masterfrom
tseykovets:word_comment_in_browse_mode
Closed

If command for report the Word comment pressed twice, presents the information in browse mode#15987
tseykovets wants to merge 4 commits into
nvaccess:masterfrom
tseykovets:word_comment_in_browse_mode

Conversation

@tseykovets

@tseykovets tseykovets commented Dec 30, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Link to issue number:

This is a minor change that improves one command for Microsoft Word.

Summary of the issue:

The text of comments often contains important voluminous information that is difficult to perceive in the mode of pronouncing a full phrase.

This pull request adds the ability to present the text of a comment in browse mode by pressing command twice (NVDA+Alt+C).

Description of user facing changes

The previously existing command for Microsoft Word (NVDA+Alt+C) gained additional functionality when it is pressed twice.

Description of development approach

  • A handler for the number of presses has been added to the script (if repeats).
  • If pressed twice, presents the information in browse mode (ui.browseableMessage).

Testing strategy:

To test, just press the NVDA+Alt+C command twice on the comment.

The changes do not contain new code to interact with the operating system and Word application.

Known issues with pull request:

No issues known.
Since the change does not contain new code to interact with the operating system and Word application, it is not expected to affect any other functionality of NVDA.

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.

@tseykovets tseykovets requested review from a team as code owners December 30, 2023 16:15
@Adriani90

Adriani90 commented Dec 30, 2023

Copy link
Copy Markdown
Collaborator

The comment editing area in MS Word is not fully accessible, I second this feature. Thanks very much for that. I think a good approach as well would be to think about a nvda specific dialog like in Excel when pressing shift+f2. This would also allow for a more accessible way of editing comments, not only reading them. However, I also understand this is primarily the responsibility of Microsoft to get their part of accessibility done properly.

@AppVeyorBot

AppVeyorBot commented Dec 30, 2023

Copy link
Copy Markdown

See test results for failed build of commit fcfa4ff909

@cary-rowen

cary-rowen commented Dec 30, 2023

Copy link
Copy Markdown
Contributor

Thank you very much, this is a feature I'm thinking of implementing, good job, please continue.
Please note, that there is a feature request #14628.
You can update the description of this PR.

@cary-rowen cary-rowen mentioned this pull request Dec 30, 2023
Closed
5 tasks
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jan 1, 2024
seanbudd
seanbudd approved these changes Jan 2, 2024
View reviewed changes
Comment thread user_docs/en/changes.t2t Outdated
Comment thread user_docs/en/userGuide.t2t Outdated
@seanbudd

seanbudd commented Jan 2, 2024

Copy link
Copy Markdown
Member

Thanks @tseykovets

@AppVeyorBot

AppVeyorBot commented Jan 2, 2024

Copy link
Copy Markdown

See test results for failed build of commit b36f92f09d

@seanbudd

seanbudd commented Jan 2, 2024

Copy link
Copy Markdown
Member

Please note the linting error, repeat is not definied

@seanbudd seanbudd marked this pull request as draft January 2, 2024 02:05
@seanbudd

seanbudd commented Jan 2, 2024

Copy link
Copy Markdown
Member

Have you tested this? Is it ready for re-review? If so, mark the PR as "ready for review" so it is no longer in draft state.

@tseykovets tseykovets marked this pull request as ready for review January 8, 2024 19:16
@seanbudd

seanbudd commented Jan 8, 2024

Copy link
Copy Markdown
Member

have you tested this? please do not mark as ready for review if you have not tested this.

@seanbudd seanbudd marked this pull request as draft January 8, 2024 23:42
Qchristensen
Qchristensen approved these changes Jan 15, 2024
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.

As with #15986, this will be a good feature for some users. The user guide reads well, and again I would make the changes.txt wording similar to my suggestion on the other PR:

  • Word: When "report any comments at the current caret position" (NVDA+alt+c) is pressed twice, the information is shown in a window. (#15987, @tseykovets)

@Adriani90 Adriani90 mentioned this pull request Feb 25, 2024
Closed
@Adriani90

Adriani90 commented Mar 23, 2024

Copy link
Copy Markdown
Collaborator

@tseykovets are you still working on this?

@Adriani90 Adriani90 mentioned this pull request Mar 23, 2024
Closed
@tseykovets

tseykovets commented Mar 30, 2024

Copy link
Copy Markdown
Contributor Author

@Adriani90, it seems to me that I have implemented everything that is needed.
I would appreciate testing and information if something is wrong.

@Adriani90

Adriani90 commented Apr 11, 2024

Copy link
Copy Markdown
Collaborator

@tseykovets you need to resolve the conflicts with the current master otherwise we cannot download any build to test.
But is it very difficult for you to test this code yourself first?

@Adriani90

Adriani90 commented May 14, 2024

Copy link
Copy Markdown
Collaborator

@tseykovets are you still working on this? Or should we mark this PR as abandoned?

@cary-rowen

cary-rowen commented May 15, 2024

Copy link
Copy Markdown
Contributor

If you are unable to continue with this I would be happy to take over

@tseykovets

tseykovets commented May 17, 2024

Copy link
Copy Markdown
Contributor Author

At the end of 2023, I considered the implementation from this pull request to be complete and debugged.
However, I don't know how true this is for the current state of the master branch.
At the moment, I do not have the opportunity to quickly test the implementation, since I do not have a computer at hand with a configured environment for the local build of NVDA.
If someone undertakes to check the relevance of the implementation from this pull request, then I will be grateful for such help.

@seanbudd

seanbudd commented May 20, 2024

Copy link
Copy Markdown
Member

You can find the build created by the pull request in the PR check for appveyor.
If you find the status check (near the close button, or last commit) it will include a link to the appveyor build.
You can use the build artifact for the installer to test.
I'd encourage you in future to please consider creating a test environment before contributing.
We cannot accept code changes without testing from the author.

@gerald-hartig

gerald-hartig commented May 23, 2024

Copy link
Copy Markdown
Contributor

As @seanbudd mentioned, PRs can only move from draft to ready status once testing has been successfully completed. Thorough testing is crucial to maintain the integrity and functionality of NVDA and is part of our Contribution Process (https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/contributing.md#overview-of-contribution-process). We strongly advise authors to test their own code but if the author is unable to do the testing, collaboration with someone in the community to carry out that testing is then essential.

If for some reason before you start you know that you cannot carry out the testing, we advise you find someone who can do the testing before you write your first line of code. You can use the issue ticket for this purpose, or reach out to the development community in one of the user/developer groups.

For this PR, it looks like @cary-rowen may be able to assist with the testing. Otherwise we may need to consider closing this PR until testing can be completed.

@seanbudd

seanbudd commented Jun 17, 2024

Copy link
Copy Markdown
Member

Closing as abandoned

@seanbudd seanbudd closed this Jun 17, 2024
@seanbudd seanbudd added the Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. label Jun 17, 2024
@cary-rowen cary-rowen mentioned this pull request Jul 3, 2024
Merged
5 tasks
seanbudd pushed a commit that referenced this pull request Jul 12, 2024
@cary-rowen
Enhance Microsoft Word Comment Command: Press Twice to Present Commen…
41418d7 
…t Content in Browsable message. (#16800)

Partially fixed #14628
Replaces abandoned #15987

Thanks to @tseykovets for most of the work.

Summary of the issue:
Sometimes the text of comment is too long, in order to understand, we may need to navigate by sentence or by word.

Description of user facing changes
This pull request adds the ability to present the text of a comment in browse mode by pressing command twice (NVDA+Alt+C).

Description of development approach
A handler for the number of presses has been added to the script (if repeats).
If pressed twice, presents the information in browse mode (ui.browseableMessage).
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

@gerald-hartig gerald-hartig Awaiting requested review from gerald-hartig gerald-hartig is a code owner automatically assigned from nvaccess/developers

Assignees

No one assigned

Labels

Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@tseykovets @Adriani90 @AppVeyorBot @cary-rowen @seanbudd @gerald-hartig @Qchristensen