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(MegaMenu): remove unnecessary role for dropdown menu/megamenu #4624

Merged

Conversation

annawen1
Copy link
Member

@annawen1 annawen1 commented Dec 3, 2020

Related Ticket(s)

React: Issues Mega menu options navigation #4196

Description

The dropdown menu/megamenu shouldn't have the role="menu" applied as listed here:
Example Disclosure for Navigation Menus | WAI-ARIA Authoring Practices 1.1
https://www.w3.org/TR/wai-aria-practices/examples/disclosure/disclosure-navigation.html

Screen Shot 2020-12-03 at 4 25 20 PM

Changelog

New

  • {{new thing}}

Changed

  • {{changed thing}}

Removed

  • role attribute from the <HeaderMenu /> component

@annawen1 annawen1 added the package: react Work necessary for the Carbon for IBM.com react components package label Dec 3, 2020
Copy link
Member

@jeffchew jeffchew left a comment

Choose a reason for hiding this comment

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

LGTM!

@jeffchew jeffchew added the Ready to merge Label for the pull requests that are ready to merge label Dec 3, 2020
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Dec 3, 2020

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Dec 3, 2020

@kodiakhq kodiakhq bot merged commit 77633bb into carbon-design-system:master Dec 4, 2020
@annawen1 annawen1 deleted the fix/megamenu-remove-role branch February 10, 2021 18:19
kodiakhq bot pushed a commit that referenced this pull request Feb 12, 2021
### Related Ticket(s)

Web component: Mega Menu - "The essentials, Hybrid Cloud" are read as menu options for screen reader users. #5081

### Description

This PR removes the `role='menu'` from the `ul` as it is unnecessary and conflicts with the elements placed in the megamenu/dropdown.

PR here from carbon-for-ibm-dotcom where we removed the menu role from our own version of the header menu
#4624

The dropdown menu/megamenu shouldn't have the role="menu" applied as listed here:
Example Disclosure for Navigation Menus | WAI-ARIA Authoring Practices 1.1
https://www.w3.org/TR/wai-aria-practices/examples/disclosure/disclosure-navigation.html

<img width="950" alt="Screen Shot 2020-12-03 at 4 25 20 PM" src="https://user-images.githubusercontent.com/54281166/101090282-661c0680-3584-11eb-8dd0-b43ca6f9aa7e.png">

### Changelog

**Changed**

- grab the `bx--header-menu` element and remove the `role` attribute


<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "package: styles": Carbon Expressive -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
IgnacioBecerra pushed a commit to IgnacioBecerra/ibm-dotcom-library that referenced this pull request Feb 22, 2021
…em#5160)

### Related Ticket(s)

Web component: Mega Menu - "The essentials, Hybrid Cloud" are read as menu options for screen reader users. carbon-design-system#5081

### Description

This PR removes the `role='menu'` from the `ul` as it is unnecessary and conflicts with the elements placed in the megamenu/dropdown.

PR here from carbon-for-ibm-dotcom where we removed the menu role from our own version of the header menu
carbon-design-system#4624

The dropdown menu/megamenu shouldn't have the role="menu" applied as listed here:
Example Disclosure for Navigation Menus | WAI-ARIA Authoring Practices 1.1
https://www.w3.org/TR/wai-aria-practices/examples/disclosure/disclosure-navigation.html

<img width="950" alt="Screen Shot 2020-12-03 at 4 25 20 PM" src="https://user-images.githubusercontent.com/54281166/101090282-661c0680-3584-11eb-8dd0-b43ca6f9aa7e.png">

### Changelog

**Changed**

- grab the `bx--header-menu` element and remove the `role` attribute


<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "package: styles": Carbon Expressive -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: react Work necessary for the Carbon for IBM.com react components package Ready to merge Label for the pull requests that are ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants