diff --git a/change/@fluentui-react-menu-fd6e876a-9e71-48ba-9ace-e1871dcb06cf.json b/change/@fluentui-react-menu-fd6e876a-9e71-48ba-9ace-e1871dcb06cf.json new file mode 100644 index 00000000000000..76e9a95c75dc30 --- /dev/null +++ b/change/@fluentui-react-menu-fd6e876a-9e71-48ba-9ace-e1871dcb06cf.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add correct nesting validation for Menu and MenuGridcomponents and subcomponents", + "packageName": "@fluentui/react-menu", + "email": "adam.samec@gmail.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-menu-grid-preview-5a9818a1-d70c-4cfd-9c01-99c3ebaf4550.json b/change/@fluentui-react-menu-grid-preview-5a9818a1-d70c-4cfd-9c01-99c3ebaf4550.json new file mode 100644 index 00000000000000..f489944db45b50 --- /dev/null +++ b/change/@fluentui-react-menu-grid-preview-5a9818a1-d70c-4cfd-9c01-99c3ebaf4550.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Feat(react-menu-grid-preview): Detect invalid MenuGrid nesting", + "packageName": "@fluentui/react-menu-grid-preview", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-menu-grid-preview/library/src/components/MenuGrid/useMenuGrid.ts b/packages/react-components/react-menu-grid-preview/library/src/components/MenuGrid/useMenuGrid.ts index ab8a1d6f91d27b..c7a922e92463e0 100644 --- a/packages/react-components/react-menu-grid-preview/library/src/components/MenuGrid/useMenuGrid.ts +++ b/packages/react-components/react-menu-grid-preview/library/src/components/MenuGrid/useMenuGrid.ts @@ -1,13 +1,16 @@ import * as React from 'react'; -import { getIntrinsicElementProps, slot } from '@fluentui/react-utilities'; +import { useMergedRefs, getIntrinsicElementProps, slot } from '@fluentui/react-utilities'; + import { useTableCompositeNavigation } from '@fluentui/react-table'; import type { MenuGridProps, MenuGridState } from './MenuGrid.types'; import { useMenuContext_unstable } from '@fluentui/react-menu'; +import { useValidateNesting } from '../../utils/useValidateNesting'; /** * Returns the props and state required to render the component */ export const useMenuGrid_unstable = (props: MenuGridProps, ref: React.Ref): MenuGridState => { + const validateNestingRef = useValidateNesting('MenuGrid'); const triggerId = useMenuContext_unstable(context => context.triggerId); const { tableRowTabsterAttribute, tableTabsterAttribute, onTableKeyDown } = useTableCompositeNavigation(); @@ -17,7 +20,7 @@ export const useMenuGrid_unstable = (props: MenuGridProps, ref: React.Ref): MenuGridCellState { + const validateNestingRef = useValidateNesting('MenuGridCell'); const { visuallyHidden } = props; return { @@ -15,7 +17,7 @@ export function useMenuGridCell_unstable(props: MenuGridCellProps, ref: React.Re }, root: slot.always( getIntrinsicElementProps('div', { - ref, + ref: useMergedRefs(ref, validateNestingRef), role: 'gridcell', ...props, }), diff --git a/packages/react-components/react-menu-grid-preview/library/src/components/MenuGridItem/useMenuGridItem.ts b/packages/react-components/react-menu-grid-preview/library/src/components/MenuGridItem/useMenuGridItem.ts index 42bc5e7963ec20..00efab4df96425 100644 --- a/packages/react-components/react-menu-grid-preview/library/src/components/MenuGridItem/useMenuGridItem.ts +++ b/packages/react-components/react-menu-grid-preview/library/src/components/MenuGridItem/useMenuGridItem.ts @@ -1,9 +1,10 @@ import * as React from 'react'; -import { slot } from '@fluentui/react-utilities'; +import { useMergedRefs, slot } from '@fluentui/react-utilities'; import { MenuGridItemProps, MenuGridItemState } from './MenuGridItem.types'; import { MenuGridCell } from './../MenuGridCell/MenuGridCell'; import { MenuGridRow } from './../MenuGridRow/MenuGridRow'; +import { useValidateNesting } from '../../utils/useValidateNesting'; /** * Given user props, returns state and render function for a MenuGridItem. @@ -17,6 +18,7 @@ export function useMenuGridItem_unstable(props: MenuGridItemProps, ref: React.Re secondSubAction, ...rest } = props; + const validateNestingRef = useValidateNesting('MenuGridItem'); return { components: { @@ -29,7 +31,7 @@ export function useMenuGridItem_unstable(props: MenuGridItemProps, ref: React.Re }, root: slot.always( { - ref, + ref: useMergedRefs(ref, validateNestingRef), ...rest, }, { elementType: MenuGridRow }, diff --git a/packages/react-components/react-menu-grid-preview/library/src/components/MenuGridRow/useMenuGridRow.ts b/packages/react-components/react-menu-grid-preview/library/src/components/MenuGridRow/useMenuGridRow.ts index 8afe2fed30b9e4..7922995e16cd82 100644 --- a/packages/react-components/react-menu-grid-preview/library/src/components/MenuGridRow/useMenuGridRow.ts +++ b/packages/react-components/react-menu-grid-preview/library/src/components/MenuGridRow/useMenuGridRow.ts @@ -1,13 +1,15 @@ import * as React from 'react'; -import { getIntrinsicElementProps, slot } from '@fluentui/react-utilities'; +import { useMergedRefs, getIntrinsicElementProps, slot } from '@fluentui/react-utilities'; import { useMenuGridContext_unstable } from '../../contexts/menuGridContext'; import { MenuGridRowProps, MenuGridRowState } from './MenuGridRow.types'; +import { useValidateNesting } from '../../utils/useValidateNesting'; /** * Given user props, returns state and render function for a MenuGridRow. */ export function useMenuGridRow_unstable(props: MenuGridRowProps, ref: React.Ref): MenuGridRowState { + const validateNestingRef = useValidateNesting('MenuGridRow'); const { tableRowTabsterAttribute } = useMenuGridContext_unstable(); return { @@ -16,7 +18,7 @@ export function useMenuGridRow_unstable(props: MenuGridRowProps, ref: React.Ref< }, root: slot.always( getIntrinsicElementProps('div', { - ref, + ref: useMergedRefs(ref, validateNestingRef), role: 'row', tabIndex: 0, ...tableRowTabsterAttribute, diff --git a/packages/react-components/react-menu-grid-preview/library/src/utils/index.ts b/packages/react-components/react-menu-grid-preview/library/src/utils/index.ts new file mode 100644 index 00000000000000..44a457f4bc88d4 --- /dev/null +++ b/packages/react-components/react-menu-grid-preview/library/src/utils/index.ts @@ -0,0 +1 @@ +export { useValidateNesting } from './useValidateNesting'; diff --git a/packages/react-components/react-menu-grid-preview/library/src/utils/useValidateNesting.ts b/packages/react-components/react-menu-grid-preview/library/src/utils/useValidateNesting.ts new file mode 100644 index 00000000000000..bb448555588ff3 --- /dev/null +++ b/packages/react-components/react-menu-grid-preview/library/src/utils/useValidateNesting.ts @@ -0,0 +1,98 @@ +import * as React from 'react'; +import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; + +import type { MenuContextValue } from '@fluentui/react-menu'; +import { useMenuContext_unstable } from '@fluentui/react-menu'; + +type NestingComponentName = 'MenuGrid' | 'MenuGridCell' | 'MenuGridItem' | 'MenuGridRow'; +type MenuItemRoles = 'menuitem' | 'menuitemcheckbox' | 'menuitemradio'; + +const menuItemRoleToNameMapping = { + menuitem: 'MenuItem', + menuitemcheckbox: 'MenuItemCheckbox', + menuitemradio: 'MenuItemRadio', +}; + +export const useValidateNesting = (componentName: NestingComponentName): React.RefObject => { + 'use no memo'; + + const { targetDocument } = useFluent(); + const triggerRef = useMenuContext_unstable((context: MenuContextValue) => context.triggerRef); + const inline = useMenuContext_unstable((context: MenuContextValue) => context.inline); + const ref = React.useRef(null); + + if (process.env.NODE_ENV !== 'production') { + // This check should run only in development mode + // It's okay to disable the ESLint rule because we ar checking env variable statically (not at runtime) + // eslint-disable-next-line react-hooks/rules-of-hooks + React.useEffect(() => { + let ancestor = ref.current; + let ancestorRole; + + do { + ancestor = ancestor?.parentElement ?? null; + ancestorRole = ancestor?.getAttribute('role'); + if (ancestor?.classList.contains('fui-MenuGrid')) { + if (componentName === 'MenuGridCell') { + throw new Error( + 'MenuGridCell is incorrectly nested within MenuGrid. You probably want to wrap it in a MenuGridRow.', + ); + } + break; + } + if (ancestor?.classList.contains('fui-MenuGridRow') && componentName === 'MenuGridCell') { + break; + } + if (['menuitem', 'menuitemcheckbox', 'menuitemradio'].includes(ancestorRole ?? '')) { + throw new Error( + `${componentName} is incorrectly nested within ${ + menuItemRoleToNameMapping[ancestorRole as MenuItemRoles] + } or within an element with the "${ancestorRole}" role.`, + ); + } + if (ancestorRole === 'menu') { + let message = ''; + let breakAncestorTraversal = false; + switch (componentName) { + case 'MenuGrid': + if (inline && getMenuOfTrigger(triggerRef.current, targetDocument) === ancestor) { + // Handle the case when MenuGrid is inline next to its menu trigger which is a submenu item of the parent menu + breakAncestorTraversal = true; + break; + } + message = 'MenuGrid is incorrectly nested within MenuList or within an element with the "menu" role.'; + break; + case 'MenuGridCell': + message = + 'MenuGridCell is incorrectly nested within MenuList or within an element with the "menu" role. You probably want to wrap it in a MenuGridRow.'; + break; + case 'MenuGridItem': + message = + 'MenuGridItem is incorrectly nested within MenuList or within an element with the "menu" role. You probably want to wrap it in a MenuGrid instead.'; + break; + case 'MenuGridRow': + message = + 'MenuGridRow is incorrectly nested within MenuList or within an element with the "menu" role. You probably want to wrap it in a MenuGrid instead.'; + break; + } + if (breakAncestorTraversal) { + break; + } + throw new Error(message); + } + } while (ancestor && ancestor !== targetDocument?.body); + }, [componentName, ref, triggerRef, inline, targetDocument]); + } + return ref; +}; + +const getMenuOfTrigger = (trigger: HTMLElement | null, targetDocument?: Document): HTMLElement | null => { + let ancestor = trigger?.parentElement; + while (ancestor && ancestor !== targetDocument?.body) { + if (ancestor?.getAttribute('role') === 'menu') { + return ancestor; + } + ancestor = ancestor?.parentElement ?? null; + } + return null; +}; diff --git a/packages/react-components/react-menu/library/src/components/MenuItem/useMenuItem.tsx b/packages/react-components/react-menu/library/src/components/MenuItem/useMenuItem.tsx index 134ff33a85c7a7..ed7288d4762893 100644 --- a/packages/react-components/react-menu/library/src/components/MenuItem/useMenuItem.tsx +++ b/packages/react-components/react-menu/library/src/components/MenuItem/useMenuItem.tsx @@ -27,6 +27,7 @@ import { } from '@fluentui/react-aria'; import { Enter, Space } from '@fluentui/keyboard-keys'; import { useIsInMenuSplitGroup, useMenuSplitGroupContext_unstable } from '../../contexts/menuSplitGroupContext'; +import { useValidateNesting } from '../../utils/useValidateNesting'; const ChevronRightIcon = bundleIcon(ChevronRightFilled, ChevronRightRegular); const ChevronLeftIcon = bundleIcon(ChevronLeftFilled, ChevronLeftRegular); @@ -53,6 +54,8 @@ export const useMenuItem_unstable = (props: MenuItemProps, ref: React.Ref>(null); const dismissedWithKeyboardRef = React.useRef(false); + const validateNestingRef = useValidateNesting(getValidateNestingComponentName(props.role)); + const state: MenuItemState = { hasSubmenu, disabled, @@ -74,7 +77,7 @@ export const useMenuItem_unstable = (props: MenuItemProps, ref: React.Ref>, + ref: useMergedRefs(ref, innerRef, validateNestingRef) as React.Ref>, onKeyDown: useEventCallback(event => { props.onKeyDown?.(event); if (!event.isDefaultPrevented() && (event.key === Space || event.key === Enter)) { @@ -158,3 +161,13 @@ const useIconAndCheckmarkAlignment = (options: { hasSubmenu: boolean }) => { hasCheckmarks: hasCheckmarks && !isSplitItemTrigger, }; }; + +const getValidateNestingComponentName = (role?: string) => { + switch (role) { + case 'menuitemcheckbox': + return 'MenuItemCheckbox'; + case 'menuitemradio': + return 'MenuItemRadio'; + } + return 'MenuItem'; +}; diff --git a/packages/react-components/react-menu/library/src/components/MenuList/useMenuList.ts b/packages/react-components/react-menu/library/src/components/MenuList/useMenuList.ts index 9846dcff0a377e..2a2c4c4fb14a00 100644 --- a/packages/react-components/react-menu/library/src/components/MenuList/useMenuList.ts +++ b/packages/react-components/react-menu/library/src/components/MenuList/useMenuList.ts @@ -17,6 +17,7 @@ import { useHasParentContext } from '@fluentui/react-context-selector'; import { useMenuContext_unstable } from '../../contexts/menuContext'; import { MenuContext } from '../../contexts/menuContext'; import type { MenuListProps, MenuListState } from './MenuList.types'; +import { useValidateNesting } from '../../utils/useValidateNesting'; /** * Returns the props and state required to render the component @@ -35,6 +36,7 @@ export const useMenuList_unstable = (props: MenuListProps, ref: React.Ref(null); + const validateNestingRef = useValidateNesting('MenuList'); React.useEffect(() => { const element = innerRef.current; @@ -142,7 +144,7 @@ export const useMenuList_unstable = (props: MenuListProps, ref: React.Ref, + ref: useMergedRefs(ref, innerRef, validateNestingRef) as React.Ref, role: 'menu', 'aria-labelledby': menuContext.triggerId, ...focusAttributes, diff --git a/packages/react-components/react-menu/library/src/utils/index.ts b/packages/react-components/react-menu/library/src/utils/index.ts index 06cda69f8f387a..e4f67a40d659e1 100644 --- a/packages/react-components/react-menu/library/src/utils/index.ts +++ b/packages/react-components/react-menu/library/src/utils/index.ts @@ -1,3 +1,4 @@ export { MENU_ENTER_EVENT, dispatchMenuEnterEvent, useOnMenuMouseEnter } from './useOnMenuEnter'; export { useIsSubmenu } from './useIsSubmenu'; +export { useValidateNesting } from './useValidateNesting'; export { MENU_SAFEZONE_TIMEOUT_EVENT, useOnMenuSafeZoneTimeout } from './useOnMenuSafeZoneTimeout'; diff --git a/packages/react-components/react-menu/library/src/utils/useValidateNesting.ts b/packages/react-components/react-menu/library/src/utils/useValidateNesting.ts new file mode 100644 index 00000000000000..677ff7a588f882 --- /dev/null +++ b/packages/react-components/react-menu/library/src/utils/useValidateNesting.ts @@ -0,0 +1,68 @@ +import * as React from 'react'; +import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; + +import type { MenuContextValue } from '../contexts/menuContext'; +import { useMenuContext_unstable } from '../contexts/menuContext'; + +type NestingComponentName = 'MenuList' | 'MenuItem' | 'MenuItemCheckbox' | 'MenuItemRadio'; + +export const useValidateNesting = (componentName: NestingComponentName): React.RefObject => { + 'use no memo'; + + const { targetDocument } = useFluent(); + const triggerRef = useMenuContext_unstable((context: MenuContextValue) => context.triggerRef); + const inline = useMenuContext_unstable((context: MenuContextValue) => context.inline); + const ref = React.useRef(null); + + if (process.env.NODE_ENV !== 'production') { + // This check should run only in development mode + // It's okay to disable the ESLint rule because we ar checking env variable statically (not at runtime) + // eslint-disable-next-line react-hooks/rules-of-hooks + React.useEffect(() => { + let ancestor = ref.current; + let ancestorComponentName = ''; + do { + ancestor = ancestor?.parentElement ?? null; + if (ancestor?.classList.contains('fui-MenuList')) { + break; + } else if (ancestor?.classList.contains('fui-MenuGrid')) { + ancestorComponentName = 'MenuGrid'; + } else if (ancestor?.classList.contains('fui-MenuGridItem')) { + ancestorComponentName = 'MenuGridItem'; + } else if (ancestor?.classList.contains('fui-MenuGridRow')) { + ancestorComponentName = 'MenuGridRow'; + } else if (ancestor?.classList.contains('fui-MenuGridCell')) { + ancestorComponentName = 'MenuGridCell'; + } + if (['MenuItem', 'MenuItemCheckbox', 'MenuItemRadio'].includes(componentName)) { + if (['MenuGrid', 'MenuGridItem', 'MenuGridRow', 'MenuGridCell'].includes(ancestorComponentName)) { + throw new Error( + `${componentName} is incorrectly nested within ${ancestorComponentName}. You probably want to wrap it in a MenuList instead.`, + ); + } + } else if (componentName === 'MenuList') { + if (ancestorComponentName === 'MenuGridCell') { + if (inline && getCellOfTrigger(triggerRef.current, targetDocument) === ancestor) { + break; + } + throw new Error(`MenuList is incorrectly nested within MenuGridCell.`); + } else if (['MenuGrid', 'MenuGridItem', 'MenuGridRow'].includes(ancestorComponentName)) { + throw new Error(`MenuList is incorrectly nested within ${ancestorComponentName}.`); + } + } + } while (ancestor && ancestor !== targetDocument?.body); + }, [componentName, ref, triggerRef, inline, targetDocument]); + } + return ref; +}; + +const getCellOfTrigger = (trigger: HTMLElement | null, targetDocument?: Document): HTMLElement | null => { + let ancestor = trigger?.parentElement; + while (ancestor && ancestor !== targetDocument?.body) { + if (ancestor?.classList.contains('fui-MenuGridCell')) { + return ancestor; + } + ancestor = ancestor?.parentElement ?? null; + } + return null; +};