From a887654f8edf2c6a582323193c74a4ebfcf44846 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 23 Oct 2024 19:42:53 +0200 Subject: [PATCH 1/5] Throw if subcomponents are not used inside top level component --- .../components/src/dropdown-menu-v2/checkbox-item.tsx | 6 ++++++ .../components/src/dropdown-menu-v2/group-label.tsx | 7 +++++++ packages/components/src/dropdown-menu-v2/group.tsx | 7 +++++++ .../src/dropdown-menu-v2/item-help-text.tsx | 11 ++++++++++- .../components/src/dropdown-menu-v2/item-label.tsx | 11 ++++++++++- packages/components/src/dropdown-menu-v2/item.tsx | 5 +++++ .../components/src/dropdown-menu-v2/radio-item.tsx | 6 ++++++ .../components/src/dropdown-menu-v2/separator.tsx | 7 +++++++ 8 files changed, 58 insertions(+), 2 deletions(-) diff --git a/packages/components/src/dropdown-menu-v2/checkbox-item.tsx b/packages/components/src/dropdown-menu-v2/checkbox-item.tsx index bcbc920cbb7205..476c63437902b8 100644 --- a/packages/components/src/dropdown-menu-v2/checkbox-item.tsx +++ b/packages/components/src/dropdown-menu-v2/checkbox-item.tsx @@ -29,6 +29,12 @@ export const DropdownMenuCheckboxItem = forwardRef< const focusVisibleFixProps = useTemporaryFocusVisibleFix( { onBlur } ); const dropdownMenuContext = useContext( DropdownMenuContext ); + if ( ! dropdownMenuContext?.store ) { + throw new Error( + 'DropdownMenu.CheckboxItem can only be rendered inside a DropdownMenu component' + ); + } + return ( >( function DropdownMenuGroup( props, ref ) { const dropdownMenuContext = useContext( DropdownMenuContext ); + + if ( ! dropdownMenuContext?.store ) { + throw new Error( + 'DropdownMenu.GroupLabel can only be rendered inside a DropdownMenu component' + ); + } + return ( >( function DropdownMenuGroup( props, ref ) { const dropdownMenuContext = useContext( DropdownMenuContext ); + + if ( ! dropdownMenuContext?.store ) { + throw new Error( + 'DropdownMenu.Group can only be rendered inside a DropdownMenu component' + ); + } + return ( >( function DropdownMenuItemHelpText( props, ref ) { + const dropdownMenuContext = useContext( DropdownMenuContext ); + + if ( ! dropdownMenuContext?.store ) { + throw new Error( + 'DropdownMenu.ItemHelpText can only be rendered inside a DropdownMenu component' + ); + } + return ( >( function DropdownMenuItemLabel( props, ref ) { + const dropdownMenuContext = useContext( DropdownMenuContext ); + + if ( ! dropdownMenuContext?.store ) { + throw new Error( + 'DropdownMenu.ItemLabel can only be rendered inside a DropdownMenu component' + ); + } + return ( >( function DropdownMenuSeparator( props, ref ) { const dropdownMenuContext = useContext( DropdownMenuContext ); + + if ( ! dropdownMenuContext?.store ) { + throw new Error( + 'DropdownMenu.Separator can only be rendered inside a DropdownMenu component' + ); + } + return ( Date: Wed, 23 Oct 2024 19:43:45 +0200 Subject: [PATCH 2/5] Add render and store props to the menu item component --- packages/components/src/dropdown-menu-v2/item.tsx | 7 +++++-- packages/components/src/dropdown-menu-v2/types.ts | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/components/src/dropdown-menu-v2/item.tsx b/packages/components/src/dropdown-menu-v2/item.tsx index 585552a876dd28..d7c354a4c48c36 100644 --- a/packages/components/src/dropdown-menu-v2/item.tsx +++ b/packages/components/src/dropdown-menu-v2/item.tsx @@ -16,7 +16,7 @@ export const DropdownMenuItem = forwardRef< HTMLDivElement, WordPressComponentProps< DropdownMenuItemProps, 'div', false > >( function DropdownMenuItem( - { prefix, suffix, children, onBlur, hideOnClick = true, ...props }, + { prefix, suffix, children, onBlur, hideOnClick = true, store, ...props }, ref ) { // TODO: Remove when https://github.com/ariakit/ariakit/issues/4083 is fixed @@ -28,6 +28,9 @@ export const DropdownMenuItem = forwardRef< 'DropdownMenu.Item can only be rendered inside a DropdownMenu component' ); } + + const computedStore = store ?? dropdownMenuContext.store; + return ( { prefix } diff --git a/packages/components/src/dropdown-menu-v2/types.ts b/packages/components/src/dropdown-menu-v2/types.ts index 795cd9ac76ff58..e64a4f4528ae77 100644 --- a/packages/components/src/dropdown-menu-v2/types.ts +++ b/packages/components/src/dropdown-menu-v2/types.ts @@ -117,6 +117,12 @@ export interface DropdownMenuItemProps { * Determines if the element is disabled. */ disabled?: boolean; + + render?: Ariakit.MenuItemProps[ 'render' ]; + /** + * @ignore + */ + store?: Ariakit.MenuItemProps[ 'store' ]; } export interface DropdownMenuCheckboxItemProps From ed07d737a430549c51686ae5711b40e1b8cffd2c Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 23 Oct 2024 19:45:29 +0200 Subject: [PATCH 3/5] Split top level component into more granular sub-components: - trigger button - submenu trigger item - popover Update types accordingly --- .../components/src/dropdown-menu-v2/index.tsx | 157 +++--------------- .../src/dropdown-menu-v2/menu-popover.tsx | 108 ++++++++++++ .../dropdown-menu-v2/submenu-trigger-item.tsx | 58 +++++++ .../src/dropdown-menu-v2/trigger-button.tsx | 46 +++++ .../components/src/dropdown-menu-v2/types.ts | 37 +++-- 5 files changed, 264 insertions(+), 142 deletions(-) create mode 100644 packages/components/src/dropdown-menu-v2/menu-popover.tsx create mode 100644 packages/components/src/dropdown-menu-v2/submenu-trigger-item.tsx create mode 100644 packages/components/src/dropdown-menu-v2/trigger-button.tsx diff --git a/packages/components/src/dropdown-menu-v2/index.tsx b/packages/components/src/dropdown-menu-v2/index.tsx index 50c4f3069d51b5..0b991494dfb14c 100644 --- a/packages/components/src/dropdown-menu-v2/index.tsx +++ b/packages/components/src/dropdown-menu-v2/index.tsx @@ -2,31 +2,21 @@ * External dependencies */ import * as Ariakit from '@ariakit/react'; -import { useStoreState } 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 { DropdownMenuContext as DropdownMenuContextType, DropdownMenuProps, } from './types'; -import * as Styled from './styles'; import { DropdownMenuContext } from './context'; import { DropdownMenuItem } from './item'; import { DropdownMenuCheckboxItem } from './checkbox-item'; @@ -36,48 +26,37 @@ import { DropdownMenuGroupLabel } from './group-label'; import { DropdownMenuSeparator } from './separator'; import { DropdownMenuItemLabel } from './item-label'; import { DropdownMenuItemHelpText } from './item-help-text'; +import { DropdownMenuTriggerButton } from './trigger-button'; +import { DropdownMenuSubmenuTriggerItem } from './submenu-trigger-item'; +import { DropdownMenuPopover } from './menu-popover'; -const UnconnectedDropdownMenu = ( - props: WordPressComponentProps< DropdownMenuProps, 'div', false >, - ref: React.ForwardedRef< HTMLDivElement > -) => { +const UnconnectedDropdownMenu = ( props: DropdownMenuProps ) => { const { - // Store props open, defaultOpen = false, onOpenChange, placement, - - // Menu trigger props - trigger, - - // Menu props - gutter, children, - shift, - modal = true, // From internal components context variant, - - // Rest - ...otherProps } = useContextSystem< + // @ts-expect-error TODO: missing 'className' typeof props & Pick< DropdownMenuContextType, 'variant' > >( props, 'DropdownMenu' ); const parentContext = useContext( DropdownMenuContext ); - 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 dropdown, // and of `right-start` for nested dropdowns. 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', @@ -100,7 +79,7 @@ const UnconnectedDropdownMenu = ( setOpen( willBeOpen ) { onOpenChange?.( willBeOpen ); }, - rtl: computedDirection === 'rtl', + rtl, } ); const contextValue = useMemo( @@ -108,112 +87,28 @@ const UnconnectedDropdownMenu = ( [ dropdownMenuStore, 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 = useStoreState( - dropdownMenuStore, - 'currentPlacement' - ).split( '-' )[ 0 ]; - - if ( - dropdownMenuStore.parent && - ! ( isValidElement( trigger ) && DropdownMenuItem === trigger.type ) - ) { - // eslint-disable-next-line no-console - console.warn( - 'For nested DropdownMenus, the `trigger` should always be a `DropdownMenuItem`.' - ); - } - - 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 */ } - - { trigger.props.suffix } -