From 7f8e087aa2470ae23e2bb0f70b2307855783cd13 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 31 Jan 2023 18:15:37 +0100 Subject: [PATCH 1/7] fix: New overflow items might not dispatch updates The dispatch process is done by analyzing the tops of the visibile and invisible queues. The update triggered after adding a new element might not be dispatched to the subscriber because the queue tops don't change. This can result in a desync between the visibility and the subscriber state. Adds a `forceDispatch` flag which is used when new items are added while observing so that the next batched update will flush to subscribers even if queue tops don't change. Fixes #23555 --- .../priority-overflow/src/overflowManager.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index 0b710e42ccd930..65dda2a1119e36 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -9,7 +9,10 @@ import type { OverflowGroupState, OverflowItemEntry, OverflowManager, ObserveOpt export function createOverflowManager(): OverflowManager { let container: HTMLElement | undefined; let overflowMenu: HTMLElement | undefined; + // 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; const options: Required = { padding: 10, overflowAxis: 'horizontal', @@ -158,7 +161,8 @@ export function createOverflowManager(): OverflowManager { } // only update when the state of visible/invisible items has changed - if (visibleItemQueue.peek() !== visibleTop || invisibleItemQueue.peek() !== invisibleTop) { + if (visibleItemQueue.peek() !== visibleTop || invisibleItemQueue.peek() !== invisibleTop || forceDispatch) { + forceDispatch = false; dispatchOverflowUpdate(); } }; @@ -197,6 +201,10 @@ export function createOverflowManager(): OverflowManager { // some options can affect priority which are only set on `observe` if (observing) { + // Updates to elements might not change the queue tops + // i.e. new element is enqueued but the top of the queue stays the same + // force a dispatch on the next batched update + forceDispatch = true; visibleItemQueue.enqueue(item.id); } From e6104eb36e5accb2843027be22df268bd0b8ba16 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 31 Jan 2023 18:19:37 +0100 Subject: [PATCH 2/7] changefile --- ...rity-overflow-06e37a2f-4190-4541-9eba-5d513a0200da.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-priority-overflow-06e37a2f-4190-4541-9eba-5d513a0200da.json diff --git a/change/@fluentui-priority-overflow-06e37a2f-4190-4541-9eba-5d513a0200da.json b/change/@fluentui-priority-overflow-06e37a2f-4190-4541-9eba-5d513a0200da.json new file mode 100644 index 00000000000000..0e4eb034956e72 --- /dev/null +++ b/change/@fluentui-priority-overflow-06e37a2f-4190-4541-9eba-5d513a0200da.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: New overflow items all always dispatch updates to subscriber", + "packageName": "@fluentui/priority-overflow", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} From a72bdf05cb9e950236fdb03ac5d1733ef989f9a6 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 1 Feb 2023 10:53:43 +0100 Subject: [PATCH 3/7] Add tests --- .../react-overflow/etc/react-overflow.api.md | 4 ++ .../react-overflow/src/Overflow.cy.tsx | 67 ++++++++++++++++++- .../react-overflow/src/index.ts | 2 + .../react-overflow/src/overflowContext.ts | 6 ++ 4 files changed, 76 insertions(+), 3 deletions(-) diff --git a/packages/react-components/react-overflow/etc/react-overflow.api.md b/packages/react-components/react-overflow/etc/react-overflow.api.md index 37adf79a6dd5a3..23ccaca79cc5a6 100644 --- a/packages/react-components/react-overflow/etc/react-overflow.api.md +++ b/packages/react-components/react-overflow/etc/react-overflow.api.md @@ -4,6 +4,7 @@ ```ts +import { ContextSelector } from '@fluentui/react-context-selector'; import type { ObserveOptions } from '@fluentui/priority-overflow'; import type { OnUpdateOverflow } from '@fluentui/priority-overflow'; import { OverflowGroupState } from '@fluentui/priority-overflow'; @@ -54,6 +55,9 @@ export interface UseOverflowContainerReturn extend containerRef: React_2.RefObject; } +// @internal (undocumented) +export const useOverflowContext: (selector: ContextSelector) => SelectedValue; + // @public (undocumented) export const useOverflowCount: () => number; diff --git a/packages/react-components/react-overflow/src/Overflow.cy.tsx b/packages/react-components/react-overflow/src/Overflow.cy.tsx index 2aca539bdb9b61..8f3311a5978b7a 100644 --- a/packages/react-components/react-overflow/src/Overflow.cy.tsx +++ b/packages/react-components/react-overflow/src/Overflow.cy.tsx @@ -7,7 +7,9 @@ import { OverflowProps, useIsOverflowGroupVisible, useOverflowMenu, + useOverflowContext, } from '@fluentui/react-overflow'; +import { Portal } from '@fluentui/react-portal'; const selectors = { container: 'data-test-container', @@ -72,6 +74,7 @@ const Item: React.FC<{ children?: React.ReactNode; width?: number } & Omit = ({ width }) => { const { isOverflowing, ref, overflowCount } = useOverflowMenu(); + const itemVisibility = useOverflowContext(ctx => ctx.itemVisibility); const selector = { [selectors.menu]: '', }; @@ -82,9 +85,21 @@ const Menu: React.FC<{ width?: number }> = ({ width }) => { // No need to actually render a menu, we're testing state return ( - + <> + + +
+ {Object.entries(itemVisibility).map(([id, visible]) => ( + <> +
{id}
+
{visible ? 'visible' : 'invisible'}
+ + ))} +
+ + ); }; @@ -452,4 +467,50 @@ describe('Overflow', () => { cy.get(`[${selectors.menu}]`).should('not.be.visible'); cy.get(`[${selectors.item}="4"]`).should('not.be.visible'); }); + + it('should dispatch update when updating priority of an item', () => { + const Example = () => { + const [priority, setPriority] = React.useState(); + + return ( + <> + + + Today + + + Agenda + + + Day + + + Three Day + + + Work Week + + + Week + + + Month + + + Search + + + Conversations + + + + + + ); + }; + mount(); + + setContainerSize(520); + cy.contains('Update priority').click().get('#week-visibility').should('have.text', 'visible'); + }); }); diff --git a/packages/react-components/react-overflow/src/index.ts b/packages/react-components/react-overflow/src/index.ts index 46b1aee8a99049..5466d7c6a93a36 100644 --- a/packages/react-components/react-overflow/src/index.ts +++ b/packages/react-components/react-overflow/src/index.ts @@ -9,5 +9,7 @@ export { useOverflowCount } from './useOverflowCount'; export { useOverflowItem } from './useOverflowItem'; export { useOverflowMenu } from './useOverflowMenu'; +export { useOverflowContext } from './overflowContext'; + export type { OverflowItemProps } from './components/OverflowItem/OverflowItem.types'; export { OverflowItem } from './components/OverflowItem/OverflowItem'; diff --git a/packages/react-components/react-overflow/src/overflowContext.ts b/packages/react-components/react-overflow/src/overflowContext.ts index 9ced465a74fc0c..87a53f791f9c27 100644 --- a/packages/react-components/react-overflow/src/overflowContext.ts +++ b/packages/react-components/react-overflow/src/overflowContext.ts @@ -1,6 +1,9 @@ import type { OverflowGroupState, OverflowItemEntry } from '@fluentui/priority-overflow'; import { ContextSelector, createContext, useContextSelector, Context } from '@fluentui/react-context-selector'; +/** + * @internal + */ export interface OverflowContextValue { itemVisibility: Record; groupVisibility: Record; @@ -23,5 +26,8 @@ const overflowContextDefaultValue: OverflowContextValue = { registerOverflowMenu: () => () => null, }; +/** + * @internal + */ export const useOverflowContext = (selector: ContextSelector) => useContextSelector(OverflowContext, (ctx = overflowContextDefaultValue) => selector(ctx)); From d536ff57b712aeb7b4621088ecdd0335750061e4 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 1 Feb 2023 10:54:51 +0100 Subject: [PATCH 4/7] changefile --- ...eact-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json | 6 ++++++ ...eact-overflow-8b8469a8-30b4-4a4d-bc67-d168fd9e2362.json | 7 +++++++ 2 files changed, 13 insertions(+) create mode 100644 change/@fluentui-react-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json create mode 100644 change/@fluentui-react-overflow-8b8469a8-30b4-4a4d-bc67-d168fd9e2362.json diff --git a/change/@fluentui-react-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json b/change/@fluentui-react-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json new file mode 100644 index 00000000000000..7e66358c8c7d11 --- /dev/null +++ b/change/@fluentui-react-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json @@ -0,0 +1,6 @@ +{ + "type": "patch", + "packageName": "@fluentui/react-overflow", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-overflow-8b8469a8-30b4-4a4d-bc67-d168fd9e2362.json b/change/@fluentui-react-overflow-8b8469a8-30b4-4a4d-bc67-d168fd9e2362.json new file mode 100644 index 00000000000000..341e2d425ee83a --- /dev/null +++ b/change/@fluentui-react-overflow-8b8469a8-30b4-4a4d-bc67-d168fd9e2362.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "Export useOverflowContext as @internal for testing purposes", + "packageName": "@fluentui/react-overflow", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "none" +} From efe9b191cda7d7097a0cdbaeddfdcaa2b2cd2918 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 1 Feb 2023 10:55:57 +0100 Subject: [PATCH 5/7] remoive old changefile --- ...react-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 change/@fluentui-react-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json diff --git a/change/@fluentui-react-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json b/change/@fluentui-react-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json deleted file mode 100644 index 7e66358c8c7d11..00000000000000 --- a/change/@fluentui-react-overflow-7c228aac-d8dc-4651-96ce-3b3b4b504c0f.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "type": "patch", - "packageName": "@fluentui/react-overflow", - "email": "lingfangao@hotmail.com", - "dependentChangeType": "patch" -} From 05ba4054c99cb5f68c927845dec2cdca7be2c1e7 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Thu, 2 Feb 2023 09:59:58 +0100 Subject: [PATCH 6/7] refactor processOverflowItems --- .../priority-overflow/src/overflowManager.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index 65dda2a1119e36..15c24ded2c02af 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -121,11 +121,12 @@ export function createOverflowManager(): OverflowManager { options.onUpdateOverflow({ visibleItems, invisibleItems, groupVisibility }); }; - const processOverflowItems = (availableSize: number) => { + const processOverflowItems = (): boolean => { if (!container) { - return; + return false; } + const availableSize = getOffsetSize(container) - options.padding; const overflowMenuOffset = overflowMenu ? getOffsetSize(overflowMenu) : 0; // Snapshot of the visible/invisible state to compare for updates @@ -161,19 +162,18 @@ export function createOverflowManager(): OverflowManager { } // only update when the state of visible/invisible items has changed - if (visibleItemQueue.peek() !== visibleTop || invisibleItemQueue.peek() !== invisibleTop || forceDispatch) { - forceDispatch = false; - dispatchOverflowUpdate(); + if (visibleItemQueue.peek() !== visibleTop || invisibleItemQueue.peek() !== invisibleTop) { + return true; } + + return false; }; const forceUpdate: OverflowManager['forceUpdate'] = () => { - if (!container) { - return; + if (processOverflowItems() || forceDispatch) { + forceDispatch = false; + dispatchOverflowUpdate(); } - - const availableSize = getOffsetSize(container) - options.padding; - processOverflowItems(availableSize); }; const update: OverflowManager['update'] = debounce(forceUpdate); From 40e8623bde840f99f591b79e23eacb0767166ddf Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Thu, 2 Feb 2023 10:37:40 +0100 Subject: [PATCH 7/7] make test cleaner --- .../react-overflow/src/Overflow.cy.tsx | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/packages/react-components/react-overflow/src/Overflow.cy.tsx b/packages/react-components/react-overflow/src/Overflow.cy.tsx index 8f3311a5978b7a..c2ae2986de316e 100644 --- a/packages/react-components/react-overflow/src/Overflow.cy.tsx +++ b/packages/react-components/react-overflow/src/Overflow.cy.tsx @@ -468,6 +468,10 @@ describe('Overflow', () => { cy.get(`[${selectors.item}="4"]`).should('not.be.visible'); }); + // Container will fit 4 items with width 100 and the overflow menu + // Once the priority of the foo item is updated it should have more + // priority than Item 5 (width 100 item), so the foo item should be visible + // once this foo item is visible, make sure that the subscriber is in sync. it('should dispatch update when updating priority of an item', () => { const Example = () => { const [priority, setPriority] = React.useState(); @@ -475,32 +479,29 @@ describe('Overflow', () => { return ( <> - - Today + + Item 1 - - Agenda + + Item 2 - - Day + + Item 3 - - Three Day + + Item 4 - - Work Week + + Item 5 - - Week + + Foo - - Month + + Item - - Search - - - Conversations + + Item @@ -510,7 +511,7 @@ describe('Overflow', () => { }; mount(); - setContainerSize(520); - cy.contains('Update priority').click().get('#week-visibility').should('have.text', 'visible'); + setContainerSize(500); + cy.contains('Update priority').click().get('#foo-visibility').should('have.text', 'visible'); }); });