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

fix(CUI-7441): Coral.CycleButton: fix menuitemradio and menuitem roles #153

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

majornista
Copy link
Collaborator

Per https://jira.corp.adobe.com/browse/CUI-7441

Description

In both the CoralUI and Coral Spectrum CycleButton implementations, the menuitemradio role on coral-cyclebutton-item and the menuitem role on coral-cyclebutton-action are not propagated to coral-selectlist-item and button[is="coral-buttonlist-item"] elements in the dropdown menu. Items within a menu should have a menuitem* role.

Behavior can be seen in Coral Spectrum:

  1. Open https://opensource.adobe.com/coral-spectrum/examples/#cyclebutton
  2. Expand dropdown menu for the "With additional actions" CycleButton example.
  3. Use the Web Inspector to inspect coral-selectlist-item elements in the coral-selectlist[role="group"] and button[is="coral-buttonlist-item"] in the coral-buttonlist contained within the coral-popover[role="menu"].

Expected behavior

coral-selectlist-item elements should have role="menuitemradio"
button[is="coral-buttonlist-item"] elements should have role="menuitem"

Actual behavior

  • coral-selectlist-item elements have role="option", which is not appropriate for an item within role="menu".
  • button[is="coral-buttonlist-item"] elements have the implicit role of button, which is not appropriate for an item within role="menu".

Suggested fix

At CycleButton.js#L653 and CycleButton.js#L668, where we are defining items for the SelectList and actions for the ButtonList, we seem to expect the role from the coral-cyclebutton-item or coral-cyclebutton-action to get applied to the SelectList.Item or ButtonList.Item, but these components do not include getters and setters for the role attribute, so the role is not applied. We should instead either explicitly set the role attribute or add a getter/setter for role to SelectList.Item and ButtonList.Item.

Related Issue

https://jira.corp.adobe.com/browse/CUI-7441

Motivation and Context

Fixes accessibility implementation for items in the cycle button dropdown menu.

How Has This Been Tested?

Tested against documentation examples.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

Per https://jira.corp.adobe.com/browse/CUI-7441

In both the CoralUI and Coral Spectrum CycleButton implementations, the `menuitemradio` role on `coral-cyclebutton-item` and the `menuitem` role on `coral-cyclebutton-action` are not propagated to `coral-selectlist-item` and `button[is="coral-buttonlist-item"]` elements in the dropdown menu. Items within a menu should have a `menuitem*` role.

### Behavior can be seen in Coral Spectrum:
1. Open https://opensource.adobe.com/coral-spectrum/examples/#cyclebutton
2. Expand dropdown menu for the "With additional actions" CycleButton example.
3. Use the Web Inspector to inspect `coral-selectlist-item` elements in the `coral-selectlist[role="group"]` and `button[is="coral-buttonlist-item"]` in the `coral-buttonlist` contained within the `coral-popover[role="menu"]`.

#### Expected behavior
coral-selectlist-item elements should have role="menuitemradio"
button[is="coral-buttonlist-item"] elements should have role="menuitem"

#### Actual behavior
* `coral-selectlist-item` elements have `role="option"`, which is not appropriate for an item within `role="menu"`.
* `button[is="coral-buttonlist-item"]` elements have the implicit role of `button`, which is not appropriate for an item within `role="menu"`.

### Suggested fix
At CycleButton.js#L653 and CycleButton.js#L668, where we are defining items for the SelectList and actions for the ButtonList, we seem to expect the role from the `coral-cyclebutton-item` or `coral-cyclebutton-action` to get applied to the SelectList.Item or ButtonList.Item, but these components do not include getters and setters for the role attribute, so the role is not applied. We should instead either explicitly set the role attribute or add a getter/setter for role to SelectList.Item and ButtonList.Item.
@majornista
Copy link
Collaborator Author

majornista commented Feb 19, 2021

@icaraps Could I get a review?

@majornista majornista requested review from lazd and icaraps and removed request for lazd February 19, 2021 20:29
@icaraps icaraps requested review from Pareesh and CezCz and removed request for icaraps February 24, 2021 09:33
Copy link
Collaborator

@Pareesh Pareesh left a comment

Choose a reason for hiding this comment

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

LGTM

@majornista majornista merged commit 7bd0ffe into adobe:master Apr 9, 2021
github-actions bot pushed a commit that referenced this pull request Apr 9, 2021
## [4.10.17](v4.10.16...v4.10.17) (2021-04-09)

### Bug Fixes

* **CUI-7441:** Coral.CycleButton: fix menuitemradio and menuitem roles ([#153](#153)) ([7bd0ffe](7bd0ffe)), closes [CycleButton.js#L653](https://github.com/CycleButton.js/issues/L653) [CycleButton.js#L668](https://github.com/CycleButton.js/issues/L668)
@github-actions
Copy link

github-actions bot commented Apr 9, 2021

🎉 This PR is included in version 4.10.17 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants