diff --git a/.changeset/fifty-kids-begin.md b/.changeset/fifty-kids-begin.md new file mode 100644 index 00000000000..e99411e220b --- /dev/null +++ b/.changeset/fifty-kids-begin.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': minor +--- + +Improved test reliability for non-rolled up actions in `ActionMenu` diff --git a/polaris-react/src/components/ActionMenu/components/Actions/Actions.module.scss b/polaris-react/src/components/ActionMenu/components/Actions/Actions.module.scss index 69199b08925..9818a880339 100644 --- a/polaris-react/src/components/ActionMenu/components/Actions/Actions.module.scss +++ b/polaris-react/src/components/ActionMenu/components/Actions/Actions.module.scss @@ -13,6 +13,11 @@ } } +.ActionsLayout--measuring { + visibility: hidden; + height: 0; +} + .ActionsLayoutMeasurer { display: flex; flex-wrap: wrap; diff --git a/polaris-react/src/components/ActionMenu/components/Actions/Actions.tsx b/polaris-react/src/components/ActionMenu/components/Actions/Actions.tsx index 8041ebe89c9..a8f90f5fa0d 100644 --- a/polaris-react/src/components/ActionMenu/components/Actions/Actions.tsx +++ b/polaris-react/src/components/ActionMenu/components/Actions/Actions.tsx @@ -1,4 +1,11 @@ -import React, {useCallback, useRef, useState} from 'react'; +import React, { + useCallback, + useRef, + useState, + useReducer, + useEffect, + useMemo, +} from 'react'; import type { ActionListItemDescriptor, @@ -9,6 +16,7 @@ import type { import {MenuGroup} from '../MenuGroup'; import {useI18n} from '../../../../utilities/i18n'; import {SecondaryAction} from '../SecondaryAction'; +import {classNames} from '../../../../utilities/css'; import styles from './Actions.module.scss'; import type {ActionsMeasurements} from './components'; @@ -23,28 +31,50 @@ interface Props { /** Callback that returns true when secondary actions are rolled up into action groups, and false when not */ onActionRollup?(hasRolledUp: boolean): void; } -interface MeasuredActionsIndices { +interface ActionsState { visibleActions: number[]; hiddenActions: number[]; visibleGroups: number[]; hiddenGroups: number[]; + actionsWidths: number[]; + containerWidth: number; + disclosureWidth: number; + hasMeasured: boolean; } -export function Actions({actions = [], groups = [], onActionRollup}: Props) { +export function Actions({actions, groups, onActionRollup}: Props) { const i18n = useI18n(); const rollupActiveRef = useRef(null); const [activeMenuGroup, setActiveMenuGroup] = useState( undefined, ); - const [ - {visibleActions, hiddenActions, visibleGroups, hiddenGroups}, - setMeasuredActionsIndices, - ] = useState({ - visibleActions: [], - hiddenActions: [], - visibleGroups: [], - hiddenGroups: [], - }); + + const [state, setState] = useReducer( + (data: ActionsState, partialData: Partial): ActionsState => { + return {...data, ...partialData}; + }, + { + disclosureWidth: 0, + containerWidth: Infinity, + actionsWidths: [], + visibleActions: [], + hiddenActions: [], + visibleGroups: [], + hiddenGroups: [], + hasMeasured: false, + }, + ); + + const { + visibleActions, + hiddenActions, + visibleGroups, + hiddenGroups, + containerWidth, + disclosureWidth, + actionsWidths, + hasMeasured, + } = state; const defaultRollupGroup: MenuGroupDescriptor = { title: i18n.translate('Polaris.ActionMenu.Actions.moreActions'), @@ -61,27 +91,62 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) { [], ); - const actionsMarkup = actions.map((action, index) => { - if (!visibleActions.includes(index)) { - return null; + useEffect(() => { + if (containerWidth === 0) { + return; } + const {visibleActions, visibleGroups, hiddenActions, hiddenGroups} = + getVisibleAndHiddenActionsIndices( + actions, + groups, + disclosureWidth, + actionsWidths, + containerWidth, + ); + setState({ + visibleActions, + visibleGroups, + hiddenActions, + hiddenGroups, + hasMeasured: containerWidth !== Infinity, + }); + }, [ + containerWidth, + disclosureWidth, + actions, + groups, + actionsWidths, + setState, + ]); - const {content, onAction, ...rest} = action; + const actionsOrDefault = useMemo(() => actions ?? [], [actions]); + const groupsOrDefault = useMemo(() => groups ?? [], [groups]); - return ( - - {content} - - ); - }); + const actionsMarkup = actionsOrDefault + .filter((_, index) => { + if (!visibleActions.includes(index)) { + return false; + } + + return true; + }) + .map((action) => { + const {content, onAction, ...rest} = action; + + return ( + + {content} + + ); + }); const groupsToFilter = hiddenGroups.length > 0 || hiddenActions.length > 0 - ? [...groups, defaultRollupGroup] - : [...groups]; + ? [...groupsOrDefault, defaultRollupGroup] + : [...groupsOrDefault]; const filteredGroups = groupsToFilter.filter((group, index) => { - const hasNoGroupsProp = groups.length === 0; + const hasNoGroupsProp = groupsOrDefault.length === 0; const isVisibleGroup = visibleGroups.includes(index); const isDefaultGroup = group === defaultRollupGroup; @@ -96,8 +161,12 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) { return isVisibleGroup; }); - const hiddenActionObjects = hiddenActions.map((index) => actions[index]); - const hiddenGroupObjects = hiddenGroups.map((index) => groups[index]); + const hiddenActionObjects = hiddenActions.map( + (index) => actionsOrDefault[index], + ); + const hiddenGroupObjects = hiddenGroups.map( + (index) => groupsOrDefault[index], + ); const groupsMarkup = filteredGroups.map((group) => { const {title, actions: groupActions, ...rest} = group; @@ -160,8 +229,8 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) { const {visibleActions, hiddenActions, visibleGroups, hiddenGroups} = getVisibleAndHiddenActionsIndices( - actions, - groups, + actionsOrDefault, + groupsOrDefault, disclosureWidth, actionsWidths, containerWidth, @@ -175,14 +244,18 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) { rollupActiveRef.current = isRollupActive; } } - setMeasuredActionsIndices({ + setState({ visibleActions, hiddenActions, visibleGroups, hiddenGroups, + actionsWidths, + containerWidth, + disclosureWidth, + hasMeasured: true, }); }, - [actions, groups, onActionRollup], + [actionsOrDefault, groupsOrDefault, onActionRollup], ); const actionsMeasurer = ( @@ -196,7 +269,12 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) { return (
{actionsMeasurer} -
+
{actionsMarkup} {groupsMarkup}
diff --git a/polaris-react/src/components/ActionMenu/components/Actions/components/ActionsMeasurer/ActionsMeasurer.tsx b/polaris-react/src/components/ActionMenu/components/Actions/components/ActionsMeasurer/ActionsMeasurer.tsx index 68d6622de16..872b3fbdb57 100644 --- a/polaris-react/src/components/ActionMenu/components/Actions/components/ActionsMeasurer/ActionsMeasurer.tsx +++ b/polaris-react/src/components/ActionMenu/components/Actions/components/ActionsMeasurer/ActionsMeasurer.tsx @@ -4,7 +4,6 @@ import type { MenuActionDescriptor, MenuGroupDescriptor, } from '../../../../../../types'; -import {useComponentDidMount} from '../../../../../../utilities/use-component-did-mount'; import {useI18n} from '../../../../../../utilities/i18n'; import {SecondaryAction} from '../../../SecondaryAction'; import {useEventListener} from '../../../../../../utilities/use-event-listener'; @@ -33,7 +32,6 @@ export function ActionsMeasurer({ }: ActionsMeasurerProps) { const i18n = useI18n(); const containerNode = useRef(null); - const animationFrame = useRef(null); const defaultRollupGroup: MenuGroupDescriptor = { title: i18n.translate('Polaris.ActionMenu.Actions.moreActions'), @@ -45,29 +43,23 @@ export function ActionsMeasurer({ ); const handleMeasurement = useCallback(() => { - if (animationFrame.current) { - cancelAnimationFrame(animationFrame.current); + if (!containerNode.current) { + return; } - animationFrame.current = requestAnimationFrame(() => { - if (!containerNode.current) { - return; - } - - const containerWidth = containerNode.current.offsetWidth; - const hiddenActionNodes = containerNode.current.children; - const hiddenActionNodesArray = Array.from(hiddenActionNodes); - const hiddenActionsWidths = hiddenActionNodesArray.map((node) => { - const buttonWidth = Math.ceil(node.getBoundingClientRect().width); - return buttonWidth + ACTION_SPACING; - }); - const disclosureWidth = hiddenActionsWidths.pop() || 0; - - handleMeasurementProp({ - containerWidth, - disclosureWidth, - hiddenActionsWidths, - }); + const containerWidth = containerNode.current.offsetWidth; + const hiddenActionNodes = containerNode.current.children; + const hiddenActionNodesArray = Array.from(hiddenActionNodes); + const hiddenActionsWidths = hiddenActionNodesArray.map((node) => { + const buttonWidth = Math.ceil(node.getBoundingClientRect().width); + return buttonWidth + ACTION_SPACING; + }); + const disclosureWidth = hiddenActionsWidths.pop() || 0; + + handleMeasurementProp({ + containerWidth, + disclosureWidth, + hiddenActionsWidths, }); }, [handleMeasurementProp]); @@ -75,12 +67,6 @@ export function ActionsMeasurer({ handleMeasurement(); }, [handleMeasurement, actions, groups]); - useComponentDidMount(() => { - if (process.env.NODE_ENV === 'development') { - setTimeout(handleMeasurement, 0); - } - }); - const actionsMarkup = actions.map((action) => { const {content, onAction, ...rest} = action; diff --git a/polaris-react/src/components/ActionMenu/components/Actions/tests/Actions.test.tsx b/polaris-react/src/components/ActionMenu/components/Actions/tests/Actions.test.tsx index 30d579d4198..15991552578 100644 --- a/polaris-react/src/components/ActionMenu/components/Actions/tests/Actions.test.tsx +++ b/polaris-react/src/components/ActionMenu/components/Actions/tests/Actions.test.tsx @@ -8,6 +8,7 @@ import {Actions, MenuGroup, RollupActions, SecondaryAction} from '../..'; import {Tooltip} from '../../../../Tooltip'; import type {getVisibleAndHiddenActionsIndices} from '../utilities'; import {ActionsMeasurer} from '../components'; +import styles from '../Actions.module.scss'; jest.mock('../components/ActionsMeasurer', () => ({ ActionsMeasurer() { @@ -71,9 +72,11 @@ describe('', () => { , ); + const wrappingDiv = findWrapper(wrapper); + forceMeasurement(wrapper); - expect(wrapper.findAll(SecondaryAction)).toHaveLength(3); + expect(wrappingDiv!.findAll(SecondaryAction)).toHaveLength(3); }); it('renders a when helpText is set on an action', () => { @@ -233,6 +236,18 @@ describe('', () => { }); }); +function findWrapper(wrapper: CustomRoot) { + const wrappingDiv = wrapper.findWhere<'div'>((node) => { + return ( + node.is('div') && + Boolean(node.prop('className')) && + node.prop('className')!.includes(styles.ActionsLayout) + ); + }); + + return wrappingDiv; +} + function forceMeasurement(wrapper: CustomRoot) { wrapper.act(() => { wrapper.find(ActionsMeasurer)!.trigger('handleMeasurement', { diff --git a/polaris-react/src/components/ActionMenu/components/Actions/utilities.ts b/polaris-react/src/components/ActionMenu/components/Actions/utilities.ts index 3ddfefbdc57..f28dc8e48c4 100644 --- a/polaris-react/src/components/ActionMenu/components/Actions/utilities.ts +++ b/polaris-react/src/components/ActionMenu/components/Actions/utilities.ts @@ -1,6 +1,6 @@ export function getVisibleAndHiddenActionsIndices( - actions: any[], - groups: any[], + actions: any[] = [], + groups: any[] = [], disclosureWidth: number, actionsWidths: number[], containerWidth: number, diff --git a/polaris-react/src/components/Page/Page.stories.tsx b/polaris-react/src/components/Page/Page.stories.tsx index 328a937f2d5..7d0da252a16 100644 --- a/polaris-react/src/components/Page/Page.stories.tsx +++ b/polaris-react/src/components/Page/Page.stories.tsx @@ -1,4 +1,4 @@ -import React, {useState, useCallback} from 'react'; +import React from 'react'; import type {ComponentMeta} from '@storybook/react'; import { DeleteIcon,