If command for report the note on the current Excel cell pressed twice, presents the information in browse mode by tseykovets · Pull Request #15986 · nvaccess/nvda

tseykovets · 2023-12-30T16:10:47Z

Link to issue number:

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

Summary of the issue:

The text of cell notes 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 note in browse mode by pressing command twice (NVDA+Alt+C).

Description of user facing changes

The previously existing command for Microsoft Excel (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+Alkt+C command twice on the cell with the note.

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

Known issues with pull request:

No issues known.
Since the change does not contain new code to interact with the operating system and Excel 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.

…e, presents the information in browse mode

AppVeyorBot · 2023-12-30T16:59:23Z

PASS: Translation comments check.
PASS: Unit tests.
FAIL: Lint check. See test results for more information.
PASS: System tests (tags: installer NVDA).
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/3ib4g05qtl86wv0n/artifacts/output/nvda_snapshot_pr15986-30487,66df28d8.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 0.9,
INSTALL_END 1.0,
BUILD_START 0.0,
BUILD_END 28.2,
TESTSETUP_START 0.0,
TESTSETUP_END 0.3,
TEST_START 0.0,
TEST_END 16.9,
FINISH_END 0.1

See test results for failed build of commit 66df28d83b

Adriani90 · 2023-12-30T17:03:49Z

I think this is not needed. Nvda has already its own handler for these notes and the nvda specific dialog with the coresponding note can be invoked with shift+f2 when focusing the relevant cell. See the MS Excel app module for more details. In this proposed change is a downgrade to the current behavior and most people will use shift+f2 because there you can even edit the note.

cary-rowen · 2023-12-30T20:42:46Z

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 · 2023-12-30T20:45:51Z

@Adriani90

I think this is not needed. Nvda has already its own handler for these notes and the nvda specific dialog with the coresponding note can be invoked with shift+f2 when focusing the relevant cell. See the MS Excel app module for more details. In this proposed change is a downgrade to the current behavior and most people will use shift+f2 because there you can even edit the note.

For consistency, if #15987 is acceptable, then this PR should also be accepted.

seanbudd · 2024-01-02T02:05:16Z

Please note the linting error, repeat is not definied

seanbudd · 2024-01-02T02:05:38Z

did you test this? i'm not sure how this code would succeed

AppVeyorBot · 2024-01-02T02:52:12Z

PASS: Translation comments check.
PASS: Unit tests.
FAIL: Lint check. See test results for more information.
PASS: System tests (tags: installer NVDA).
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/on4exypdelhoah47/artifacts/output/nvda_snapshot_pr15986-30503,a266137b.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.0,
INSTALL_END 1.0,
BUILD_START 0.0,
BUILD_END 27.5,
TESTSETUP_START 0.0,
TESTSETUP_END 0.4,
TEST_START 0.0,
TEST_END 17.1,
FINISH_END 0.2

See test results for failed build of commit a266137b50

seanbudd · 2024-01-02T22:44:51Z

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.

seanbudd · 2024-01-08T23:41:51Z

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

Qchristensen

User guide reads well. The changes.txt text seems cumbersome to me - I would perhaps say:

Excel: When "report any notes for the currently focused cell" (NVDA+alt+c) is pressed twice, the information is shown in a window. (#15986, @tseykovets)

Adriani90 · 2024-03-23T12:21:52Z

@tseykovets are you still working on this?

tseykovets · 2024-03-30T14:52:29Z

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

hwf1324 · 2024-03-30T16:30:04Z

@tseykovets If everything is ready, switch the status of the PR from Draft to "ready for review". so that NV Access personnel can review your PR.

When you think a contribution is ready, or you would like feedback, open a draft pull request. When you would like a review, mark the PR as "ready for review".

seanbudd · 2024-03-30T19:24:01Z

As requested three times previously, have you tested this? please do not mark as ready for review if you have not tested this.
You are expected to test your work when making a code contribution.

Adriani90 · 2024-04-11T05:48:59Z

@tseykovets you need to solve the conflicts also in this PR, otherwise we cannot download a build with this PR to test.
Is it too difficult for you to test it yourself? Or what is the reason you didn't test it when raising this pull request?

Adriani90 · 2024-05-14T23:08:56Z

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

tseykovets · 2024-05-17T13:32:51Z

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.

XLTechie · 2024-05-17T21:55:10Z

@tseykovets You can always perform your own AppVeyor builds of NVDA, in which case you do not need a local dev environment. https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/buildingNVDAOnAppVeyor.md

gerald-hartig · 2024-05-23T06:45:18Z

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, if no one is available to assist with the testing, we may need to consider closing this PR until testing can be completed.

seanbudd · 2024-06-17T00:07:25Z

Closing as abandoned

@tseykovets

…ontent in browsable message. (#16878) Fixed #14628 Replaces abandoned #15986 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). Additionally, I added a type hint for the gestures.

If command for report the note on the current Excel cell pressed twic…

522b8d3

…e, presents the information in browse mode

tseykovets requested review from a team as code owners December 30, 2023 16:10

tseykovets requested review from Qchristensen and gerald-hartig December 30, 2023 16:10

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jan 1, 2024

seanbudd approved these changes Jan 2, 2024

View reviewed changes

Comment thread source/NVDAObjects/window/excel.py Outdated

Comment thread user_docs/en/changes.t2t Outdated

Comment thread user_docs/en/userGuide.t2t Outdated

Apply suggestions from code review

6d1dad4

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

Fix a bug with handling script repeats

3db8636

tseykovets marked this pull request as ready for review January 8, 2024 19:15

seanbudd marked this pull request as draft January 8, 2024 23:41

Qchristensen approved these changes Jan 15, 2024

View reviewed changes

Qchristensen mentioned this pull request Jan 15, 2024

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

Closed

5 tasks

Reformulate the description of the changes

9232132

Adriani90 mentioned this pull request Mar 23, 2024

Elements list: Copy to clipboard #5155

Closed

seanbudd closed this Jun 17, 2024

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 mentioned this pull request Jul 18, 2024

Enhance Microsoft Excel notes command: Press twice to present notes content in browsable message. #16878

Merged

5 tasks

Uh oh!

Conversation

tseykovets commented Dec 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

AppVeyorBot commented Dec 30, 2023

Uh oh!

Adriani90 commented Dec 30, 2023

Uh oh!

cary-rowen commented Dec 30, 2023

Uh oh!

cary-rowen commented Dec 30, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seanbudd commented Jan 2, 2024

Uh oh!

seanbudd commented Jan 2, 2024

Uh oh!

AppVeyorBot commented Jan 2, 2024

Uh oh!

seanbudd commented Jan 2, 2024

Uh oh!

seanbudd commented Jan 8, 2024

Uh oh!

Qchristensen left a comment

Choose a reason for hiding this comment

Uh oh!

Adriani90 commented Mar 23, 2024

Uh oh!

tseykovets commented Mar 30, 2024

Uh oh!

hwf1324 commented Mar 30, 2024

Uh oh!

seanbudd commented Mar 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Adriani90 commented Apr 11, 2024

Uh oh!

Adriani90 commented May 14, 2024

Uh oh!

tseykovets commented May 17, 2024

Uh oh!

XLTechie commented May 17, 2024 via email

Uh oh!

gerald-hartig commented May 23, 2024

Uh oh!

seanbudd commented Jun 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

tseykovets commented Dec 30, 2023 •

edited

Loading

seanbudd commented Mar 30, 2024 •

edited

Loading