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

<sl-menu> and <sl-menu-item> regression in 2.0.0-beta.79 caused by allowing focus on disabled items for improved accessibility #845

Closed
CetinSert opened this issue Aug 2, 2022 · 6 comments
Assignees
Labels
bug Things that aren't working right in the library.

Comments

@CetinSert
Copy link

CetinSert commented Aug 2, 2022

Describe the bug

Disabled (and also not displayed (CSS: display: none)) menu items allow focus (via keyboard o__O and mouse).
This weird behavior introduced in 2.0.0-beta.79 potentially for project-internal consistency runs contrary to all established expectations.

No other desktop/web app allows focus on disabled menu items (even when these are still displayed) via keyboard or pointer.

Please see examples below. I can add dozens more to convince you to revert this regression.

@claviska

@CetinSert CetinSert added the bug Things that aren't working right in the library. label Aug 2, 2022
@CetinSert
Copy link
Author

CetinSert commented Aug 2, 2022

Windows / Notepad

press key result
image image

Pointer focus is also not possible for disabled items.

@CetinSert
Copy link
Author

CetinSert commented Aug 2, 2022

macOS / Finder

press key result
image image

Pointer focus is also not possible for disabled items.

@CetinSert
Copy link
Author

CetinSert commented Aug 2, 2022

web / vscode.dev

press key result
image image

Pointer focus is also not possible for disabled items.

@CetinSert CetinSert changed the title Improved accessibility of <sl-menu> and <sl-menu-item> by allowing focus on disabled items Improved accessibility of <sl-menu> and <sl-menu-item> by allowing focus on disabled items causes regression Aug 2, 2022
@CetinSert
Copy link
Author

CetinSert commented Aug 2, 2022

web components / Adobe Spectrum

press key result
image image

Pointer focus is also not possible for disabled items.

@CetinSert
Copy link
Author

CetinSert commented Aug 2, 2022

Vaadin: https://vaadin.com/docs/latest/components/menu-bar/#disabled-items (focus not possible via keyboard and pointer.)

@claviska
Copy link
Member

claviska commented Aug 2, 2022

This wasn't sitting right with me either. I've always heard and read — although I can't find the W3 site that says it at the moment — that disabled items should be focusable contrary to what operating systems and even form controls in the browser do.

This is inconsistent, as demonstrated here:

https://jsfiddle.net/8rh9o7ut/

I've also been in the room for multiple arguments about whether form controls should ever even be disabled, which seems to be more of a tabs vs. spaces-style discussion. I did some more reading on this and reverted my opinion with one condition that I've outlined in the contribution guidelines, (emphasis mine):

Focusing on Disabled Items

When an item within a keyboard navigable set is disabled (e.g. tabs, trees, menu items, etc.), the disabled item should not receive focus via keyboard, click, or tap. It should be skipped just like in operating system menus and in native HTML form controls. There is no exception to this. If a particular item requires focus for assistive devices to provide a good user experience, the item should not be disabled and, upon activation, it should inform the user why the respective action cannot be completed.

The bottom line is, sighted users expect disabled items to be skipped. This behavior saves keypresses and maintains consistency with operating system menus, form controls, etc. Additionally, disabled items are still discoverable by screen readers when using modifier keys (a common way to navigate HTML documents).

In this video, I arrow through the menu items twice. The first time, using Up|Down. The second time, using Control + Option + Left|Right.

This is the convention Shoelace will stick with unless there's definitive proof that this approach is wrong, harmful, or inaccessible to users.

keyboard.mp4

@CetinSert CetinSert changed the title Improved accessibility of <sl-menu> and <sl-menu-item> by allowing focus on disabled items causes regression Improved accessibility of <sl-menu> and <sl-menu-item> by allowing focus on disabled items causes regression Aug 16, 2022
@CetinSert CetinSert changed the title Improved accessibility of <sl-menu> and <sl-menu-item> by allowing focus on disabled items causes regression <sl-menu> and <sl-menu-item> regression caused by allowing focus on disabled items for improved accessibility Aug 16, 2022
@CetinSert CetinSert changed the title <sl-menu> and <sl-menu-item> regression caused by allowing focus on disabled items for improved accessibility <sl-menu> and <sl-menu-item> regression in 2.0.0-beta.79 caused by allowing focus on disabled items for improved accessibility Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

No branches or pull requests

2 participants