-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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(ui-shell): remove role='menu' from ui-shell #5235
fix(ui-shell): remove role='menu' from ui-shell #5235
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
duplicate of #5234? I self assigned those tickets...will close in favor of this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
couldn't add this in the review but I also edited this comment and removed the role on L222 |
Deploy preview for the-carbon-components ready! Built with commit bbf65e2 https://deploy-preview-5235--the-carbon-components.netlify.com |
4f5342e
to
4949f24
Compare
@emyarod updated |
Deploy preview for carbon-components-react failed. Built with commit 4f5342e https://app.netlify.com/sites/carbon-components-react/deploys/5e345de34e45400008246931 |
Deploy preview for carbon-elements ready! Built with commit 4f5342e |
Deploy preview for carbon-components-react failed. Built with commit bbf65e2 https://app.netlify.com/sites/carbon-components-react/deploys/5e3487be48554e0008921853 |
Deploy preview for carbon-elements ready! Built with commit bbf65e2 |
Closes #3583
Closes #3590
Closes #3584
We erroneously had role="menu" implying an application menu.
Changelog
Removed
role="menu"
and 'role="menuitem"`Testing / Reviewing
Make sure UI Shell still looks the same, and there are no new DAP errors.