feat(HeaderMenu): include isCurrentPage props to HeaderMenu component #9785
Conversation
|
✔️ Deploy Preview for carbon-react-next ready! 🔨 Explore the source changes: acc4045 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/61708b6c0d259f00088e65df 😎 Browse the preview: https://deploy-preview-9785--carbon-react-next.netlify.app |
|
DCO Assistant Lite bot All contributors have signed the DCO. |
|
Related to issue #9784 |
set HeaderMenu as active when selected, contribute to the UX and accessibility
RianTavares
force-pushed
the
feature/headerMenu-active-style
branch
from
9df322f to
e3990a7
Compare
|
✔️ Deploy Preview for carbon-components-react ready! 🔨 Explore the source changes: 9df322f 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/6156505d37346d0007b9f05c 😎 Browse the preview: https://deploy-preview-9785--carbon-components-react.netlify.app |
|
✔️ Deploy Preview for carbon-elements ready! 🔨 Explore the source changes: 9df322f 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/6156505d058f060008e85baf 😎 Browse the preview: https://deploy-preview-9785--carbon-elements.netlify.app |
|
✔️ Deploy Preview for carbon-components-react ready! 🔨 Explore the source changes: acc4045 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/61708b6c23c55f0008136bed 😎 Browse the preview: https://deploy-preview-9785--carbon-components-react.netlify.app |
|
✔️ Deploy Preview for carbon-elements ready! 🔨 Explore the source changes: acc4045 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/61708b6c291a820007b4af17 😎 Browse the preview: https://deploy-preview-9785--carbon-elements.netlify.app |
|
I have read the DCO document and I hereby sign the DCO. |
|
recheck |
tay1orjones
requested review from
a team and
johnbister
and removed request for
a team
There was a problem hiding this comment.
@RianTavares Thanks for the PR! I think we should just modify the story to have Link 4 / Sub-link 2 have isCurrentPage so we can easily view the functionality in storybook
|
I updated the PR to include the change to the storybook and resolve the merge conflict. @johnbister you should be able to view these changes in the storybook deploy preview Header Base w/ Navigation story once it finishes redeploying. |
Closes #9784
Currently, the links of a HeaderNavigation component (HeaderMenuItem) already have this behavior of Convey menu items and their states setting border-bottom style. Example:
The point is that when creating a dropdown menu, I mean a HeaderMenu with HeaderMenuItems as children, like in "link 4" in the image above, there isn't a prop expected to set it as an active menu.
There is already a way to set the items inside the HeaderMenu as Active like below:
As described in one item of W3 WAI (Web Accessibility Tutorials) it's important to:
"Convey menu items and their states by using color and other styling options. For example, alter the shape of a menu item, or add an icon, in addition to changing its color when it is selected."
When the HeaderMenu is closed and a page inside is active the user has no signal of wich page is select looking at the menu.
Before
After
<img width="510" alt="4selected" src="https://user-images.githubusercontent.com/8935295/135545416-ef7e69ef-0801-4ef5-89e3-a793eaf5d409.png"
Changelog
New
carbon/packages/components/src/components/ui-shell/_header.scssand thecarbon/packages/styles/scss/components/ui-shell/header/_header.scssa new class (${prefix}--header__submenu--current) to set the active border style (same style as HeaderMenuItem).Changed
Testing / Reviewing
react/src/components/UIShell/UIShell-story.js