Focus tab title instead of panel switching tabs in add-on store by ABuffEr · Pull Request #15988 · nvaccess/nvda

ABuffEr · 2023-12-30T20:41:09Z

Link to issue number:

See discussion in #14986

Summary of the issue:

Switching tabs in add-on store with ctrl+tab announces "tab control panel", and moves the focus in a strange position.

Description of user facing changes

Users will be positioned on tab list, over the new current tab, that will be read by NVDA.

Description of development approach

Simply SetFocus on addonListTabs (wx.Notebook) if it's not already here, at the end of page change event handler.

Testing strategy:

Manual, with steps in #14986.

Known issues with pull request:

It's a partial fix, as discussed in #14986, because it doesn't move focus to a panel element like the channel filter or the add-on list, that seems unreasonably complex to reach. But it's certainly better than nothing/current situation.

Being a small but very perceptible change, I suggest to integrate into 2024.1, and only later to work on another better solution.

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.

nvdaes · 2023-12-30T21:10:05Z

Being a small but very perceptible change, I suggest to integrate into 2024.1, and only later to work on another better solution.

I strongly agree with this proposal. I've reviewed changes and tested them, and I'd approve them if the changes.t2t file would reflect this, probably in the section corresponding to fixes, with credits to you @ABuffEr

amirsol81 · 2023-12-30T21:27:11Z

@ABuffEr really nice touch! I was always baffled by NVDA's behavior upon pressing Control+Tab in the Add-on Manager.

ABuffEr · 2023-12-30T21:47:10Z

I've reviewed changes and tested them, and I'd approve them if the changes.t2t file would reflect this, probably in the section corresponding to fixes, with credits to you @ABuffEr

Hi @nvdaes,
thanks, my last PR dates back a lot of time ago, I updated the first comment and filled the changelog part (that I remembered differently). Hope it's ok now.

nvdaes · 2023-12-30T21:58:32Z

I updated the first comment and filled the changelog part (that I remembered differently).

You need to update 'changes.t2t`. See the Change log entry section in contributing.md

XLTechie

@ABuffEr this is great, a much improved user experience for tab changing in the store.

@seanbudd @michaelDCurran Please do consider including in beta!

lukaszgo1

I'm fine with this, as long as we agree that this is a work around, and ideally list of add-ons should be focused.

nvdaes · 2023-12-31T14:21:25Z

Sincerely, I'm not totally convinced about the best element to be focused. Some people may prefer to focus the box to filter add-ons. But imo this pr is crutial and should be included in NVDA 2024.1.

ABuffEr · 2023-12-31T14:35:26Z

Sincerely, I'm not totally convinced about the best element to be focused. Some people may prefer to focus the box to filter add-ons.

I agree at least until #15947 is solved/implemented; then, focusing on add-on list sounds more reasonable, and coherent with what happens e.g. in symbols dialog.

nvdaes · 2023-12-31T14:47:21Z

with what happens e.g. in symbols dialog.

I think this should be discussed in a different thread, since the fact that we use lists at the end doesn't mean that we want to start focusing lists in NVDA dialogs, even if this has been done during long time.

LeonarddeR · 2024-01-01T18:16:45Z

Looking at this pr and associated issue, I wonder why the MultiCategorySettingsDialog approach wasn't chosen for the add-ons manager. That has custom logic for ctrl+tab I believe.

CyrilleB79 · 2024-01-01T18:37:42Z

I have not tested this PR. But from the description, I fully support its inclusion in beta branch (cc @seanbudd, @michaelDCurran).
This is still much better than today's very bad user experience.

The discussion on the best place where the focus should land can still remain open after merging this PR.

seanbudd · 2024-01-01T23:29:24Z

  - When reinstalling an incompatible add-on it is no longer forcefully disabled. (#15584, @lukaszgo1)
  - Disabled and incompatible add-ons can now be updated. (#15568, #15029)
  - NVDA now recovers and displays an error in a case where an add-on fails to download correctly. (#15796)
+  - When pressing ctrl+tab, focus properly moves to the new current tab title. (#14986, @ABuffEr)


this PR is targeting master and should go into 2024.2. Please move the change log to 2024.2.

My mistake, looking now at branches available I suppose to have to do a PR targeting beta; if you'll accept it I close this and open another PR. I'll move changelog line otherwise, but 2024.1 would be preferable.

2024.1 is already deep into beta stage - it is too late for risky changes of behaviour - particularly this case where it is generally agreed that it might not be the final behaviour. I believe the behaviour included in this PR was part of the original add-on store PR (if not, it was something similar) which was reverted to the current behaviour based on feedback.

I strongly doubt that there was a feedback against this feature, I'm quite sure it's an accessibility issue of that wx control from a long time ago (I mean, at least since we changed settings structure years and years ago).
Anyway, ok, I disagree but will change release target in changelog with next commit. Move it as needed if discussion proceeds.

For reference, the prototype set focus to the add-on list view, this was later reverted due to feedback and the comment/issue was created to find a new solution.

7009a95

643fa00

Add-on store discussion #14912 (comment)

cary-rowen · 2024-01-02T16:15:46Z

Hi @seanbudd

This is a useful improvement, I receive this feedback frequently in the Chinese community, and I agree that it should be targeted for 2024.1.

@ABuffEr

Because it doesn't move focus to a panel element like the channel filter or the add-on list, that seems unreasonably complex to reach.
Also, I agree with @CyrilleB79's comment in #14986.

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

seanbudd

Thanks @ABuffEr

…cess#15988) See discussion in nvaccess#14986 Summary of the issue: Switching tabs in add-on store with ctrl+tab announces "tab control panel", and moves the focus in a strange position. Description of user facing changes Users will be positioned on tab list, over the new current tab, that will be read by NVDA. Description of development approach Simply SetFocus on addonListTabs (wx.Notebook) if it's not already here, at the end of page change event handler.

Focus tab title instead of panel switching tabs in add-on store

5fe4a26

ABuffEr requested a review from a team as a code owner December 30, 2023 20:41

ABuffEr requested a review from gerald-hartig December 30, 2023 20:41

Commit changes

2bbe9c9

nvdaes approved these changes Dec 30, 2023

View reviewed changes

XLTechie approved these changes Dec 31, 2023

View reviewed changes

lukaszgo1 approved these changes Dec 31, 2023

View reviewed changes

seanbudd reviewed Jan 1, 2024

View reviewed changes

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

seanbudd added this to the 2024.2 milestone Jan 2, 2024

Moved change to bug fixes of 2024.2 release

c0a8aaf

seanbudd reviewed Jan 3, 2024

View reviewed changes

Comment thread source/gui/addonStoreGui/controls/storeDialog.py

Comment thread source/gui/addonStoreGui/controls/storeDialog.py

Comment thread user_docs/en/changes.t2t

seanbudd marked this pull request as draft January 3, 2024 22:43

ABuffEr and others added 3 commits January 4, 2024 00:13

Update source/gui/addonStoreGui/controls/storeDialog.py

c82a1cf

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

Update user_docs/en/changes.t2t

96217da

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

Restore deleted empty lines

a2bbf98

ABuffEr marked this pull request as ready for review January 3, 2024 23:24

seanbudd approved these changes Jan 3, 2024

View reviewed changes

seanbudd merged commit 88ceef4 into nvaccess:master Jan 4, 2024

Adriani90 mentioned this pull request Feb 20, 2024

Add-on Store: First press of Ctrl+Tab shouldn't be improperly announced #14986

Closed

Uh oh!

Conversation

ABuffEr 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!

nvdaes commented Dec 30, 2023

Uh oh!

amirsol81 commented Dec 30, 2023

Uh oh!

ABuffEr commented Dec 30, 2023

Uh oh!

nvdaes commented Dec 30, 2023

Uh oh!

XLTechie left a comment

Choose a reason for hiding this comment

Uh oh!

lukaszgo1 left a comment

Choose a reason for hiding this comment

Uh oh!

nvdaes commented Dec 31, 2023

Uh oh!

ABuffEr commented Dec 31, 2023

Uh oh!

nvdaes commented Dec 31, 2023

Uh oh!

LeonarddeR commented Jan 1, 2024

Uh oh!

CyrilleB79 commented Jan 1, 2024

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ABuffEr Jan 2, 2024

Choose a reason for hiding this comment

Uh oh!

seanbudd Jan 2, 2024

Choose a reason for hiding this comment

Uh oh!

ABuffEr Jan 3, 2024

Choose a reason for hiding this comment

Uh oh!

seanbudd Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

cary-rowen commented Jan 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seanbudd left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ABuffEr commented Dec 30, 2023 •

edited

Loading

seanbudd Jan 1, 2024 •

edited

Loading