diff --git a/packages/gamut/src/DataList/Controls/FilterControl.tsx b/packages/gamut/src/DataList/Controls/FilterControl.tsx index a30fbc5f5b..00de48c6e8 100644 --- a/packages/gamut/src/DataList/Controls/FilterControl.tsx +++ b/packages/gamut/src/DataList/Controls/FilterControl.tsx @@ -74,7 +74,7 @@ export const FilterControl: React.FC = ({ maxHeight={300} overflowY="auto" width="max-content" - variant="action" + variant="popover" > {[SELECT_ALL, ...options].map((opt) => { const { text, value } = formatOption(opt); diff --git a/packages/gamut/src/Menu/Menu.tsx b/packages/gamut/src/Menu/Menu.tsx index 6a5af90f9e..279b4d1920 100644 --- a/packages/gamut/src/Menu/Menu.tsx +++ b/packages/gamut/src/Menu/Menu.tsx @@ -8,7 +8,7 @@ export const Menu = forwardRef< Omit, 'root'> >( ( - { children, variant = 'select', spacing = 'normal', role, ...rest }, + { children, variant = 'popover', spacing = 'normal', role, ...rest }, ref ) => { const currentContext = useMenu({ variant, role, spacing }); diff --git a/packages/gamut/src/Menu/MenuContext.tsx b/packages/gamut/src/Menu/MenuContext.tsx index 8f91c4698f..c96c0c9ddf 100644 --- a/packages/gamut/src/Menu/MenuContext.tsx +++ b/packages/gamut/src/Menu/MenuContext.tsx @@ -4,14 +4,14 @@ import { ListProps } from '../List'; export interface MenuContextProps { spacing: 'normal' | 'condensed'; - variant: 'select' | 'navigation' | 'action'; + variant: 'popover' | 'fixed'; depth: number; role: ListProps['role']; } export const MenuContext = createContext({ spacing: 'normal', - variant: 'select', + variant: 'popover', depth: 0, role: undefined, }); diff --git a/packages/gamut/src/Menu/MenuItem.tsx b/packages/gamut/src/Menu/MenuItem.tsx index bae08593b7..ff00143f9b 100644 --- a/packages/gamut/src/Menu/MenuItem.tsx +++ b/packages/gamut/src/Menu/MenuItem.tsx @@ -15,9 +15,8 @@ const getListItemType = (href: boolean, onClick: boolean) => href ? 'link' : onClick ? 'button' : 'item'; const activePropnames = { - navigation: 'active-navlink', - action: 'active', - select: 'selected', + fixed: 'active-navlink', + popover: 'active', }; const currentItemText = { @@ -41,7 +40,7 @@ export const MenuItem = forwardRef< const computed = { ...props, ...rest, - variant: variant === 'select' ? 'select' : 'link', + variant: 'link', role: role === 'menu' ? 'menuitem' : undefined, [activeProp]: active, } as ListItemProps; diff --git a/packages/gamut/src/Menu/MenuSeparator.tsx b/packages/gamut/src/Menu/MenuSeparator.tsx index 4f257fa85c..e70d6bec52 100644 --- a/packages/gamut/src/Menu/MenuSeparator.tsx +++ b/packages/gamut/src/Menu/MenuSeparator.tsx @@ -2,16 +2,12 @@ import * as React from 'react'; import { Box, FlexBox } from '../Box'; import { FlexBoxProps } from '../Box/props'; -import { useMenuContext } from './MenuContext'; interface MenuSeperatorProps extends FlexBoxProps { children?: never; } export const MenuSeparator: React.FC = (props) => { - const { variant } = useMenuContext(); - if (variant !== 'action') return null; - return ( diff --git a/packages/gamut/src/Menu/__tests__/Menu.test.tsx b/packages/gamut/src/Menu/__tests__/Menu.test.tsx index 9a1c8ea706..13e7937105 100644 --- a/packages/gamut/src/Menu/__tests__/Menu.test.tsx +++ b/packages/gamut/src/Menu/__tests__/Menu.test.tsx @@ -39,17 +39,17 @@ describe('Menu', () => { screen.getByRole('button'); expect(screen.queryByRole('menuitem')).toBeNull(); }); - it('renders menu separators when the variant is action', () => { + it('renders menu separators when variant is popover', () => { renderView({ - variant: 'action', + variant: 'popover', children: , }); screen.getByRole('separator'); }); - it('renders deep menu separators while the parent variant is action', () => { + it('renders deep menu separators', () => { renderView({ - variant: 'action', + variant: 'popover', children: ( @@ -59,20 +59,13 @@ describe('Menu', () => { screen.getByRole('separator'); }); - it('does not render separators when the variant is select + navigation', () => { + it('renders menu separators when variant is fixed', () => { renderView({ - variant: 'select', + variant: 'fixed', children: , }); - expect(screen.queryByRole('separator')).toBeNull(); - - renderView({ - variant: 'navigation', - children: , - }); - - expect(screen.queryByRole('separator')).toBeNull(); + screen.getByRole('separator'); }); it('renders and icon only when specified', () => { renderView({ diff --git a/packages/gamut/src/Menu/elements.tsx b/packages/gamut/src/Menu/elements.tsx index 47006f5c49..655219b364 100644 --- a/packages/gamut/src/Menu/elements.tsx +++ b/packages/gamut/src/Menu/elements.tsx @@ -32,11 +32,12 @@ export interface ListProps extends ListStyleProps, StyleStateProps { /** How offset spacing should be */ spacing?: 'normal' | 'condensed'; /** Menu variants for specific use cases and styles */ - variant?: 'select' | 'navigation' | 'action'; + variant?: 'popover' | 'fixed'; /** is root menu */ root?: boolean; /** bordered */ as?: 'ul' | 'ol'; + showBorder?: boolean; } const StyledList = styled('ul', styledOptions<'ul'>())( @@ -54,6 +55,8 @@ const StyledList = styled('ul', styledOptions<'ul'>())( minWidth: 192, bg: 'background', p: 0, + }, + showBorder: { border: 1, borderRadius: 'sm', }, @@ -65,8 +68,16 @@ const StyledList = styled('ul', styledOptions<'ul'>())( export const List = forwardRef< HTMLUListElement, ComponentProps ->(({ context = true, m = 0, root = true, ...rest }, ref) => ( - +>(({ context = true, m = 0, root = true, variant, ...rest }, ref) => ( + )); const interactiveVariants = system.variant({ diff --git a/packages/styleguide/src/lib/Molecules/Menu/Menu.mdx b/packages/styleguide/src/lib/Molecules/Menu/Menu.mdx index da0adbf9b6..5bbcdbf378 100644 --- a/packages/styleguide/src/lib/Molecules/Menu/Menu.mdx +++ b/packages/styleguide/src/lib/Molecules/Menu/Menu.mdx @@ -1,7 +1,6 @@ -import { Column, LayoutGrid } from '@codecademy/gamut'; import { Canvas, Controls, Meta } from '@storybook/blocks'; -import { ComponentHeader } from '~styleguide/blocks'; +import { ComponentHeader, ImageWrapper, LinkTo } from '~styleguide/blocks'; import * as MenuStories from './Menu.stories'; @@ -24,7 +23,8 @@ export const parameters = { ## Usage -While most Menus are used as layout elements, Menus can also be used as popover elements within components like Selects. + +Use a Menu to organize and present a list of actions, options, or navigation links. ### Components @@ -34,71 +34,81 @@ While most Menus are used as layout elements, Menus can also be used as popover ### Best practices: -- A `Menu` should only have the `menu` role when it is a list of actions or functions a user can evoke. - - For example - a popover menu that allows the user to copy text, paste text, or reset a workspace. -- If your `Menu` is simply a list of links it should not have the `menu` role. -- Adding the role of `menu` to the menu component will programmatically set the correct roles on its `MenuItem`s and `MenuSeparator`s. +- The Popover menu has the `menu` role by default, which is intended for presenting actions or options. However, if it contains only navigation links, it should use the `