Skip to content
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.

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

Accessibility : Enhance Dropdown Accessibility with Arrow Key Navigation #10378

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Darshan-upadhyay1110
Copy link
Contributor

  • Resolves: #
  • Target version: master

Summary

TODO

  • ...

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

let pos = parseInt(data.pos)
do {
pos++; // Move to the next position
nextElement = document.getElementById(data.comboboxId + '-entry-' + pos);
Copy link
Contributor

@eszkadev eszkadev Oct 30, 2024

Choose a reason for hiding this comment

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

please try to avoid using getElementById, it's fragile on future changes.
Can we maybe use JSDialog.GetFocusableElements which we will just iterate?

or can we create helper: JSDialog.GetNextFocusableElement which will find element by iterating the DOM?

EDIT: it seems something exists already:

JSDialog.FindFocusableElement(
  potentialCurrentElement.previousElementSibling,
  'previous'
  );

Copy link
Contributor

Choose a reason for hiding this comment

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

or maybe even we can just use JSDialog.KeyboardGridNavigation ?

Copy link
Contributor

Choose a reason for hiding this comment

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

we shouldn't implement the same ting few times, ideally it should be simple algorithm which uses "siblings" and not any "ids" and just looks for next one focusable on arrow right, previous on arrow left etc...
this way we can use it in any place without modification

Copy link
Contributor Author

@Darshan-upadhyay1110 Darshan-upadhyay1110 Oct 30, 2024

Choose a reason for hiding this comment

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

Yes i can try on that but still i think to get Dropdown container i need to use byID one time.
const comboboxEntries = document.getElementById(data.id +'-entries');

Copy link
Contributor Author

@Darshan-upadhyay1110 Darshan-upadhyay1110 Oct 30, 2024

Choose a reason for hiding this comment

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

EDIT: it seems something exists already:
JSDialog.FindFocusableElement(
potentialCurrentElement.previousElementSibling,
'previous'
);

This will not work as it also return the element if current is focusable we need to pass potential next element to this function and this will work...

like this :

image

Copy link
Contributor

Choose a reason for hiding this comment

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

there is helper for that too: var dropdownRoot = JSDialog.GetDropdown(data.id);

we at least need to encapsulate all these "get by id" so it will be single place to modify in the future (if id generation will be changed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So indirectly need to get next element and pass to this function. Or better we just create new helper for it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is helper for that too: var dropdownRoot = JSDialog.GetDropdown(data.id);

we at least need to encapsulate all these "get by id" so it will be single place to modify in the future (if id generation will be changed)

I see okay then it will work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me do the changes and test, Thanks :)

Copy link
Contributor

Choose a reason for hiding this comment

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

less duplicated code == better :)

I would try:

  1. JSDialog.KeyboardGridNavigation (it works left, right, up, down, might be not good)
  2. if above doesn't work, please try to create generic: JSDialog.KeyboardListNavigation (up,down only)
  3. do custom thing for this one

@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the private/darshan/accessibility-add-keyboard-navigation-in-combobox-entries branch 2 times, most recently from 6fd6217 to 9fc9d1f Compare November 4, 2024 13:15
- to compare text value we need to compare the property and not a whole object

Signed-off-by: Darshan-upadhyay1110 <darshan.upadhyay@collabora.com>
Change-Id: I7db33c9280da229ec656e52c2fe8158739291085
- Enable Arrow key navigation in dropdown menus.
- Allow users to move between options using Up/Down keys.( Specially for comboboxes entry)
- Improve usability for screen reader and keyboard-only users.

Signed-off-by: Darshan-upadhyay1110 <darshan.upadhyay@collabora.com>
Change-Id: I60ad384d1ee2b176be8dd8c12c37fcd02dafe5a2
Signed-off-by: Darshan-upadhyay1110 <darshan.upadhyay@collabora.com>
Change-Id: I21954a8c5c10c6f8f355fd6e4cb6949b2be9824e
Problem:
    - in case of submenu (for example in Calc there is a conditional format menu )
    - when we open submenu it and then press ESC it changes focus back to document
    - and the main parent menu stays open without focus

Solution: - call LastElement focus before Clear dialog so it works as expected (MSO does the same)
Signed-off-by: Darshan-upadhyay1110 <darshan.upadhyay@collabora.com>
Change-Id: Ic8fa988f3a8854a99c44d0fde5a7dec32e398739
@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the private/darshan/accessibility-add-keyboard-navigation-in-combobox-entries branch from 9fc9d1f to 4012ddd Compare November 6, 2024 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To Review
Development

Successfully merging this pull request may close these issues.

2 participants