Adding commands to jump to first/last row/column in tables#13435
Conversation
|
cc: @feerrenrut, @seanbudd |
See test results for failed build of commit 31d2e99ecf |
lukaszgo1
left a comment
There was a problem hiding this comment.
Please add the newly introduced gestures to the user guide.
| """ | ||
| result = origRow if axis == "row" else origCol | ||
| row, col = origRow, origCol | ||
| maxTableSize = 1000 |
There was a problem hiding this comment.
A comment here explaining how you come with the 1000 as a right value would be nice.
There was a problem hiding this comment.
This is pretty much an arbitrary large number. I added a pretty obvious comment. There is no science behind it.
|
|
||
| def script_firstRow(self, gesture): | ||
| self._tableMovementScriptHelper(axis="row", movement="first") | ||
| # Translators: the description for the first table row script on browseMode documents. |
There was a problem hiding this comment.
For new scripts script decorator should be used to set script's metadata. Obviously for documentBase this cannot be done at present as it would reintroduce #12399, but perhaps an issue should be created to at least avoid this problem being forgotten.
| def script_firstRow(self, gesture): | ||
| self._tableMovementScriptHelper(axis="row", movement="first") | ||
| # Translators: the description for the first table row script on browseMode documents. | ||
| script_firstRow.__doc__ = _("moves to the first table row") |
There was a problem hiding this comment.
I don't have the background on why this would be impossible, so I don't feel I can create a meaningful issue.
|
@lukaszgo1, Addressed your comments. |
|
The build is failing as linting needs to be fixed. |
|
@mltony can the commands be reassigned by the user? Me personally for example I would prefer to use ctrl+alt+nvda+arrow keys to jump to last column or row. The thing is that my laptop keyboard does not have home, end, page up and page down keys, so I had to replace some f keys with sharp keys to acomplish this function. But is quite unconfortable to use them very often. |
CyrilleB79
left a comment
There was a problem hiding this comment.
Hi Tony
Really happy to see this PR arriving in core!
Some comments here:
- You should test (and implement if needed) the same type of commands in list views where table navigation commands are also available.
If this is not possible for any reason, you should document it and explain why in this PR's initial description.
Note: IMO, if this is not possible for list views, it would already be a great improvement to merge this PR only for real tables.. But it must be clearly documented. - Testing: For Word, did you test with and without UIA enabled? Please update the test section in the initial description accordingly.
Thanks!
| - Added commands for toggling multiple modifiers simultaneously with a Braille display (#13152) | ||
| - The Speech Dictionary dialog now features a "Remove all" button to help clear a whole dictionary. (#11802) | ||
| - Added support for Windows 11 Calculator. (#13212) | ||
| - Added new commands to jump to first/last row/column in tables. |
There was a problem hiding this comment.
Please remove the change on this file. To avoid dealing with many merge conflicts on the change log, NVAccess prefers to update this file themselves. Once your PR is accepted, they will look at the change log section in the initial description of the PR and they will copy/paste its content to this file.
Qchristensen
left a comment
There was a problem hiding this comment.
User Guide change is good, thanks - good work!
michaelDCurran
left a comment
There was a problem hiding this comment.
It is possible to write code to get the dimensions of the table without having to resort to hit testing to find the limits.
From any given textInfo position, you can expand to character, getTextWithFields, walk up the ancestor controlFields until reaching the inner most table controlField, and then fetch table-rowcount and table-columncount attributes.
Refer to _getTableCellCoords for a similar example of where attributes are fetched off a table controlField.
See test results for failed build of commit deaa2da59b |
|
@seanbudd, fixed lint and build. |
|
Also PR #13470 provides same keystrokes in list views and tree views. |
|
@seanbudd, resolved conflicts! |
See test results for failed build of commit 7ef2fa439b |
seanbudd
left a comment
There was a problem hiding this comment.
LGTM, just some changes on documentation / types
|
@seanbudd, fixed and rebased. |
See test results for failed build of commit 1a80e13492 |
seanbudd
left a comment
There was a problem hiding this comment.
thanks @mltony.
please avoid rebases in future for minor changes if possible. After a rebase, GitHub no longer allows you to review pushed changes since your last review. This means the entire PR must be re-reviewed, instead of the changes.
… fake table navigation in list views. (#13788) Follow-up of #13435. Summary of the issue: In fake table navigation in list views, horizontal movements only move the navigator object, whereas vertical movements move the navigator object but also the focus. This was not reflected in input help for the newly introduced commands to jump to first/last row. Description of how this pull request fixes the issue: Updated the two scripts documentation. While at it, I have also converted remaining fake table navigation scripts to syntax with decorator.
Link to issue number:
fixes #957
Summary of the issue:
Introduce commands to jump to first/last row/column in current table.
Description of how this pull request fixes the issue:
Implements new commands in class DocumentWithTableNavigation.
Testing strategy:
Known issues with pull request:
Will cause merge conflicts with #13345 - will be happy to resolve them once it lands.
Change log entries:
New features
Changes
Bug fixes
For Developers
Code Review Checklist: