Skip to content

fix-combobox-a11y #13308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

fix-combobox-a11y #13308

wants to merge 10 commits into from

Conversation

@rparth07
Copy link
Contributor

@rparth07 rparth07 commented Oct 29, 2025 •
edited
Loading

Navigation Preview:

msedge_ZUugqTjvfl.mp4

Changes:

  1. added tabindex support to grid and combobox widgets, only if exists. Now, listbox will have tabindex=0 and its option will have tabinex=-1 to make programmatically navigable.
  2. removed Tab key navigation from listbox keyboard navigation per accessibility guideline
  3. enabled keyboard navigation for listbox using only arrow keys
  4. added aria-selected attribute to combobox entries
  5. restricted keyboard navigation to ArrowDown and ArrowUp keys in listbox(earlier exist otherwise)
  6. added support to close dropdown on Tab key press as per accessibility compliance
  7. added aria-activedescendant attribute to improve accessibility in listbox navigation
  8. updated Widget.PageMarginEntry to also behave as above.
  9. Added missing Enter and Space key accessibility in Widget.PageMarginEntry

Checklist

  • I have run make prettier-write and formatted the code.
  • All commits have Change-Id
  • I have run tests with make check
  • I have issued make run and manually verified that everything looks okay
  • Documentation (manuals or wiki) has been updated or is not required

@rparth07
fix: add tabindex support to grid and combobox widgets for improved a…
b19f16a 
…ccessibility

Signed-off-by: Parth Raiyani <[email protected]>
Change-Id: Ifc5612c1b016d1b9e915826bafc90728d3304e68
@rparth07
fix: remove Tab key navigation from listbox keyboard navigation per a…
8b949cc 
…ccessibility guideline

Signed-off-by: Parth Raiyani <[email protected]>
Change-Id: I0696c517e52c05766e5360f0beee69364babba21
eszkadev
eszkadev reviewed Oct 30, 2025
View reviewed changes
@rparth07
fix: enable keyboard navigation for listbox using only arrow keys
3bd6b99 
- added default selection to first item from dropdown if none is selected
- enhanced setupInitialFocus to have focus on first selectable item
- updated isFocusable method to make listbox -> options focusable

Change-Id: Ie9c3c552ae860c6edee2c259804dd3f0730367b6
Signed-off-by: Parth Raiyani <[email protected]>
@rparth07
fix: add aria-selected attribute to combobox entries for improved acc…
b27b171 
…essibility

Signed-off-by: Parth Raiyani <[email protected]>
Change-Id: I31842d6ed3d2643742e21da093dfa0ca350f3ccc
@rparth07
fix: restrict keyboard navigation to ArrowDown and ArrowUp keys in li…
52e9ffb 
…stbox

Signed-off-by: Parth Raiyani <[email protected]>
Change-Id: I96ff2acedae0928deaf46a8fcb86705e11da4c4a
@rparth07
fix: close dropdown on Tab key press to preserve focus order and acce…
20964d5 
…ssibility compliance

Signed-off-by: Parth Raiyani <[email protected]>
Change-Id: I4a3bad96f0e1f713d1eea8653fbaba240dda942a
@rparth07
fix: add aria-activedescendant attribute to improve accessibility in …
9763e00 
…listbox navigation

Change-Id: Id83abc8201b895ba824c7795dec787cef50e1232
Signed-off-by: Parth Raiyani <[email protected]>
@rparth07
fix: enhance accessibility by adding tabindex and aria attributes to …
3bc9c78 
…page margin entry widget

- This change wil ensure that  by default first is selected and gets initial focus.
- Added logic to close the dropdown on tab

Signed-off-by: Parth Raiyani <[email protected]>
Change-Id: I95f835c88fee6708f4735e1abef30ab75c804ef8
@rparth07
fix: enhance keyboard accessibility by allowing Enter and Space keys …
cdd7c8b 
…to trigger relevant selection logic in page margin widget

Signed-off-by: Parth Raiyani <[email protected]>
Change-Id: Ie8a2735655a6fe14e00d023b554c48bafcfce47d
@rparth07 rparth07 force-pushed the private/rparth07/fix-combobox-a11y branch from 44b0748 to cdd7c8b Compare October 30, 2025 13:54
@rparth07
Copy link
Contributor Author

rparth07 commented Oct 30, 2025

Changes:

  1. added tabindex support to grid and combobox widgets, only if exists. Now, listbox will have tabindex=0 and its option will have tabinex=-1 to make programmatically navigable.
  2. removed Tab key navigation from listbox keyboard navigation per accessibility guideline
  3. enabled keyboard navigation for listbox using only arrow keys
  4. added aria-selected attribute to combobox entries
  5. restricted keyboard navigation to ArrowDown and ArrowUp keys in listbox(earlier exist otherwise)
  6. added support to close dropdown on Tab key press as per accessibility compliance
  7. added aria-activedescendant attribute to improve accessibility in listbox navigation
  8. updated Widget.PageMarginEntry to also behave as above.
  9. Added missing Enter and Space key accessibility in Widget.PageMarginEntry

Added last two changes with this update.

eszkadev
eszkadev reviewed Oct 30, 2025
View reviewed changes
browser/src/control/jsdialog/Util.Dropdown.js Outdated
selected: (i === 0 && shouldSelectFirstEntry) ? true : entries[i].selected,
hasSubMenu: !!entries[i].items
};
if(entry.selected) initialSelectedId = entry.id;
Copy link
Contributor

@eszkadev eszkadev Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if_ (space)

Copy link
Contributor Author

@rparth07 rparth07 Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks:) I fixed this now. Will it be okay if I start with this file and remove it from .beforeprettier as a separate PR?

eszkadev
eszkadev approved these changes Oct 30, 2025
View reviewed changes
Copy link
Contributor

@eszkadev eszkadev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, thanks :)

@github-project-automation github-project-automation bot moved this from To Review to To Test in Collabora Online Oct 30, 2025
@eszkadev
Copy link
Contributor

eszkadev commented Oct 30, 2025

tests are failing and it looks like related
Expected to find element: [id$="-dropdown"].modalpopup img[alt="Alef"], but never found it. Queried from:
Error: The 'selections' image is different. Threshold limit of '0.02' exceeded: '0.02' - [Show Difference](#visualRegressionPopup0.282562661049395)

@rparth07
Copy link
Contributor Author

rparth07 commented Oct 30, 2025

image

Updated to prevent adding aria-labelledby attribute if does not exist.

@rparth07
Copy link
Contributor Author

rparth07 commented Oct 30, 2025 •
edited
Loading

tests are failing and it looks like related Expected to find element: [id$="-dropdown"].modalpopup img[alt="Alef"], but never found it. Queried from: Error: The 'selections' image is different. Threshold limit of '0.02' exceeded: '0.02' - [Show Difference](#visualRegressionPopup0.282562661049395)

Strange! it is passing locally.
image

@eszkadev
Copy link
Contributor

eszkadev commented Oct 31, 2025

please fix code formatting:

  ESLINT   eslint
  PRETTIER prettier
[warn] src/control/jsdialog/Util.KeyboardListNavigation.ts
[warn] src/control/jsdialog/Widget.Containers.ts
[warn] src/control/jsdialog/Widget.PageMarginEntry.ts
[warn] Code style issues found in 3 files. Run Prettier with --write to fix.
make[1]: *** [Makefile:1881: prettier] Error 1

@rparth07
fix: only add aria-labelledby attribute to dialog form if exist
e3c121c 
Change-Id: Ic8544a66f620c4b20c95d2a39864ae65c0548518
Signed-off-by: Parth Raiyani <[email protected]>
@rparth07 rparth07 force-pushed the private/rparth07/fix-combobox-a11y branch from 5c2bc0b to e3c121c Compare October 31, 2025 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eszkadev eszkadev eszkadev approved these changes

@Darshan-upadhyay1110 Darshan-upadhyay1110 Awaiting requested review from Darshan-upadhyay1110

Assignees

No one assigned

Labels

None yet

Projects

Collabora Online
Status: To Test

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rparth07 @eszkadev