diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 4d384ca84c66..a826c3907cce 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -4608,15 +4608,7 @@ Map { "menuAlignment": Object { "type": "string", }, - "mode": Object { - "args": Array [ - Array [ - "full", - "basic", - ], - ], - "type": "oneOf", - }, + "mode": [Function], "onClose": Object { "type": "func", }, diff --git a/packages/react/src/components/ComboButton/ComboButton-test.js b/packages/react/src/components/ComboButton/ComboButton-test.js index 9bb5a65715e2..09734a343688 100644 --- a/packages/react/src/components/ComboButton/ComboButton-test.js +++ b/packages/react/src/components/ComboButton/ComboButton-test.js @@ -186,56 +186,6 @@ describe('ComboButton', () => { ).toHaveTextContent(/^Additional action$/); }); - it('warns when MenuItemSelectable is used in children', async () => { - const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - - render( - - - - ); - - await userEvent.click(screen.getAllByRole('button')[1]); - - expect(spy).toHaveBeenCalled(); - spy.mockRestore(); - }); - - it('warns when MenuItemRadioGoup is used in children', async () => { - const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - - render( - - - - ); - - await userEvent.click(screen.getAllByRole('button')[1]); - - expect(spy).toHaveBeenCalled(); - spy.mockRestore(); - }); - - it('warns when a nested Menu is used in children', async () => { - const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - - render( - - - - - - ); - - await userEvent.click(screen.getAllByRole('button')[1]); - - expect(spy).toHaveBeenCalled(); - spy.mockRestore(); - }); - it('supports ellipsis in ComboButton by checking the className', async () => { render( diff --git a/packages/react/src/components/ComboButton/index.tsx b/packages/react/src/components/ComboButton/index.tsx index 2972cc99d9b0..3c9c77bbcaac 100644 --- a/packages/react/src/components/ComboButton/index.tsx +++ b/packages/react/src/components/ComboButton/index.tsx @@ -235,7 +235,6 @@ const ComboButton = React.forwardRef( ref={refs.setFloating} id={id} label={t('carbon.combo-button.additional-actions')} - mode="basic" size={size} open={open} onClose={handleClose}> diff --git a/packages/react/src/components/ContextMenu/useContextMenu.tsx b/packages/react/src/components/ContextMenu/useContextMenu.tsx index 8fa72b968a04..a53a600ff778 100644 --- a/packages/react/src/components/ContextMenu/useContextMenu.tsx +++ b/packages/react/src/components/ContextMenu/useContextMenu.tsx @@ -14,7 +14,6 @@ interface ContextMenuProps { x: number; y: number; onClose: () => void; - mode: string; } /** @@ -62,7 +61,6 @@ function useContextMenu(trigger: TriggerType = document): ContextMenuProps { x: position[0], y: position[1], onClose, - mode: 'full', }; } diff --git a/packages/react/src/components/Menu/Menu-test.js b/packages/react/src/components/Menu/Menu-test.js index c1a2d4979fa1..d1a1cc85e504 100644 --- a/packages/react/src/components/Menu/Menu-test.js +++ b/packages/react/src/components/Menu/Menu-test.js @@ -83,21 +83,6 @@ describe('Menu', () => { expect(document.querySelector('.custom-class')).toBeInTheDocument(); document.body.removeChild(el); }); - - it('warns about nested menus in basic mode', async () => { - const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - - render( - - - - - - ); - await waitForPosition(); - expect(spy).toHaveBeenCalled(); - spy.mockRestore(); - }); }); describe('Submenu behavior', () => { @@ -224,35 +209,6 @@ describe('MenuItem', () => { expect(screen.getByText('item')).toBeInTheDocument(); }); - - it('warns about MenuItemSelectable in basic mode', () => { - const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - - render( - - - - ); - - expect(spy).toHaveBeenCalled(); - spy.mockRestore(); - }); - - it('warns about MenuItemRadioGroup in basic mode', () => { - const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - - render( - - - - ); - - expect(spy).toHaveBeenCalled(); - spy.mockRestore(); - }); }); it('should call MenuItemRadioGroup onChange once', async () => { diff --git a/packages/react/src/components/Menu/Menu.stories.js b/packages/react/src/components/Menu/Menu.stories.js index 80a9ca1bf264..8bfe485a97a2 100644 --- a/packages/react/src/components/Menu/Menu.stories.js +++ b/packages/react/src/components/Menu/Menu.stories.js @@ -8,6 +8,16 @@ import React from 'react'; import { action } from '@storybook/addon-actions'; +import { + Copy, + Cut, + FolderShared, + Paste, + TextBold, + TextItalic, + TrashCan, +} from '@carbon/icons-react'; + import { Menu, MenuItem, @@ -49,7 +59,7 @@ export const Default = (args) => { return ( - + { /> - - - + + + + - { shortcut="⌫" kind="danger" onClick={itemOnClick} + renderIcon={TrashCan} /> ); diff --git a/packages/react/src/components/Menu/Menu.tsx b/packages/react/src/components/Menu/Menu.tsx index b8485091e7c0..a90f2da2b899 100644 --- a/packages/react/src/components/Menu/Menu.tsx +++ b/packages/react/src/components/Menu/Menu.tsx @@ -23,7 +23,7 @@ import { createPortal } from 'react-dom'; import { keys, match } from '../../internal/keyboard'; import { useMergedRefs } from '../../internal/useMergedRefs'; import { usePrefix } from '../../internal/usePrefix'; -import { warning } from '../../internal/warning.js'; +import deprecate from '../../prop-types/deprecate'; import { MenuContext, menuReducer } from './MenuContext'; import { useLayoutDirection } from '../LayoutDirection'; @@ -57,6 +57,7 @@ interface MenuProps extends React.HTMLAttributes { menuAlignment?: string; /** + * @deprecated Menus now always support both icons as well as selectable items and nesting. * The mode of this menu. Defaults to full. * `full` supports nesting and selectable menu items, but no icons. * `basic` supports icons but no nesting or selectable menu items. @@ -110,7 +111,7 @@ const Menu = forwardRef(function Menu( containerRef, label, menuAlignment, - mode = 'full', + mode, onClose, onOpen, open, @@ -132,19 +133,11 @@ const Menu = forwardRef(function Menu( const isRoot = context.state.isRoot; - if (context.state.mode === 'basic' && !isRoot) { - warning( - false, - 'Nested menus are not supported when the menu is in "basic" mode.' - ); - } - const menuSize = isRoot ? size : context.state.size; const [childState, childDispatch] = useReducer(menuReducer, { ...context.state, isRoot: false, - mode, size, requestCloseRoot: isRoot ? handleClose : context.state.requestCloseRoot, }); @@ -424,6 +417,8 @@ const Menu = forwardRef(function Menu( [`${prefix}--menu--shown`]: (open && !legacyAutoalign) || (position[0] >= 0 && position[1] >= 0), [`${prefix}--menu--with-icons`]: childContext.state.hasIcons, + [`${prefix}--menu--with-selectable-items`]: + childContext.state.hasSelectableItems, [`${prefix}--autoalign`]: !legacyAutoalign, } ); @@ -474,13 +469,17 @@ Menu.propTypes = { menuAlignment: PropTypes.string, /** + * **Deprecated**: Menus now always support both icons as well as selectable items and nesting. * The mode of this menu. Defaults to full. * `full` supports nesting and selectable menu items, but no icons. * `basic` supports icons but no nesting or selectable menu items. * * **This prop is not intended for use and will be set by the respective implementation (like useContextMenu, MenuButton, and ComboButton).** */ - mode: PropTypes.oneOf(['full', 'basic']), + mode: deprecate( + PropTypes.oneOf(['full', 'basic']), + 'Menus now always support both icons as well as selectable items and nesting.' + ), /** * Provide an optional function to be called when the Menu should be closed. diff --git a/packages/react/src/components/Menu/MenuContext.ts b/packages/react/src/components/Menu/MenuContext.ts index 3731beb1fce2..9b7862fd93f4 100644 --- a/packages/react/src/components/Menu/MenuContext.ts +++ b/packages/react/src/components/Menu/MenuContext.ts @@ -8,14 +8,14 @@ import { createContext, KeyboardEvent, RefObject } from 'react'; type ActionType = { - type: 'enableIcons' | 'registerItem'; + type: 'enableIcons' | 'enableSelectableItems' | 'registerItem'; payload: any; }; type StateType = { isRoot: boolean; - mode: 'full' | 'basic'; hasIcons: boolean; + hasSelectableItems: boolean; size: 'xs' | 'sm' | 'md' | 'lg' | null; items: any[]; requestCloseRoot: (e: Pick, 'type'>) => void; @@ -23,8 +23,8 @@ type StateType = { const menuDefaultState: StateType = { isRoot: true, - mode: 'full', hasIcons: false, + hasSelectableItems: false, size: null, items: [], requestCloseRoot: () => {}, @@ -37,6 +37,11 @@ function menuReducer(state: StateType, action: ActionType) { ...state, hasIcons: true, }; + case 'enableSelectableItems': + return { + ...state, + hasSelectableItems: true, + }; case 'registerItem': return { ...state, @@ -48,7 +53,7 @@ function menuReducer(state: StateType, action: ActionType) { } type DispatchFuncProps = { - type: 'registerItem' | 'enableIcons'; + type: ActionType['type']; payload: { ref: RefObject; disabled: boolean; diff --git a/packages/react/src/components/Menu/MenuItem.tsx b/packages/react/src/components/Menu/MenuItem.tsx index b709a8f6633c..dc417956e8f2 100644 --- a/packages/react/src/components/Menu/MenuItem.tsx +++ b/packages/react/src/components/Menu/MenuItem.tsx @@ -77,7 +77,7 @@ export interface MenuItemProps extends LiHTMLAttributes { ) => void; /** - * Only applicable if the parent menu is in `basic` mode. Sets the menu item's icon. + * Sets the menu item's icon. */ renderIcon?: FC; @@ -216,17 +216,12 @@ export const MenuItem = forwardRef( } }, [direction]); - const iconsAllowed = - context.state.mode === 'basic' || - rest.role === 'menuitemcheckbox' || - rest.role === 'menuitemradio'; - useEffect(() => { - if (iconsAllowed && IconElement && !context.state.hasIcons) { + if (IconElement && !context.state.hasIcons) { // @ts-ignore - TODO: Should we be passing payload? context.dispatch({ type: 'enableIcons' }); } - }, [iconsAllowed, IconElement, context.state.hasIcons, context]); + }, [IconElement, context.state.hasIcons, context]); useEffect(() => { Object.keys(floatingStyles).forEach((style) => { @@ -253,8 +248,11 @@ export const MenuItem = forwardRef( onClick={handleClick} onKeyDown={handleKeyDown} {...getReferenceProps()}> +
+ {rest['aria-checked'] && } +
- {iconsAllowed && IconElement && } + {IconElement && }
{ - if (!context.state.hasIcons) { + if (!context.state.hasSelectableItems) { // @ts-ignore - TODO: Should we be passing payload? - context.dispatch({ type: 'enableIcons' }); + context.dispatch({ type: 'enableSelectableItems' }); } - }, [context.state.hasIcons, context]); + }, [context.state.hasSelectableItems, context]); const classNames = cx(className, `${prefix}--menu-item-selectable--selected`); @@ -396,7 +387,6 @@ export const MenuItemSelectable = forwardRef< className={classNames} role="menuitemcheckbox" aria-checked={checked} - renderIcon={checked ? Checkmark : undefined} onClick={handleClick} /> ); @@ -538,13 +528,6 @@ export const MenuItemRadioGroup = forwardRef(function MenuItemRadioGroup( const prefix = usePrefix(); const context = useContext(MenuContext); - if (context.state.mode === 'basic') { - warning( - false, - 'MenuItemRadioGroup is not supported when the menu is in "basic" mode.' - ); - } - const [selection, setSelection] = useControllableState({ value: selectedItem, onChange, @@ -556,11 +539,11 @@ export const MenuItemRadioGroup = forwardRef(function MenuItemRadioGroup( } useEffect(() => { - if (!context.state.hasIcons) { + if (!context.state.hasSelectableItems) { // @ts-ignore - TODO: Should we be passing payload? - context.dispatch({ type: 'enableIcons' }); + context.dispatch({ type: 'enableSelectableItems' }); } - }, [context.state.hasIcons, context]); + }, [context.state.hasSelectableItems, context]); const classNames = cx(className, `${prefix}--menu-item-radio-group`); @@ -573,7 +556,6 @@ export const MenuItemRadioGroup = forwardRef(function MenuItemRadioGroup( label={itemToString(item)} role="menuitemradio" aria-checked={item === selection} - renderIcon={item === selection ? Checkmark : undefined} onClick={(e) => { handleClick(item, e); }} diff --git a/packages/react/src/components/MenuButton/MenuButton-test.js b/packages/react/src/components/MenuButton/MenuButton-test.js index e3b7b7ddf78e..bbe18e47b4bc 100644 --- a/packages/react/src/components/MenuButton/MenuButton-test.js +++ b/packages/react/src/components/MenuButton/MenuButton-test.js @@ -9,7 +9,7 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; -import { MenuItem, MenuItemSelectable, MenuItemRadioGroup } from '../Menu'; +import { MenuItem } from '../Menu'; import { MenuButton } from './'; @@ -115,57 +115,8 @@ describe('MenuButton', () => { expect(screen.getByRole('menu')).toBeInTheDocument(); expect(screen.getByRole('menuitem')).toHaveTextContent(/^Action$/); }); - - it('warns when MenuItemSelectable is used in children', async () => { - const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - - render( - - - - ); - - await userEvent.click(screen.getByRole('button')); - - expect(spy).toHaveBeenCalled(); - spy.mockRestore(); - }); - - it('warns when MenuItemRadioGoup is used in children', async () => { - const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - - render( - - - - ); - - await userEvent.click(screen.getByRole('button')); - - expect(spy).toHaveBeenCalled(); - spy.mockRestore(); - }); - - it('warns when a nested Menu is used in children', async () => { - const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - - render( - - - - - - ); - - await userEvent.click(screen.getByRole('button')); - - expect(spy).toHaveBeenCalled(); - spy.mockRestore(); - }); }); + describe('supports props.menuAlignment', () => { const alignments = [ 'top', diff --git a/packages/react/src/components/MenuButton/MenuButton.stories.js b/packages/react/src/components/MenuButton/MenuButton.stories.js index 18ffd5b36900..9b59604a0723 100644 --- a/packages/react/src/components/MenuButton/MenuButton.stories.js +++ b/packages/react/src/components/MenuButton/MenuButton.stories.js @@ -121,6 +121,22 @@ export const WithIcons = (args) => ( WithIcons.argTypes = { ...sharedArgTypes }; +export const WithNestedMenu = (args) => ( + + + + + + + + + + + +); + +WithNestedMenu.argTypes = { ...sharedArgTypes }; + export const WithMenuAlignment = () => ( <>
diff --git a/packages/react/src/components/MenuButton/index.tsx b/packages/react/src/components/MenuButton/index.tsx index 2ab8748a0786..8fc880fdca62 100644 --- a/packages/react/src/components/MenuButton/index.tsx +++ b/packages/react/src/components/MenuButton/index.tsx @@ -130,7 +130,6 @@ const MenuButton = forwardRef( }) ); } - const { refs, floatingStyles, placement, middlewareData } = useFloating({ placement: menuAlignment, @@ -140,6 +139,16 @@ const MenuButton = forwardRef( // https://floating-ui.com/docs/misc#clipping strategy: 'fixed', + // Submenus are using a fixed position to break out of the parent menu's + // box avoiding clipping while allowing for vertical scroll. When an + // element is using transform it establishes a new containing block + // block for all of its descendants. Therefore, its padding box will be + // used for fixed-positioned descendants. This would cause the submenu + // to be clipped by its parent menu. + // Reference: https://www.w3.org/TR/2019/CR-css-transforms-1-20190214/#current-transformation-matrix-computation + // Reference: https://github.com/carbon-design-system/carbon/pull/18153#issuecomment-2498548835 + transform: false, + // Middleware order matters, arrow should be last middleware: middlewares, whileElementsMounted: autoUpdate, @@ -155,7 +164,16 @@ const MenuButton = forwardRef( useLayoutEffect(() => { Object.keys(floatingStyles).forEach((style) => { if (refs.floating.current) { - refs.floating.current.style[style] = floatingStyles[style]; + let value = floatingStyles[style]; + + if ( + ['top', 'right', 'bottom', 'left'].includes(style) && + Number(value) + ) { + value += 'px'; + } + + refs.floating.current.style[style] = value; } }); }, [floatingStyles, refs.floating, middlewareData, placement, open]); @@ -206,7 +224,6 @@ const MenuButton = forwardRef( id={id} legacyAutoalign={false} label={label} - mode="basic" size={size} open={open} onClose={handleClose} diff --git a/packages/styles/scss/components/menu-button/_menu-button.scss b/packages/styles/scss/components/menu-button/_menu-button.scss index acced1261605..0ef33b85d648 100644 --- a/packages/styles/scss/components/menu-button/_menu-button.scss +++ b/packages/styles/scss/components/menu-button/_menu-button.scss @@ -30,19 +30,4 @@ .#{$prefix}--menu-button__trigger--open svg { transform: rotate(180deg); } - - // Menu alignment classes - $popover-offset: custom-property.get-var('popover-offset', 3rem); - - .#{$prefix}--menu-button__top { - transform: translate(0, calc(-100% - $popover-offset)); - } - - .#{$prefix}--menu-button__top-start { - transform: translate(0, calc(-100% - $popover-offset)); - } - - .#{$prefix}--menu-button__top-end { - transform: translate(0, calc(-100% - $popover-offset)); - } } diff --git a/packages/styles/scss/components/menu/_menu.scss b/packages/styles/scss/components/menu/_menu.scss index d294a13358b5..17d3a79a9e21 100644 --- a/packages/styles/scss/components/menu/_menu.scss +++ b/packages/styles/scss/components/menu/_menu.scss @@ -97,14 +97,6 @@ } } - .#{$prefix}--menu-item__icon { - display: none; - } - - .#{$prefix}--menu--with-icons .#{$prefix}--menu-item__icon { - display: flex; - } - .#{$prefix}--menu-item__label { overflow: hidden; text-overflow: ellipsis; @@ -120,18 +112,74 @@ @include component-reset.reset; } + .#{$prefix}--menu-item__icon, + .#{$prefix}--menu-item__selection-icon { + display: none; + } + .#{$prefix}--menu--with-icons > .#{$prefix}--menu-item, .#{$prefix}--menu--with-icons > .#{$prefix}--menu-item-group > ul > .#{$prefix}--menu-item, .#{$prefix}--menu--with-icons + > .#{$prefix}--menu-item-radio-group + > ul + > .#{$prefix}--menu-item, + .#{$prefix}--menu--with-selectable-items > .#{$prefix}--menu-item, + .#{$prefix}--menu--with-selectable-items + > .#{$prefix}--menu-item-group + > ul + > .#{$prefix}--menu-item, + .#{$prefix}--menu--with-selectable-items > .#{$prefix}--menu-item-radio-group > ul > .#{$prefix}--menu-item { grid-template-columns: 1rem 1fr max-content; } + .#{$prefix}--menu--with-icons + > .#{$prefix}--menu-item + > .#{$prefix}--menu-item__icon, + .#{$prefix}--menu--with-icons + > .#{$prefix}--menu-item-group + > ul + > .#{$prefix}--menu-item + > .#{$prefix}--menu-item__icon, + .#{$prefix}--menu--with-icons + > .#{$prefix}--menu-item-radio-group + > ul + > .#{$prefix}--menu-item + > .#{$prefix}--menu-item__icon, + .#{$prefix}--menu--with-selectable-items + > .#{$prefix}--menu-item + > .#{$prefix}--menu-item__selection-icon, + .#{$prefix}--menu--with-selectable-items + > .#{$prefix}--menu-item-group + > ul + > .#{$prefix}--menu-item + > .#{$prefix}--menu-item__selection-icon, + .#{$prefix}--menu--with-selectable-items + > .#{$prefix}--menu-item-radio-group + > ul + > .#{$prefix}--menu-item + > .#{$prefix}--menu-item__selection-icon { + display: flex; + } + + .#{$prefix}--menu--with-icons.#{$prefix}--menu--with-selectable-items + > .#{$prefix}--menu-item, + .#{$prefix}--menu--with-icons.#{$prefix}--menu--with-selectable-items + > .#{$prefix}--menu-item-group + > ul + > .#{$prefix}--menu-item, + .#{$prefix}--menu--with-icons.#{$prefix}--menu--with-selectable-items + > .#{$prefix}--menu-item-radio-group + > ul + > .#{$prefix}--menu-item { + grid-template-columns: 1rem 1rem 1fr max-content; + } + .#{$prefix}--menu-item--disabled { color: $text-disabled; cursor: not-allowed;