Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Menu: more granular sub-components #67422

Merged
merged 22 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f448b20
MenuItem: add render and store props
ciampo Nov 29, 2024
ed4f816
Extract sub-components: popover, trigger button, submenu trigger item
ciampo Nov 29, 2024
51c7031
Unit tests
ciampo Nov 29, 2024
2ed865a
CHANGELOG
ciampo Dec 2, 2024
90f981a
Add more memory to node on CI
ciampo Dec 2, 2024
afb35c4
Refactor block bindings panel menu (#67633)
ciampo Dec 6, 2024
39d8535
Storybook (#67632)
ciampo Dec 11, 2024
3100abe
Refactor dataviews item actions menu (#67636)
ciampo Dec 11, 2024
25be050
Refactor dataviews view config menu (#67637)
ciampo Dec 11, 2024
186f3b3
Refactor global styles shadows edit panel menu (#67641)
ciampo Dec 11, 2024
0c7a18d
Refactor global styles font size menus (#67642)
ciampo Dec 11, 2024
89cb8e7
Refactor "Add filter" dataviews menu (#67634)
ciampo Dec 11, 2024
f43329e
Menu granular subcomponents: Refactor dataviews list layout actions m…
ciampo Dec 12, 2024
9a4942a
Menu granular subcomponents: Refactor dataviews table layout header m…
ciampo Dec 12, 2024
e6ab105
Menu granular subcomponents: Refactor post actions menu (#67645)
ciampo Dec 12, 2024
6ef71ac
Better comments for submenu trigger store
ciampo Dec 12, 2024
07c1f35
Typo
ciampo Dec 12, 2024
2391098
Remove unnecessary MenuSubmenuTriggerItemProps type
ciampo Dec 12, 2024
ccd1341
Don't break the rules of hooks 🪝
ciampo Dec 12, 2024
de72818
Move CHANGELOG entry to unreleased section
ciampo Dec 12, 2024
5bba91c
Add explicit MenuProps to improve TS performance
ciampo Dec 13, 2024
3949e54
Remove node memory settings
ciampo Dec 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 15 additions & 16 deletions packages/block-editor/src/hooks/block-bindings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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>
);
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
225 changes: 61 additions & 164 deletions packages/components/src/menu/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
ciampo marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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';
Expand All @@ -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
ciampo marked this conversation as resolved.
Show resolved Hide resolved
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',
Expand All @@ -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;
10 changes: 8 additions & 2 deletions packages/components/src/menu/item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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;
ciampo marked this conversation as resolved.
Show resolved Hide resolved

return (
<Styled.MenuItem
ref={ ref }
{ ...props }
accessibleWhenDisabled
hideOnClick={ hideOnClick }
store={ menuContext.store }
store={ computedStore }
>
<Styled.ItemPrefixWrapper>{ prefix }</Styled.ItemPrefixWrapper>

Expand Down
Loading
Loading