From 22b528a9e0fe35f768d0f7a3e388a76d733c3372 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 20 Dec 2024 18:19:55 +0100 Subject: [PATCH 1/4] Menu: use ariakit types --- packages/components/src/menu/types.ts | 105 +++++++++++++++++--------- 1 file changed, 68 insertions(+), 37 deletions(-) diff --git a/packages/components/src/menu/types.ts b/packages/components/src/menu/types.ts index f58b5bcc89b95..106171672a4ce 100644 --- a/packages/components/src/menu/types.ts +++ b/packages/components/src/menu/types.ts @@ -2,7 +2,6 @@ * External dependencies */ import type * as Ariakit from '@ariakit/react'; -import type { Placement } from '@floating-ui/react-dom'; export interface MenuContext { /** @@ -19,36 +18,36 @@ export interface MenuProps { /** * The contents of the menu (ie. one or more menu items). */ - children?: React.ReactNode; + children?: Ariakit.MenuProviderProps[ 'children' ]; /** * The open state of the menu popover when it is initially rendered. Use when * not wanting to control its open state. * * @default false */ - defaultOpen?: boolean; + defaultOpen?: Ariakit.MenuProviderProps[ 'defaultOpen' ]; /** * The controlled open state of the menu popover. Must be used in conjunction * with `onOpenChange`. */ - open?: boolean; + open?: Ariakit.MenuProviderProps[ 'open' ]; /** * Event handler called when the open state of the menu popover changes. */ - onOpenChange?: ( open: boolean ) => void; + onOpenChange?: Ariakit.MenuProviderProps[ 'setOpen' ]; /** * The placement of the menu popover. * * @default 'bottom-start' for root-level menus, 'right-start' for nested menus */ - placement?: Placement; + placement?: Ariakit.MenuProviderProps[ 'placement' ]; } export interface MenuPopoverProps { /** * The contents of the dropdown. */ - children?: React.ReactNode; + children?: Ariakit.MenuProps[ 'children' ]; /** * The modality of the menu popover. When set to true, interaction with * outside elements will be disabled and only menu content will be visible to @@ -56,38 +55,34 @@ export interface MenuPopoverProps { * * @default true */ - modal?: boolean; + modal?: Ariakit.MenuProps[ 'modal' ]; /** * The distance between the popover and the anchor element. * * @default 8 for root-level menus, 16 for nested menus */ - gutter?: number; + gutter?: Ariakit.MenuProps[ 'gutter' ]; /** * The skidding of the popover along the anchor element. Can be set to * negative values to make the popover shift to the opposite side. * * @default 0 for root-level menus, -8 for nested menus */ - shift?: number; + shift?: Ariakit.MenuProps[ 'shift' ]; /** * Determines whether the menu popover will be hidden when the user presses * the Escape key. * * @default `( event ) => { event.preventDefault(); return true; }` */ - hideOnEscape?: - | boolean - | ( ( - event: KeyboardEvent | React.KeyboardEvent< Element > - ) => boolean ); + hideOnEscape?: Ariakit.MenuProps[ 'hideOnEscape' ]; } export interface MenuTriggerButtonProps { /** * The contents of the menu trigger button. */ - children?: React.ReactNode; + children?: Ariakit.MenuButtonProps[ 'children' ]; /** * Allows the component to be rendered as a different HTML element or React * component. The value can be a React element or a function that takes in the @@ -132,21 +127,21 @@ export interface MenuGroupProps { * The contents of the menu group (ie. an optional menu group label and one * or more menu items). */ - children: React.ReactNode; + children: Ariakit.MenuGroupProps[ 'children' ]; } export interface MenuGroupLabelProps { /** * The contents of the menu group label. */ - children: React.ReactNode; + children: Ariakit.MenuGroupLabelProps[ 'children' ]; } export interface MenuItemProps { /** * The contents of the menu item. */ - children: React.ReactNode; + children: Ariakit.MenuItemProps[ 'children' ]; /** * The contents of the menu item's prefix. */ @@ -160,11 +155,11 @@ export interface MenuItemProps { * * @default true */ - hideOnClick?: boolean; + hideOnClick?: Ariakit.MenuItemProps[ 'hideOnClick' ]; /** * Determines if the element is disabled. */ - disabled?: boolean; + disabled?: Ariakit.MenuItemProps[ 'disabled' ]; /** * Allows the component to be rendered as a different HTML element or React * component. The value can be a React element or a function that takes in the @@ -179,67 +174,103 @@ export interface MenuItemProps { store?: Ariakit.MenuItemProps[ 'store' ]; } -export interface MenuCheckboxItemProps - extends Omit< MenuItemProps, 'prefix' | 'hideOnClick' > { +export interface MenuCheckboxItemProps { + /** + * The contents of the menu item. + */ + children: Ariakit.MenuItemCheckboxProps[ 'children' ]; + /** + * The contents of the menu item's suffix. + */ + suffix?: React.ReactNode; /** * Whether to hide the menu popover when the menu item is clicked. * * @default false */ - hideOnClick?: boolean; + hideOnClick?: Ariakit.MenuItemCheckboxProps[ 'hideOnClick' ]; + /** + * Determines if the element is disabled. + */ + disabled?: Ariakit.MenuItemCheckboxProps[ 'disabled' ]; + /** + * Allows the component to be rendered as a different HTML element or React + * component. The value can be a React element or a function that takes in the + * original component props and gives back a React element with the props + * merged. + */ + render?: Ariakit.MenuItemCheckboxProps[ 'render' ]; /** * The checkbox menu item's name. */ - name: string; + name: Ariakit.MenuItemCheckboxProps[ 'name' ]; /** * The checkbox item's value, useful when using multiple checkbox menu items * associated to the same `name`. */ - value?: string; + value?: Ariakit.MenuItemCheckboxProps[ 'value' ]; /** * The controlled checked state of the checkbox menu item. */ - checked?: boolean; + checked?: Ariakit.MenuItemCheckboxProps[ 'checked' ]; /** * The checked state of the checkbox menu item when it is initially rendered. * Use when not wanting to control its checked state. */ - defaultChecked?: boolean; + defaultChecked?: Ariakit.MenuItemCheckboxProps[ 'defaultChecked' ]; /** * Event handler called when the checked state of the checkbox menu item changes. */ - onChange?: ( event: React.ChangeEvent< HTMLInputElement > ) => void; + onChange?: Ariakit.MenuItemCheckboxProps[ 'onChange' ]; } -export interface MenuRadioItemProps - extends Omit< MenuItemProps, 'prefix' | 'hideOnClick' > { +export interface MenuRadioItemProps { + /** + * The contents of the menu item. + */ + children: Ariakit.MenuItemRadioProps[ 'children' ]; + /** + * The contents of the menu item's suffix. + */ + suffix?: React.ReactNode; /** * Whether to hide the menu popover when the menu item is clicked. * * @default false */ - hideOnClick?: boolean; + hideOnClick?: Ariakit.MenuItemRadioProps[ 'hideOnClick' ]; + /** + * Determines if the element is disabled. + */ + disabled?: Ariakit.MenuItemRadioProps[ 'disabled' ]; + /** + * Allows the component to be rendered as a different HTML element or React + * component. The value can be a React element or a function that takes in the + * original component props and gives back a React element with the props + * merged. + */ + render?: Ariakit.MenuItemRadioProps[ 'render' ]; /** * The radio item's name. */ - name: string; + name: Ariakit.MenuItemRadioProps[ 'name' ]; /** * The radio item's value. */ - value: string | number; + value: Ariakit.MenuItemRadioProps[ 'value' ]; /** * The controlled checked state of the radio menu item. */ - checked?: boolean; + checked?: Ariakit.MenuItemRadioProps[ 'checked' ]; /** * The checked state of the radio menu item when it is initially rendered. * Use when not wanting to control its checked state. */ - defaultChecked?: boolean; + defaultChecked?: Ariakit.MenuItemRadioProps[ 'defaultChecked' ]; /** * Event handler called when the checked radio menu item changes. */ - onChange?: ( event: React.ChangeEvent< HTMLInputElement > ) => void; + onChange?: Ariakit.MenuItemRadioProps[ 'onChange' ]; } export interface MenuSeparatorProps {} From 711de4427e767eff874796af0e21a246576d7434 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 20 Dec 2024 18:50:30 +0100 Subject: [PATCH 2/4] Update types descriptions --- packages/components/src/menu/types.ts | 116 +++++++++++++++++++------- 1 file changed, 84 insertions(+), 32 deletions(-) diff --git a/packages/components/src/menu/types.ts b/packages/components/src/menu/types.ts index 106171672a4ce..40aeedd89adc2 100644 --- a/packages/components/src/menu/types.ts +++ b/packages/components/src/menu/types.ts @@ -14,38 +14,48 @@ export interface MenuContext { variant?: 'toolbar'; } +// TODO: add portal and preventBodyScroll to MenuPopoverProps, change default modality, move open props to popover? + export interface MenuProps { /** - * The contents of the menu (ie. one or more menu items). + * The elements, which should include one instance of the `Menu.TriggerButton` + * component and one instance of the `Menu.Popover` component. */ children?: Ariakit.MenuProviderProps[ 'children' ]; /** - * The open state of the menu popover when it is initially rendered. Use when - * not wanting to control its open state. + * Whether the menu popover and its contents should be visible by default. * + * Note: this prop will be overridden by the `open` prop if it is + * provided (meaning the component will be used in "controlled" mode). * @default false */ defaultOpen?: Ariakit.MenuProviderProps[ 'defaultOpen' ]; /** - * The controlled open state of the menu popover. Must be used in conjunction - * with `onOpenChange`. + * Whether the menu popover and its contents should be visible. + * Should be used in conjunction with `onOpenChange` in order to control + * the open state of the menu popover. + * + * Note: this prop will set the component in "controlled" mode, and it will + * override the `defaultOpen` prop. */ open?: Ariakit.MenuProviderProps[ 'open' ]; /** - * Event handler called when the open state of the menu popover changes. + * A callback that gets called when the `open` state changes. */ onOpenChange?: Ariakit.MenuProviderProps[ 'setOpen' ]; /** * The placement of the menu popover. * - * @default 'bottom-start' for root-level menus, 'right-start' for nested menus + * @default 'bottom-start' for root-level menus, 'right-start' for submenus */ placement?: Ariakit.MenuProviderProps[ 'placement' ]; } export interface MenuPopoverProps { /** - * The contents of the dropdown. + * The contents of the menu popover, which should include instances of the + * `Menu.Item`, `Menu.CheckboxItem`, `Menu.RadioItem`, `Menu.Group`, and + * `Menu.Separator` components. */ children?: Ariakit.MenuProps[ 'children' ]; /** @@ -53,6 +63,12 @@ export interface MenuPopoverProps { * outside elements will be disabled and only menu content will be visible to * screen readers. * + * Determines whether the menu popover is modal. Modal dialogs have distinct + * states and behaviors: + * - The `portal` and `preventBodyScroll` props are set to `true`. They can + * still be manually set to `false`. + * - When the dialog is open, element tree outside it will be inert. + * * @default true */ modal?: Ariakit.MenuProps[ 'modal' ]; @@ -70,8 +86,13 @@ export interface MenuPopoverProps { */ shift?: Ariakit.MenuProps[ 'shift' ]; /** - * Determines whether the menu popover will be hidden when the user presses - * the Escape key. + * Determines if the menu popover will hide when the user presses the + * Escape key. + * + * This prop can be either a boolean or a function that accepts an event as an + * argument and returns a boolean. The event object represents the keydown + * event that initiated the hide action, which could be either a native + * keyboard event or a React synthetic event. * * @default `( event ) => { event.preventDefault(); return true; }` */ @@ -98,9 +119,6 @@ export interface MenuTriggerButtonProps { * This feature can be combined with the `accessibleWhenDisabled` prop to * make disabled elements still accessible via keyboard. * - * **Note**: For this prop to work, the `focusable` prop must be set to - * `true`, if it's not set by default. - * * @default false */ disabled?: Ariakit.MenuButtonProps[ 'disabled' ]; @@ -124,40 +142,50 @@ export interface MenuTriggerButtonProps { export interface MenuGroupProps { /** - * The contents of the menu group (ie. an optional menu group label and one - * or more menu items). + * The contents of the menu group, which should include one instance of the + * `Menu.GroupLabel` component and one or more instances of `Menu.Item`, + * `Menu.CheckboxItem`, and `Menu.RadioItem`. */ children: Ariakit.MenuGroupProps[ 'children' ]; } export interface MenuGroupLabelProps { /** - * The contents of the menu group label. + * The contents of the menu group label, which should provide an accessible + * label for the menu group. */ children: Ariakit.MenuGroupLabelProps[ 'children' ]; } export interface MenuItemProps { /** - * The contents of the menu item. + * The contents of the menu item, which could include one instance of the + * `Menu.ItemLabel` component and/or one instance of the `Menu.ItemHelpText` + * component. */ children: Ariakit.MenuItemProps[ 'children' ]; /** - * The contents of the menu item's prefix. + * The contents of the menu item's prefix, such as an icon. */ prefix?: React.ReactNode; /** - * The contents of the menu item's suffix. + * The contents of the menu item's suffix, such as a keyboard shortcut. */ suffix?: React.ReactNode; /** - * Whether to hide the menu popover when the menu item is clicked. + * Determines if the menu should hide when this item is clicked. + * + * **Note**: This behavior isn't triggered if this menu item is rendered as a + * link and modifier keys are used to either open the link in a new tab or + * download it. * * @default true */ hideOnClick?: Ariakit.MenuItemProps[ 'hideOnClick' ]; /** - * Determines if the element is disabled. + * Determines if the element is disabled. This sets the `aria-disabled` + * attribute accordingly, enabling support for all elements, including those + * that don't support the native `disabled` attribute. */ disabled?: Ariakit.MenuItemProps[ 'disabled' ]; /** @@ -168,7 +196,7 @@ export interface MenuItemProps { */ render?: Ariakit.MenuItemProps[ 'render' ]; /** - * The ariakit store. This prop is only meant for internal use. + * The ariakit menu store. This prop is only meant for internal use. * @ignore */ store?: Ariakit.MenuItemProps[ 'store' ]; @@ -176,21 +204,29 @@ export interface MenuItemProps { export interface MenuCheckboxItemProps { /** - * The contents of the menu item. + * The contents of the menu item, which could include one instance of the + * `Menu.ItemLabel` component and/or one instance of the `Menu.ItemHelpText` + * component. */ children: Ariakit.MenuItemCheckboxProps[ 'children' ]; /** - * The contents of the menu item's suffix. + * The contents of the menu item's suffix, such as a keyboard shortcut. */ suffix?: React.ReactNode; /** - * Whether to hide the menu popover when the menu item is clicked. + * Determines if the menu should hide when this item is clicked. + * + * **Note**: This behavior isn't triggered if this menu item is rendered as a + * link and modifier keys are used to either open the link in a new tab or + * download it. * * @default false */ hideOnClick?: Ariakit.MenuItemCheckboxProps[ 'hideOnClick' ]; /** - * Determines if the element is disabled. + * Determines if the element is disabled. This sets the `aria-disabled` + * attribute accordingly, enabling support for all elements, including those + * that don't support the native `disabled` attribute. */ disabled?: Ariakit.MenuItemCheckboxProps[ 'disabled' ]; /** @@ -211,36 +247,48 @@ export interface MenuCheckboxItemProps { value?: Ariakit.MenuItemCheckboxProps[ 'value' ]; /** * The controlled checked state of the checkbox menu item. + * + * Note: this prop will override the `defaultChecked` prop. */ checked?: Ariakit.MenuItemCheckboxProps[ 'checked' ]; /** * The checked state of the checkbox menu item when it is initially rendered. * Use when not wanting to control its checked state. + * + * Note: this prop will be overriden by the `checked` prop, if it is defined. */ defaultChecked?: Ariakit.MenuItemCheckboxProps[ 'defaultChecked' ]; /** - * Event handler called when the checked state of the checkbox menu item changes. + * A function that is called when the checkbox's checked state changes. */ onChange?: Ariakit.MenuItemCheckboxProps[ 'onChange' ]; } export interface MenuRadioItemProps { /** - * The contents of the menu item. + * The contents of the menu item, which could include one instance of the + * `Menu.ItemLabel` component and/or one instance of the `Menu.ItemHelpText` + * component. */ children: Ariakit.MenuItemRadioProps[ 'children' ]; /** - * The contents of the menu item's suffix. + * The contents of the menu item's suffix, such as a keyboard shortcut. */ suffix?: React.ReactNode; /** - * Whether to hide the menu popover when the menu item is clicked. + * Determines if the menu should hide when this item is clicked. + * + * **Note**: This behavior isn't triggered if this menu item is rendered as a + * link and modifier keys are used to either open the link in a new tab or + * download it. * * @default false */ hideOnClick?: Ariakit.MenuItemRadioProps[ 'hideOnClick' ]; /** - * Determines if the element is disabled. + * Determines if the element is disabled. This sets the `aria-disabled` + * attribute accordingly, enabling support for all elements, including those + * that don't support the native `disabled` attribute. */ disabled?: Ariakit.MenuItemRadioProps[ 'disabled' ]; /** @@ -260,15 +308,19 @@ export interface MenuRadioItemProps { value: Ariakit.MenuItemRadioProps[ 'value' ]; /** * The controlled checked state of the radio menu item. + * + * Note: this prop will override the `defaultChecked` prop. */ checked?: Ariakit.MenuItemRadioProps[ 'checked' ]; /** * The checked state of the radio menu item when it is initially rendered. * Use when not wanting to control its checked state. + * + * Note: this prop will be overriden by the `checked` prop, if it is defined. */ defaultChecked?: Ariakit.MenuItemRadioProps[ 'defaultChecked' ]; /** - * Event handler called when the checked radio menu item changes. + * A function that is called when the checkbox's checked state changes. */ onChange?: Ariakit.MenuItemRadioProps[ 'onChange' ]; } From 09d3085da7ffa6b48e285b1874dead6ab5d9d2fd Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 20 Dec 2024 18:51:44 +0100 Subject: [PATCH 3/4] Add default value for disabled prop --- packages/components/src/menu/checkbox-item.tsx | 3 ++- packages/components/src/menu/item.tsx | 11 ++++++++++- packages/components/src/menu/radio-item.tsx | 3 ++- packages/components/src/menu/types.ts | 7 +++++++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/components/src/menu/checkbox-item.tsx b/packages/components/src/menu/checkbox-item.tsx index ddb700b43324a..69339387c3add 100644 --- a/packages/components/src/menu/checkbox-item.tsx +++ b/packages/components/src/menu/checkbox-item.tsx @@ -21,7 +21,7 @@ export const MenuCheckboxItem = forwardRef< HTMLDivElement, WordPressComponentProps< MenuCheckboxItemProps, 'div', false > >( function MenuCheckboxItem( - { suffix, children, hideOnClick = false, ...props }, + { suffix, children, disabled = false, hideOnClick = false, ...props }, ref ) { const menuContext = useContext( MenuContext ); @@ -37,6 +37,7 @@ export const MenuCheckboxItem = forwardRef< ref={ ref } { ...props } accessibleWhenDisabled + disabled={ disabled } hideOnClick={ hideOnClick } store={ menuContext.store } > diff --git a/packages/components/src/menu/item.tsx b/packages/components/src/menu/item.tsx index 84ff050bcc223..a716cbcc89654 100644 --- a/packages/components/src/menu/item.tsx +++ b/packages/components/src/menu/item.tsx @@ -15,7 +15,15 @@ export const MenuItem = forwardRef< HTMLDivElement, WordPressComponentProps< MenuItemProps, 'div', false > >( function MenuItem( - { prefix, suffix, children, hideOnClick = true, store, ...props }, + { + prefix, + suffix, + children, + disabled = false, + hideOnClick = true, + store, + ...props + }, ref ) { const menuContext = useContext( MenuContext ); @@ -37,6 +45,7 @@ export const MenuItem = forwardRef< ref={ ref } { ...props } accessibleWhenDisabled + disabled={ disabled } hideOnClick={ hideOnClick } store={ computedStore } > diff --git a/packages/components/src/menu/radio-item.tsx b/packages/components/src/menu/radio-item.tsx index 5534a6b7f3e10..28b3199d7d36b 100644 --- a/packages/components/src/menu/radio-item.tsx +++ b/packages/components/src/menu/radio-item.tsx @@ -28,7 +28,7 @@ export const MenuRadioItem = forwardRef< HTMLDivElement, WordPressComponentProps< MenuRadioItemProps, 'div', false > >( function MenuRadioItem( - { suffix, children, hideOnClick = false, ...props }, + { suffix, children, disabled = false, hideOnClick = false, ...props }, ref ) { const menuContext = useContext( MenuContext ); @@ -44,6 +44,7 @@ export const MenuRadioItem = forwardRef< ref={ ref } { ...props } accessibleWhenDisabled + disabled={ disabled } hideOnClick={ hideOnClick } store={ menuContext.store } > diff --git a/packages/components/src/menu/types.ts b/packages/components/src/menu/types.ts index 40aeedd89adc2..1ccb4c54b4b5d 100644 --- a/packages/components/src/menu/types.ts +++ b/packages/components/src/menu/types.ts @@ -27,6 +27,7 @@ export interface MenuProps { * * Note: this prop will be overridden by the `open` prop if it is * provided (meaning the component will be used in "controlled" mode). + * * @default false */ defaultOpen?: Ariakit.MenuProviderProps[ 'defaultOpen' ]; @@ -186,6 +187,8 @@ export interface MenuItemProps { * Determines if the element is disabled. This sets the `aria-disabled` * attribute accordingly, enabling support for all elements, including those * that don't support the native `disabled` attribute. + * + * @default false */ disabled?: Ariakit.MenuItemProps[ 'disabled' ]; /** @@ -227,6 +230,8 @@ export interface MenuCheckboxItemProps { * Determines if the element is disabled. This sets the `aria-disabled` * attribute accordingly, enabling support for all elements, including those * that don't support the native `disabled` attribute. + * + * @default false */ disabled?: Ariakit.MenuItemCheckboxProps[ 'disabled' ]; /** @@ -289,6 +294,8 @@ export interface MenuRadioItemProps { * Determines if the element is disabled. This sets the `aria-disabled` * attribute accordingly, enabling support for all elements, including those * that don't support the native `disabled` attribute. + * + * @default false */ disabled?: Ariakit.MenuItemRadioProps[ 'disabled' ]; /** From bfc6593a9ae4423c282376bedea25ae75047b863 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Sun, 22 Dec 2024 16:14:11 +0100 Subject: [PATCH 4/4] Remove TODO comment --- packages/components/src/menu/types.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/components/src/menu/types.ts b/packages/components/src/menu/types.ts index 1ccb4c54b4b5d..f9bb0782529d1 100644 --- a/packages/components/src/menu/types.ts +++ b/packages/components/src/menu/types.ts @@ -14,8 +14,6 @@ export interface MenuContext { variant?: 'toolbar'; } -// TODO: add portal and preventBodyScroll to MenuPopoverProps, change default modality, move open props to popover? - export interface MenuProps { /** * The elements, which should include one instance of the `Menu.TriggerButton`