feat(Menu)!: updates menu variants and usage guidance #2993
Conversation
|
View your CI Pipeline Execution ↗ for commit ac0a472. ☁️ Nx Cloud last updated this comment at |
…y/gamut into ajr-update-menu-variants-usage
aresnik11
changed the title
| @@ -37,7 +37,7 @@ Use an alert to display an important, succinct message with actions for users to | |||
| - Form field validation– for inline form errors or validation messages, use inline messaging within the form fields instead of an alert. | |||
|
|
|||
| ## Anatomy | |||
| <ImageWrapper src="./alertAnatomy.png" alt="Alert Anatomy diagram"/> | |||
| <ImageWrapper src="./molecules/alertAnatomy.png" alt="Alert Anatomy diagram"/> | |||
|
|
||
| ## Popover menus | ||
|
|
||
| You can create popover menus by utilizing our **PopoverContainer**. Once you have your base positioning, you just need to adjust the `y` axis by `48` for the `normal` spacing or `32` for the `condensed` spacing for each of the `MenuItem`s. You may also need to change the `alignment` of the `PopoverContainer` to ensure correct positioning. | ||
| You can create popover menus by utilizing our <LinkTo id="Atoms/PopoverContainer">PopoverContainer</LinkTo>. Once you have your base positioning, you just need to adjust the y axis by 48 for the normal spacing or 32 for the condensed spacing for each of the MenuItems. You may also need to change the alignment of the PopoverContainer to ensure correct positioning. |
There was a problem hiding this comment.
Another potential question, for a future style guide?
When to use markdown styling? E.g. the old story added backticks to the attributes and the component, but this new copy in Figma strips it out.
| <Canvas of={MenuStories.Default} /> | ||
| A general purpose menu type that can be used for actions or navigation. | ||
| </Column> | ||
| Use the Popover menu to present a temporary surface for actions, options, or links, ensuring access only when needed while keeping the interface clean and focused. The Popover menu has the menu role by default, but if it contains only a list of navigation links, it should use the `<nav>` element, which implicitly includes the navigation role. |
There was a problem hiding this comment.
for the 2nd sentence:
The Popover menu has the menu role by default, but if it contains only a list of navigation links, it should use the
<nav>element, which implicitly includes the navigation role.
The sentence could be more clear about what "it should use the <nav> element ..."
Would suggest a rewrite of:
"The Popover menu has the menu role by default. But, if the menu contains only a list of navigation links, it should be nested in a <nav> element, which implicitly includes the navigation role."
And then also provide a story that showcases how this would look like.
There was a problem hiding this comment.
maybe we should just add the menu + nav role behavior as default for each of the types. i'll take a look at mono and see if there's any example of Menus that are neither a navigation list or a menu list
There was a problem hiding this comment.
updated the copy. i'll defer to you both if you want to ship this when i'm out as is or make these additional updates
| @@ -187,8 +224,3 @@ export const PopoverMenuExample: React.FC = () => { | |||
| </FlexBox> | |||
| ); | |||
| }; | |||
|
|
|||
| export const Popover: Story = { | |||
There was a problem hiding this comment.
TIL you can set a canvas with just a component!
…y/gamut into ajr-update-menu-variants-usage
📬Published Alpha Packages:@codecademy/[email protected] |
|
🚀 Styleguide deploy preview ready! |
Overview
Updates Menu variants and usage guidance according to figma. The action variant is now popover, the navigation variant is now fixed and the border has been removed, and the select variant is no more.
PR Checklist
Testing Instructions
In storybook preview confirm you see new usage guidance and variants for Menu.
See monolith and mono PRs for specific testing instructions for Menus used in action.
PR Links and Envs