Menu: more granular sub-components (WordPress#67422)

* MenuItem: add render and store props * Extract sub-components: popover, trigger button, submenu trigger item * Unit tests * CHANGELOG * Add more memory to node on CI * Refactor block bindings panel menu (WordPress#67633) Co-authored-by: ciampo <[email protected]> * Storybook (WordPress#67632) * Refactor dataviews item actions menu (WordPress#67636) * Refactor dataviews view config menu (WordPress#67637) * Refactor global styles shadows edit panel menu (WordPress#67641) * Refactor global styles font size menus (WordPress#67642) * Refactor "Add filter" dataviews menu (WordPress#67634) * Menu granular subcomponents: Refactor dataviews list layout actions menu (WordPress#67639) Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: oandregal <[email protected]> * Menu granular subcomponents: Refactor dataviews table layout header menu (WordPress#67640) Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: oandregal <[email protected]> * Menu granular subcomponents: Refactor post actions menu (WordPress#67645) Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]> * Better comments for submenu trigger store * Typo * Remove unnecessary MenuSubmenuTriggerItemProps type * Don't break the rules of hooks 🪝 * Move CHANGELOG entry to unreleased section * Add explicit MenuProps to improve TS performance * Remove node memory settings --------- Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: mirka <[email protected]> Co-authored-by: oandregal <[email protected]>
MaxMood96 · Dec 16, 2024 · 2cdd37d · 2cdd37d
1 parent 2d7f2d8
commit 2cdd37d
Show file tree

Hide file tree

Showing 20 changed files with 1,384 additions and 997 deletions.
diff --git a/packages/block-editor/src/hooks/block-bindings.js b/packages/block-editor/src/hooks/block-bindings.js
@@ -51,7 +51,7 @@ const useToolsPanelDropdownMenuProps = () => {
 		: {};
 };
 
-function BlockBindingsPanelDropdown( { fieldsList, attribute, binding } ) {
+function BlockBindingsPanelMenuContent( { fieldsList, attribute, binding } ) {
 	const { clientId } = useBlockEditContext();
 	const registeredSources = getBlockBindingsSources();
 	const { updateBlockBindings } = useBlockBindingsUtils();
@@ -179,22 +179,21 @@ function EditableBlockBindingsPanelItems( {
 							placement={
 								isMobile ? 'bottom-start' : 'left-start'
 							}
-							gutter={ isMobile ? 8 : 36 }
-							trigger={
-								<Item>
-									<BlockBindingsAttribute
-										attribute={ attribute }
-										binding={ binding }
-										fieldsList={ fieldsList }
-									/>
-								</Item>
-							}
 						>
-							<BlockBindingsPanelDropdown
-								fieldsList={ fieldsList }
-								attribute={ attribute }
-								binding={ binding }
-							/>
+							<Menu.TriggerButton render={ <Item /> }>
+								<BlockBindingsAttribute
+									attribute={ attribute }
+									binding={ binding }
+									fieldsList={ fieldsList }
+								/>
+							</Menu.TriggerButton>
+							<Menu.Popover gutter={ isMobile ? 8 : 36 }>
+								<BlockBindingsPanelMenuContent
+									fieldsList={ fieldsList }
+									attribute={ attribute }
+									binding={ binding }
+								/>
+							</Menu.Popover>
 						</Menu>
 					</ToolsPanelItem>
 				);

diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -19,6 +19,7 @@
 ### Experimental
 
 -   Add new `Badge` component ([#66555](https://github.com/WordPress/gutenberg/pull/66555)).
+-   `Menu`: refactor to more granular sub-components ([#67422](https://github.com/WordPress/gutenberg/pull/67422)).
 
 ### Internal
 

diff --git a/packages/components/src/menu/index.tsx b/packages/components/src/menu/index.tsx
@@ -6,23 +6,14 @@ import * as Ariakit from '@ariakit/react';
 /**
  * WordPress dependencies
  */
-import {
-	useContext,
-	useMemo,
-	cloneElement,
-	isValidElement,
-	useCallback,
-} from '@wordpress/element';
-import { isRTL } from '@wordpress/i18n';
-import { chevronRightSmall } from '@wordpress/icons';
+import { useContext, useMemo } from '@wordpress/element';
+import { isRTL as isRTLFn } from '@wordpress/i18n';
 
 /**
  * Internal dependencies
  */
-import { useContextSystem, contextConnect } from '../context';
-import type { WordPressComponentProps } from '../context';
+import { useContextSystem, contextConnectWithoutRef } from '../context';
 import type { MenuContext as MenuContextType, MenuProps } from './types';
-import * as Styled from './styles';
 import { MenuContext } from './context';
 import { MenuItem } from './item';
 import { MenuCheckboxItem } from './checkbox-item';
@@ -32,49 +23,36 @@ import { MenuGroupLabel } from './group-label';
 import { MenuSeparator } from './separator';
 import { MenuItemLabel } from './item-label';
 import { MenuItemHelpText } from './item-help-text';
+import { MenuTriggerButton } from './trigger-button';
+import { MenuSubmenuTriggerItem } from './submenu-trigger-item';
+import { MenuPopover } from './popover';
 
-const UnconnectedMenu = (
-	props: WordPressComponentProps< MenuProps, 'div', false >,
-	ref: React.ForwardedRef< HTMLDivElement >
-) => {
+const UnconnectedMenu = ( props: MenuProps ) => {
 	const {
-		// Store props
-		open,
+		children,
 		defaultOpen = false,
+		open,
 		onOpenChange,
 		placement,
 
-		// Menu trigger props
-		trigger,
-
-		// Menu props
-		gutter,
-		children,
-		shift,
-		modal = true,
-
 		// From internal components context
 		variant,
-
-		// Rest
-		...otherProps
-	} = useContextSystem< typeof props & Pick< MenuContextType, 'variant' > >(
-		props,
-		'Menu'
-	);
+	} = useContextSystem<
+		// @ts-expect-error TODO: missing 'className' in MenuProps
+		typeof props & Pick< MenuContextType, 'variant' >
+	>( props, 'Menu' );
 
 	const parentContext = useContext( MenuContext );
 
-	const computedDirection = isRTL() ? 'rtl' : 'ltr';
+	const rtl = isRTLFn();
 
 	// If an explicit value for the `placement` prop is not passed,
 	// apply a default placement of `bottom-start` for the root menu popover,
 	// and of `right-start` for nested menu popovers.
 	let computedPlacement =
-		props.placement ??
-		( parentContext?.store ? 'right-start' : 'bottom-start' );
+		placement ?? ( parentContext?.store ? 'right-start' : 'bottom-start' );
 	// Swap left/right in case of RTL direction
-	if ( computedDirection === 'rtl' ) {
+	if ( rtl ) {
 		if ( /right/.test( computedPlacement ) ) {
 			computedPlacement = computedPlacement.replace(
 				'right',
@@ -97,142 +75,61 @@ const UnconnectedMenu = (
 		setOpen( willBeOpen ) {
 			onOpenChange?.( willBeOpen );
 		},
-		rtl: computedDirection === 'rtl',
+		rtl,
 	} );
 
 	const contextValue = useMemo(
 		() => ( { store: menuStore, variant } ),
 		[ menuStore, variant ]
 	);
 
-	// Extract the side from the applied placement — useful for animations.
-	// Using `currentPlacement` instead of `placement` to make sure that we
-	// use the final computed placement (including "flips" etc).
-	const appliedPlacementSide = Ariakit.useStoreState(
-		menuStore,
-		'currentPlacement'
-	).split( '-' )[ 0 ];
-
-	if (
-		menuStore.parent &&
-		! ( isValidElement( trigger ) && MenuItem === trigger.type )
-	) {
-		// eslint-disable-next-line no-console
-		console.warn(
-			'For nested Menus, the `trigger` should always be a `MenuItem`.'
-		);
-	}
-
-	const hideOnEscape = useCallback(
-		( event: React.KeyboardEvent< Element > ) => {
-			// Pressing Escape can cause unexpected consequences (ie. exiting
-			// full screen mode on MacOs, close parent modals...).
-			event.preventDefault();
-			// Returning `true` causes the menu to hide.
-			return true;
-		},
-		[]
-	);
-
-	const wrapperProps = useMemo(
-		() => ( {
-			dir: computedDirection,
-			style: {
-				direction:
-					computedDirection as React.CSSProperties[ 'direction' ],
-			},
-		} ),
-		[ computedDirection ]
-	);
-
 	return (
-		<>
-			{ /* Menu trigger */ }
-			<Ariakit.MenuButton
-				ref={ ref }
-				store={ menuStore }
-				render={
-					menuStore.parent
-						? cloneElement( trigger, {
-								// Add submenu arrow, unless a `suffix` is explicitly specified
-								suffix: (
-									<>
-										{ trigger.props.suffix }
-										<Styled.SubmenuChevronIcon
-											aria-hidden="true"
-											icon={ chevronRightSmall }
-											size={ 24 }
-											preserveAspectRatio="xMidYMid slice"
-										/>
-									</>
-								),
-						  } )
-						: trigger
-				}
-			/>
-
-			{ /* Menu popover */ }
-			<Ariakit.Menu
-				{ ...otherProps }
-				modal={ modal }
-				store={ menuStore }
-				// Root menu has an 8px distance from its trigger,
-				// otherwise 0 (which causes the submenu to slightly overlap)
-				gutter={ gutter ?? ( menuStore.parent ? 0 : 8 ) }
-				// Align nested menu by the same (but opposite) amount
-				// as the menu container's padding.
-				shift={ shift ?? ( menuStore.parent ? -4 : 0 ) }
-				hideOnHoverOutside={ false }
-				data-side={ appliedPlacementSide }
-				wrapperProps={ wrapperProps }
-				hideOnEscape={ hideOnEscape }
-				unmountOnHide
-				render={ ( renderProps ) => (
-					// Two wrappers are needed for the entry animation, where the menu
-					// container scales with a different factor than its contents.
-					// The {...renderProps} are passed to the inner wrapper, so that the
-					// menu element is the direct parent of the menu item elements.
-					<Styled.MenuPopoverOuterWrapper variant={ variant }>
-						<Styled.MenuPopoverInnerWrapper { ...renderProps } />
-					</Styled.MenuPopoverOuterWrapper>
-				) }
-			>
-				<MenuContext.Provider value={ contextValue }>
-					{ children }
-				</MenuContext.Provider>
-			</Ariakit.Menu>
-		</>
+		<MenuContext.Provider value={ contextValue }>
+			{ children }
+		</MenuContext.Provider>
 	);
 };
 
-export const Menu = Object.assign( contextConnect( UnconnectedMenu, 'Menu' ), {
-	Context: Object.assign( MenuContext, {
-		displayName: 'Menu.Context',
-	} ),
-	Item: Object.assign( MenuItem, {
-		displayName: 'Menu.Item',
-	} ),
-	RadioItem: Object.assign( MenuRadioItem, {
-		displayName: 'Menu.RadioItem',
-	} ),
-	CheckboxItem: Object.assign( MenuCheckboxItem, {
-		displayName: 'Menu.CheckboxItem',
-	} ),
-	Group: Object.assign( MenuGroup, {
-		displayName: 'Menu.Group',
-	} ),
-	GroupLabel: Object.assign( MenuGroupLabel, {
-		displayName: 'Menu.GroupLabel',
-	} ),
-	Separator: Object.assign( MenuSeparator, {
-		displayName: 'Menu.Separator',
-	} ),
-	ItemLabel: Object.assign( MenuItemLabel, {
-		displayName: 'Menu.ItemLabel',
-	} ),
-	ItemHelpText: Object.assign( MenuItemHelpText, {
-		displayName: 'Menu.ItemHelpText',
-	} ),
-} );
+export const Menu = Object.assign(
+	contextConnectWithoutRef( UnconnectedMenu, 'Menu' ),
+	{
+		Context: Object.assign( MenuContext, {
+			displayName: 'Menu.Context',
+		} ),
+		Item: Object.assign( MenuItem, {
+			displayName: 'Menu.Item',
+		} ),
+		RadioItem: Object.assign( MenuRadioItem, {
+			displayName: 'Menu.RadioItem',
+		} ),
+		CheckboxItem: Object.assign( MenuCheckboxItem, {
+			displayName: 'Menu.CheckboxItem',
+		} ),
+		Group: Object.assign( MenuGroup, {
+			displayName: 'Menu.Group',
+		} ),
+		GroupLabel: Object.assign( MenuGroupLabel, {
+			displayName: 'Menu.GroupLabel',
+		} ),
+		Separator: Object.assign( MenuSeparator, {
+			displayName: 'Menu.Separator',
+		} ),
+		ItemLabel: Object.assign( MenuItemLabel, {
+			displayName: 'Menu.ItemLabel',
+		} ),
+		ItemHelpText: Object.assign( MenuItemHelpText, {
+			displayName: 'Menu.ItemHelpText',
+		} ),
+		Popover: Object.assign( MenuPopover, {
+			displayName: 'Menu.Popover',
+		} ),
+		TriggerButton: Object.assign( MenuTriggerButton, {
+			displayName: 'Menu.TriggerButton',
+		} ),
+		SubmenuTriggerItem: Object.assign( MenuSubmenuTriggerItem, {
+			displayName: 'Menu.SubmenuTriggerItem',
+		} ),
+	}
+);
 
 export default Menu;
diff --git a/packages/components/src/menu/item.tsx b/packages/components/src/menu/item.tsx
@@ -15,7 +15,7 @@ export const MenuItem = forwardRef<
 	HTMLDivElement,
 	WordPressComponentProps< MenuItemProps, 'div', false >
 >( function MenuItem(
-	{ prefix, suffix, children, hideOnClick = true, ...props },
+	{ prefix, suffix, children, hideOnClick = true, store, ...props },
 	ref
 ) {
 	const menuContext = useContext( MenuContext );
@@ -26,13 +26,19 @@ export const MenuItem = forwardRef<
 		);
 	}
 
+	// In most cases, the menu store will be retrieved from context (ie. the store
+	// created by the top-level menu component). But in rare cases (ie.
+	// `Menu.SubmenuTriggerItem`), the context store wouldn't be correct. This is
+	// why the component accepts a `store` prop to override the context store.
+	const computedStore = store ?? menuContext.store;
+
 	return (
 		<Styled.MenuItem
 			ref={ ref }
 			{ ...props }
 			accessibleWhenDisabled
 			hideOnClick={ hideOnClick }
-			store={ menuContext.store }
+			store={ computedStore }
 		>
 			<Styled.ItemPrefixWrapper>{ prefix }</Styled.ItemPrefixWrapper>