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

Buttons with dropdown should feature proper semantic markup #12258

Closed
mlewand opened this issue Aug 11, 2022 · 2 comments · Fixed by #12283
Closed

Buttons with dropdown should feature proper semantic markup #12258

mlewand opened this issue Aug 11, 2022 · 2 comments · Fixed by #12283
Assignees
Labels
domain:accessibility This issue reports an accessibility problem. package:ui squad:features Issue to be handled by the Features team. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@mlewand
Copy link
Contributor

mlewand commented Aug 11, 2022

📝 Provide a description of the improvement

Extracted from #1037

The screen reader should properly announce that the button has a popup,

We already cover aria-haspopup well enough.

However, we must handle aria-expanded for dropdown button views. Interestingly split buttons already have it implemented, so this could serve as a reference.


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@mlewand mlewand added type:improvement This issue reports a possible enhancement of an existing feature. package:ui domain:accessibility This issue reports an accessibility problem. squad:features Issue to be handled by the Features team. labels Aug 11, 2022
@mlewand
Copy link
Contributor Author

mlewand commented Aug 11, 2022

There has been some discussion in #1037 already regarding this:

@oleq wrote:

  1. Our dropdowns have no aria-expanded but should have (see an example from w3c https://www.w3.org/TR/wai-aria-practices-1.1/examples/listbox/listbox-collapsible.html)
    • The only places that have that attribute are CollapsibleView (list properties: start index, reversed section) and SplitButtonView (a button for dropdowns with an “extra arrow”).

@Comandeer wrote:

The screen reader should properly announce that the button has a popup,

It can be done by adding the [aria-haspopup] and [aria-expanded] attributes to buttons triggering dropdowns. Depending on what the dropdown is, the [aria-haspopup] attribute can take different values instead of generic true (e.g. menu if the dropdown is some kind of a menu). We also need to ensure that the dropdown itself has an appropriate role (e.g. [role=menu] in case of menu dropdown). ARIA Practices contains a decent menu button sample.

@Comandeer
Copy link
Member

Tested on [https://ckeditor.com/docs/ckeditor5/latest/examples/builds-custom/full-featured-editor.html](https://ckeditor.com/docs/ckeditor5/latest/examples/builds-custom/full-featured-editor.htmla) and "Text Color" button.

  1. The button is correctly marked up as having a submenu by the [aria-haspopup=true] attribute

    1. Optionally: check if we can't find more concrete value for this attribute (this could be tricky as it probably depends on context – some buttons would have menus, some grids, etc.)
  2. There is no info about the state of the menu – if it is expanded or collapsed. It could be fixed by adding the [aria-expanded] attribute to the button.

  3. The menu itself does not have a proper role. It is a requirement in ARIA spec:

    Authors MUST ensure that the role of the element that serves as the container for the popup content is menu, listbox, tree, grid, or dialog, and that the value of aria-haspopup matches the role of the popup container.

  4. We can also think about adding the [aria-controls] attribute to indicate the relationship between the button and the menu.

@mlewand mlewand assigned mateuszzagorski and unassigned Comandeer Aug 16, 2022
@CKEditorBot CKEditorBot added status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Aug 16, 2022
mlewand added a commit that referenced this issue Aug 17, 2022
…rkup

Fix (ui): Added semantic attribute for marking the dropdown button as expanded. Closes #12258.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Aug 17, 2022
@CKEditorBot CKEditorBot added this to the iteration 56 milestone Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:accessibility This issue reports an accessibility problem. package:ui squad:features Issue to be handled by the Features team. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants