Fixing table navigation commands by caching last column/row index#11923
Fixing table navigation commands by caching last column/row index#11923mltony wants to merge 4 commits into
Conversation
See test results for failed build of commit cb4469815e |
See test results for failed build of commit 64d22aa1a9 |
See test results for failed build of commit 7196fee706 |
|
Thanks @mltony for this quick PR! Let's hope it can be merged soon, this would really help me. I have tested it and almost all is working well. Navigation with simple arrow key do not cause column/row change in Excel, but Excel already behaves this way without screen reader. Just the table nav in MS Word still cause column/row change when crossing a merged cell. I do not know how much it is linked to Word object model and if something can be done. |
After looking a bit at the code, it seems that NVDA is responsible of row / column location, not Word object model. @mltony do you plan to fix table navigation also in Word in this PR? |
|
@mltony would you mind to address the MS Word question if you have time? This would help to progress this PR. |
|
@CyrilleB79, I don't have bandwidth to work on MSWord issue - so if you have time feel free to take over. |
|
Thanks for the answer. |
|
@seanbudd Any updates on this PR? |
There was a problem hiding this comment.
Apologies for the late review.
I am glad that caching this seems to be a viable fix.
Would it be possible to use caching decorators for a simpler solution? https://docs.python.org/3/library/functools.html#functools.lru_cache
Edit: perhaps functools.cached_property is more relevant
could a dataclass be used to make the cache more structured?
https://docs.python.org/3/library/dataclasses.html
| _lastTableSelection = None | ||
| _lastTableAxis = None | ||
| _lastTableRow = None | ||
| _lastTableRowSpan = None | ||
| _lastTableCol = None | ||
| _lastTableColSpan = None |
There was a problem hiding this comment.
Could this be turned into a dataclass?
|
@codeofdusk are you interested in picking this up? |
|
Excuse, how to download a try build for this PR? Or could you consider to add this feature to an addon. Thanks. |
|
I plan to fix up this PR slightly and open a new one for 2022.1 |
|
Closing in favour of #13345 |
…lumn/row index (#13345) Follow up on the work done by @mltony in #11923 Fixes #7278, #11919 Summary of the issue: When issuing table navigation commands repeatedly, e.g. pressing Control+Alt+Down several times in a row, the original column index is currently not preserved. Thus, if there happens to be a merged cell on the way, the column index will be reset to the minimal column index of merged cell. Description of how this pull request fixes the issue: This PR caches original column index for vertical table navigation and original row index for horizontal table navigation.
Link to issue number:
#11919, #7278
Summary of the issue:
When issuing table navigation commands repeadetly, e.g. pressing Control+Alt+Down several times in a row, the original column index is currently not preserved. Thus, if there happens to be a merged cell on the way, the column index will be reset to the minimal column index of merged cell.
Description of how this pull request fixes the issue:
This PR caches original column index for vertical table navigation and original row index for horizontal table navigation.
Testing performed:
Performed test described in #11919.
Known issues with pull request:
None known at the moment.
Change log entry:
Fix table navigation commands to preserve column/row index in the presence of merged cells.
Section: Bug fixes