Navigator object updates braille while navigating between FakeTableCell objects by Emil-18 · Pull Request #15825 · nvaccess/nvda

Emil-18 · 2023-11-24T17:26:54Z

Link to issue number:

Summary of the issue:

When navigating between FakeTableCell objects, braille doesn't show the new object, but instead continues to show the previous object. This is because NVDA thinks the two objects are the same (oldObj == newObj returns True)

Description of user facing changes

The user will be able to navigate FakeTableCell objects with braille

Description of development approach

Added an _isEqual method to the FakeTableCell class, that checks if the two objects has the same row number, column number, and parent, if not, it returns False

Testing strategy:

tested with the sys list view 32 control in the SAM.exe application. Braille followed the navigator object as expected

Known issues with pull request:

None so far

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.

…l objects

LeonarddeR

Good catch!

Co-authored-by: Leonard de Ruijter <leonardder@users.noreply.github.com>

CyrilleB79 · 2023-11-24T18:41:06Z

Thanks @Emil-18 for this fix.

Could you update the initial description with the description of the new equality test?

And could you merge the latest master. For now your PR is not able to build on CI system (appVeyor) due to merge conflict.
Once you have merged latest master, your PR will be able to be built by appVeyor. It will allow to run the automated tests on your build. And it will allow to download the installer so that you can test on your side running it as a temporary copy.

Emil-18 · 2023-11-24T19:49:59Z

@CyrilleB79 I have changed the changes.t2t file, but it still says I have merge conflicts. Aren't I supposed to edit it directly?

CyrilleB79 · 2023-11-24T21:26:05Z

@Emil-18, you should merge the latest master branch in your branch.

burmancomp · 2023-11-25T09:02:53Z

Poor man's solution could be:

save your work somewhere
close this pull request
reset your main branch to state before your changes
update it with git pull
make local branch for your pull request
move to that branch, make your changes, commit them and push your branch: git push origin your_branch
open pull request with your branch.
Keep your local main branch up to date, merge it to your branch, and push your branch again if needed (after your changes and after your have resolved conflicts when merged main branch to your branch (at least if appveyor complains)).

AppVeyorBot · 2023-11-25T17:46:38Z

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/v7cdompl2hdl757y/artifacts/output/nvda_snapshot_pr15825-30050,b377afd3.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 0.9,
INSTALL_END 1.0,
BUILD_START 0.0,
BUILD_END 28.1,
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 b377afd3d1

AppVeyorBot · 2023-11-25T20:28:20Z

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/b0f4auc9s0i9x0or/artifacts/output/nvda_snapshot_pr15825-30051,b4ca2fc3.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 0.9,
INSTALL_END 1.0,
BUILD_START 0.0,
BUILD_END 26.7,
TESTSETUP_START 0.0,
TESTSETUP_END 0.3,
TEST_START 0.0,
TEST_END 16.6,
FINISH_END 0.1

See test results for failed build of commit b4ca2fc37a

CyrilleB79 · 2023-11-25T21:33:54Z

@Emil-18, there is a linting issue; for the rest, the tests are OK.

Linting issues can be found following the link "details" of appVeyor status: in the "Artifacts" page, you will find the file "Flake8.txt".
In your case, the linting is failed because of an empty line missing before the function you have added.

Emil-18 · 2023-11-25T22:03:39Z

@CyrilleB79 Ok. Should I update changes.t2t as well?

CyrilleB79 · 2023-11-25T22:54:42Z

Yes, update the change log please.

CyrilleB79 · 2023-11-27T08:29:36Z

@Emil-18, there are merging conflicts again, due to nvaccess/master branch going forward. Could you merge again nvaccess/master into Emil-18/master and push it?

Once you have done this, do not keep your PR in "Draft" state: please switch your PR to "Ready" state so that NV Access know that your PR is ready to review. Thanks.

Also, a tip for your future contributions and Git usage in general:
You'd better create a dedicated branch for each work/contribution rather than working in your master branch. This is a very common good practice and it allows you to work on two or more topics in parallel.
For this PR however, and in order not to restart from scratch, you can continue working with your master branch.

CyrilleB79 · 2023-11-27T16:32:13Z

 - In Word, the landing cell will now be correctly reported when using the native Word commands for table navigation ``alt+home``, ``alt+end``, ``alt+pageUp`` and ``alt+pageDown``. (#15805, @CyrilleB79)
 - NVDA now resumes audio if the configuration of the output device changes or another application releases exclusive control of the device. (#15758, #15775, @jcsteh)
 - Contracted braille input works properly again. (#15773, @aaclause)
+- Braille is now updated when moving the navigator object between table cells in more situations (#15755)


You may want to add your GitHub's name in the change log as a credit to recognize your work and in case people (e.g. devs) have questions on this topic.

Adding your GitHub in the change log is optional. Feel free to accept the suggestion or not:

Suggested change

- Braille is now updated when moving the navigator object between table cells in more situations (#15755)

- Braille is now updated when moving the navigator object between table cells in more situations (#15755, @Emil-18)

Emil-18 added 3 commits November 10, 2023 01:04

Fixed issue nvaccess#15755

af238cc

navigator object updates braille when navigating between FakeTableCel…

7dd9a87

…l objects

navigator object updates braille when navigating between FakeTableCel…

e634dce

…l objects

LeonarddeR reviewed Nov 24, 2023

View reviewed changes

Comment thread source/NVDAObjects/behaviors.py Outdated

Update source/NVDAObjects/behaviors.py

79b2274

Co-authored-by: Leonard de Ruijter <leonardder@users.noreply.github.com>

Update user_docs/en/changes.t2t

de0ce9b

restored changes.t2t to its original state

a2a3d92

Merge branch 'nvaccess:master' into master

1f175f5

Added empty line before _isEqual function to fix linting issue

28a4e42

updated user_docs/en/changes.t2t

4b48312

CyrilleB79 reviewed Nov 27, 2023

View reviewed changes

Emil-18 and others added 4 commits November 27, 2023 18:34

restored changes.t2t

3b95938

Merge branch 'nvaccess:master' into master

2950855

updated userdocs/t2t

fa13868

Merge branch 'master' of https://github.com/Emil-18/nvda

df235e4

Emil-18 marked this pull request as ready for review November 27, 2023 17:56

Emil-18 requested a review from a team as a code owner November 27, 2023 17:56

Emil-18 requested a review from michaelDCurran November 27, 2023 17:56

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Nov 27, 2023

seanbudd approved these changes Nov 27, 2023

View reviewed changes

Comment thread source/NVDAObjects/behaviors.py Outdated

fix types

ba5ec1a

seanbudd merged commit ad93594 into nvaccess:master Nov 28, 2023

nvaccessAuto added this to the 2024.1 milestone Nov 28, 2023

	- Braille is now updated when moving the navigator object between table cells in more situations (#15755)
	- Braille is now updated when moving the navigator object between table cells in more situations (#15755, @Emil-18)

Uh oh!

Conversation

Emil-18 commented Nov 24, 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!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CyrilleB79 commented Nov 24, 2023

Uh oh!

Emil-18 commented Nov 24, 2023

Uh oh!

CyrilleB79 commented Nov 24, 2023

Uh oh!

burmancomp commented Nov 25, 2023

Uh oh!

AppVeyorBot commented Nov 25, 2023

Uh oh!

AppVeyorBot commented Nov 25, 2023

Uh oh!

CyrilleB79 commented Nov 25, 2023

Uh oh!

Emil-18 commented Nov 25, 2023

Uh oh!

CyrilleB79 commented Nov 25, 2023

Uh oh!

CyrilleB79 commented Nov 27, 2023

Uh oh!

CyrilleB79 Nov 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Emil-18 commented Nov 24, 2023 •

edited

Loading