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

feat(@clayui/css): LPD-40160 Adds menubar-primary for CMS Product Menu variant #5898

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pat270
Copy link
Member

@pat270 pat270 commented Nov 27, 2024

https://liferay.atlassian.net/browse/LPD-40160

vertical-bar-primary

I'm sending this as a draft because we have a problem with the accordion headers. They can only be button and in some of the headers we have nested plus buttons. Having a button nested in a button violates:

Phrasing content, but there must be no interactive content descendant and no descendant with the tabindex attribute specified.

from the w3c spec. I need some help figuring out which component we should place <div role="button"> to make our markup valid.

@ethib137
Copy link
Member

@marcoscv-work any recommendations for this one?

@marcoscv-work
Copy link
Member

Hey guys @ethib137 @pat270 if we use role="menuitem" it should work fine.
An example here: https://codepen.io/marcoscv/full/NWQbJbx
You can change the for a button and validate with Axe or Wave. it should be valid.

Comment on lines +279 to +300
<Nav aria-orientation="vertical" nested role="menubar">
<VerticalNavContext.Provider
value={{
activeKey:
active && collection.hasItem(active)
? active
: hasDepthActive
? null
: undefined,
ariaCurrent: ariaCurrent ? 'page' : null,
childrenRoot: childrenRootRef,
close,
expandedKeys,
firstKey: collection.getFirstItem().key,
open,
spritemap,
toggle,
}}
>
<Collection<T> collection={collection} />
</VerticalNavContext.Provider>
</Nav>
Copy link
Member

Choose a reason for hiding this comment

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

You could store this in a constant to avoid repeating the code and reuse it below.

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

Successfully merging this pull request may close these issues.

4 participants