From 6f135d70360c546aaa2a8209d16abdd1d7e31ed4 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Thu, 4 May 2023 10:46:29 +0200 Subject: [PATCH 1/2] fix: overflowManager should always dispatch initial state Fixes #27656 which was caused by a priority queue edge case. On initial mount the resize observer will always run. If there is already overflow then the tops of the visibility queues will be different from the initial state (all items are considered visible by default). In the cause where there is no overflow initially the queue tops will not change, which does not trigger a dispatch to the react bindings so the `useIsOverflowItemVisible` hook will always return `false` until overflow occurs. The fix is quite simple: set the initial value of flag `forceDispatch` to be `true` which will always trigger a dispatch for initial mount. --- .../priority-overflow/src/overflowManager.ts | 3 +- .../react-overflow/src/Overflow.cy.tsx | 32 +++++++++++++++ .../src/components/Overflow.tsx | 40 ++++++++++++------- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index 15c24ded2c02a..3dece40e10472 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -12,7 +12,8 @@ export function createOverflowManager(): OverflowManager { // Set as true when resize observer is observing let observing = false; // If true, next update will dispatch to onUpdateOverflow even if queue top states don't change - let forceDispatch = false; + // Initially true to force dispatch on first mount + let forceDispatch = true; const options: Required = { padding: 10, overflowAxis: 'horizontal', diff --git a/packages/react-components/react-overflow/src/Overflow.cy.tsx b/packages/react-components/react-overflow/src/Overflow.cy.tsx index 6d427e2ab7bc9..66ee268948f97 100644 --- a/packages/react-components/react-overflow/src/Overflow.cy.tsx +++ b/packages/react-components/react-overflow/src/Overflow.cy.tsx @@ -8,6 +8,7 @@ import { useIsOverflowGroupVisible, useOverflowMenu, useOverflowContext, + useIsOverflowItemVisible, } from '@fluentui/react-overflow'; import { Portal } from '@fluentui/react-portal'; @@ -539,4 +540,35 @@ describe('Overflow', () => { setContainerSize(500); cy.contains('Update priority').click().get('#foo-visibility').should('have.text', 'visible'); }); + + it('Should have correct initial visibility state', () => { + const mapHelper = new Array(10).fill(0).map((_, i) => i); + const Assert = () => { + const isVisible = mapHelper.map(i => { + // eslint-disable-next-line react-hooks/rules-of-hooks + return useIsOverflowItemVisible(i.toString()); + }); + + if (isVisible.every(x => x)) { + return ; + } + + return null; + }; + + mount( + + {mapHelper.map(i => ( + + {i} + + ))} + + + , + ); + + setContainerSize(500); + cy.get('[data-passed="true"]').should('exist'); + }); }); diff --git a/packages/react-components/react-overflow/src/components/Overflow.tsx b/packages/react-components/react-overflow/src/components/Overflow.tsx index 3dbd46e94a6e4..1a599fd3c65f3 100644 --- a/packages/react-components/react-overflow/src/components/Overflow.tsx +++ b/packages/react-components/react-overflow/src/components/Overflow.tsx @@ -7,6 +7,12 @@ import { OverflowContext } from '../overflowContext'; import { updateVisibilityAttribute, useOverflowContainer } from '../useOverflowContainer'; import { useOverflowStyles } from './useOverflowStyles.styles'; +interface OverflowState { + hasOverflow: boolean; + itemVisibility: Record; + groupVisibility: Record; +} + /** * Overflow Props */ @@ -24,21 +30,27 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { const { children, minimumVisible, overflowAxis = 'horizontal', overflowDirection, padding } = props; - const [hasOverflow, setHasOverflow] = React.useState(false); - const [itemVisibility, setItemVisibility] = React.useState>({}); - const [groupVisibility, setGroupVisibility] = React.useState>({}); + const [overflowState, setOverflowState] = React.useState({ + hasOverflow: false, + itemVisibility: {}, + groupVisibility: {}, + }); // useOverflowContainer wraps this method in a useEventCallback. - // TODO: Do we need a useEventCallback here too? const update: OnUpdateOverflow = data => { - setHasOverflow(() => data.invisibleItems.length > 0); - setItemVisibility(() => { - const newState: Record = {}; - data.visibleItems.forEach(x => (newState[x.id] = true)); - data.invisibleItems.forEach(x => (newState[x.id] = false)); - return newState; + const { visibleItems, invisibleItems, groupVisibility } = data; + + const itemVisibility: Record = {}; + visibleItems.forEach(x => (itemVisibility[x.id] = true)); + invisibleItems.forEach(x => (itemVisibility[x.id] = false)); + + setOverflowState(() => { + return { + hasOverflow: data.invisibleItems.length > 0, + itemVisibility, + groupVisibility, + }; }); - setGroupVisibility(data.groupVisibility); }; const { containerRef, registerItem, updateOverflow, registerOverflowMenu } = useOverflowContainer(update, { @@ -57,9 +69,9 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { return ( Date: Thu, 4 May 2023 10:51:11 +0200 Subject: [PATCH 2/2] changefile --- ...rity-overflow-a6802865-2d8b-43e0-8ac7-09dcca1570c8.json | 7 +++++++ ...eact-overflow-3a8651a5-c348-403c-a0a7-eaab0b660b52.json | 7 +++++++ 2 files changed, 14 insertions(+) create mode 100644 change/@fluentui-priority-overflow-a6802865-2d8b-43e0-8ac7-09dcca1570c8.json create mode 100644 change/@fluentui-react-overflow-3a8651a5-c348-403c-a0a7-eaab0b660b52.json diff --git a/change/@fluentui-priority-overflow-a6802865-2d8b-43e0-8ac7-09dcca1570c8.json b/change/@fluentui-priority-overflow-a6802865-2d8b-43e0-8ac7-09dcca1570c8.json new file mode 100644 index 0000000000000..6a7987d6b1e11 --- /dev/null +++ b/change/@fluentui-priority-overflow-a6802865-2d8b-43e0-8ac7-09dcca1570c8.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: overflowManager should always dispatch initial state", + "packageName": "@fluentui/priority-overflow", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-overflow-3a8651a5-c348-403c-a0a7-eaab0b660b52.json b/change/@fluentui-react-overflow-3a8651a5-c348-403c-a0a7-eaab0b660b52.json new file mode 100644 index 0000000000000..442f8805a4480 --- /dev/null +++ b/change/@fluentui-react-overflow-3a8651a5-c348-403c-a0a7-eaab0b660b52.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "refactor: Consolidate all overflow state into one object", + "packageName": "@fluentui/react-overflow", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +}