fix(UIShell): replace "javascript:void(0)" href value in HeaderMenu #4354
Conversation
Replace "javascript:void(0)" href value with "#".
ghost
requested review from
| @@ -72,7 +72,9 @@ class HeaderMenu extends React.Component { | |||
| /** | |||
| * Toggle the expanded state of the menu on click. | |||
| */ | |||
| handleOnClick = () => { | |||
| handleOnClick = e => { | |||
| e.preventDefault(); | |||
There was a problem hiding this comment.
Should we preventing the default here?
There was a problem hiding this comment.
I could go either way, but I feel that preventing the default would be expected, especially to preserve a cleaner url (i.e. no "#" appended).
|
Deploy preview for carbon-elements ready! Built with commit 31c40b6 |
|
cc @dakahn are there any UI Shell issues that we have around this pattern for the HeaderMenu in terms of accessibility? Under the hood it was using the menubar pattern but I think we're nixing that right? |
|
Deploy preview for carbon-components-react ready! Built with commit efab55a https://deploy-preview-4354--carbon-components-react.netlify.com |
dakahn
changed the title
|
Deploy preview for carbon-components-react ready! Built with commit 31c40b6 https://deploy-preview-4354--carbon-components-react.netlify.com |
|
Deploy preview for the-carbon-components ready! Built with commit 31c40b6 https://deploy-preview-4354--the-carbon-components.netlify.com |
|
@joshblack Yeah, we're going to be rethinking and tinkering with accessibility and the UIShell in a future work cycle. We're definitely not looking for a OS style menu keyboard interaction in the future though. |
Closes #4353
This PR replaces the "javascript:void(0)" href value with "#" in HeaderMenu to remove the React warning message.
Changelog
New
handleOnClickmethodChanged
Testing / Reviewing
In the React workspace, run
yarn startto start the storybook locally. Navigate to the UI Shell Header Base w/ Navigation story. Open the console; the React warning should not be emitted.