From 4047352430c217d67ea640af9dbc8da693bf8cd1 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 5 Oct 2022 14:33:55 +0200 Subject: [PATCH 1/7] feat: Overflow menu should be registered in overflowManager Fixes #25090 The overflow manager treats the overflow menu like any other overflow item. This causes the unfortunate bug that there is no way to whether the last invisible item can simply replace the overflow menu. Adds special treatment to the overflow menu so it is a special kind of item in the overflow manager. Updates `processOverflow` so that there is a special case at the end to check for the case that the overflow menu can be replaced with the last overflow item. --- .../priority-overflow/src/overflowManager.ts | 25 +++++++++++++++++++ .../priority-overflow/src/priorityQueue.ts | 5 ++-- .../priority-overflow/src/types.ts | 11 ++++++++ .../src/components/Overflow.tsx | 3 ++- .../react-overflow/src/overflowContext.ts | 2 ++ .../Overflow/ReverseDomOrder.stories.tsx | 2 +- .../react-overflow/src/types.ts | 11 ++------ .../src/useOverflowContainer.ts | 16 +++++++++++- .../react-overflow/src/useOverflowMenu.ts | 25 ++++++++----------- 9 files changed, 71 insertions(+), 29 deletions(-) diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index c853f914feaebb..a811bb1dbec2e3 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -8,6 +8,7 @@ import type { OverflowGroupState, OverflowItemEntry, OverflowManager, ObserveOpt */ export function createOverflowManager(): OverflowManager { let container: HTMLElement | undefined; + let overflowMenu: HTMLElement | undefined; const options: Required = { padding: 10, overflowAxis: 'horizontal', @@ -121,6 +122,8 @@ export function createOverflowManager(): OverflowManager { return; } + const overflowMenuSize = overflowMenu ? getOffsetSize(overflowMenu) : 0; + // Snapshot of the visible/invisible state to compare for updates const visibleTop = visibleItemQueue.peek(); const invisibleTop = invisibleItemQueue.peek(); @@ -143,6 +146,10 @@ export function createOverflowManager(): OverflowManager { currentWidth -= makeItemInvisible(); } + if (invisibleItemQueue.size() > 0 && currentWidth + overflowMenuSize > availableSize) { + makeItemInvisible(); + } + // only update when the state of visible/invisible items has changed if (visibleItemQueue.peek() !== visibleTop || invisibleItemQueue.peek() !== invisibleTop) { dispatchOverflowUpdate(); @@ -171,6 +178,10 @@ export function createOverflowManager(): OverflowManager { }; const addItem: OverflowManager['addItem'] = item => { + if (overflowItems[item.id]) { + return; + } + overflowItems[item.id] = item; visibleItemQueue.enqueue(item.id); @@ -188,7 +199,19 @@ export function createOverflowManager(): OverflowManager { update(); }; + const addOverflowMenu: OverflowManager['addOverflowMenu'] = el => { + overflowMenu = el; + }; + + const removeOverflowMenu: OverflowManager['removeOverflowMenu'] = () => { + overflowMenu = undefined; + }; + const removeItem: OverflowManager['removeItem'] = itemId => { + if (!overflowItems[itemId]) { + return; + } + const item = overflowItems[itemId]; visibleItemQueue.remove(itemId); invisibleItemQueue.remove(itemId); @@ -209,5 +232,7 @@ export function createOverflowManager(): OverflowManager { observe, removeItem, update, + addOverflowMenu, + removeOverflowMenu, }; } diff --git a/packages/react-components/priority-overflow/src/priorityQueue.ts b/packages/react-components/priority-overflow/src/priorityQueue.ts index 5a727f43d82ff3..719559586eb00f 100644 --- a/packages/react-components/priority-overflow/src/priorityQueue.ts +++ b/packages/react-components/priority-overflow/src/priorityQueue.ts @@ -88,13 +88,14 @@ export function createPriorityQueue(compare: PriorityQueueCompareFn): Prio }; const contains = (item: T) => { - return arr.indexOf(item) >= 0; + const index = arr.indexOf(item); + return index >= 0 && index < size; }; const remove = (item: T) => { const i = arr.indexOf(item); - if (i === -1) { + if (i === -1 || i >= size) { return; } diff --git a/packages/react-components/priority-overflow/src/types.ts b/packages/react-components/priority-overflow/src/types.ts index f5c5f66d5f9c7b..75cb10d193a937 100644 --- a/packages/react-components/priority-overflow/src/types.ts +++ b/packages/react-components/priority-overflow/src/types.ts @@ -104,4 +104,15 @@ export interface OverflowManager { * Manually update the overflow sync */ forceUpdate: () => void; + + /** + * Adds an element that opens an overflow menu. This is used to calculate + * available space and check if additional items need to overflow + */ + addOverflowMenu: (element: HTMLElement) => void; + + /** + * Unsets the overflow menu element + */ + removeOverflowMenu: () => void; } diff --git a/packages/react-components/react-overflow/src/components/Overflow.tsx b/packages/react-components/react-overflow/src/components/Overflow.tsx index 2afd461950d926..9bf0962eb1859b 100644 --- a/packages/react-components/react-overflow/src/components/Overflow.tsx +++ b/packages/react-components/react-overflow/src/components/Overflow.tsx @@ -55,7 +55,7 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { setGroupVisibility(data.groupVisibility); }; - const { containerRef, registerItem, updateOverflow } = useOverflowContainer(update, { + const { containerRef, registerItem, updateOverflow, registerOverflowMenu } = useOverflowContainer(update, { overflowDirection, overflowAxis, padding, @@ -76,6 +76,7 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => { hasOverflow, registerItem, updateOverflow, + registerOverflowMenu, }} > {clonedChild} diff --git a/packages/react-components/react-overflow/src/overflowContext.ts b/packages/react-components/react-overflow/src/overflowContext.ts index ab6a7a50bcc0c1..9ced465a74fc0c 100644 --- a/packages/react-components/react-overflow/src/overflowContext.ts +++ b/packages/react-components/react-overflow/src/overflowContext.ts @@ -6,6 +6,7 @@ export interface OverflowContextValue { groupVisibility: Record; hasOverflow: boolean; registerItem: (item: OverflowItemEntry) => () => void; + registerOverflowMenu: (el: HTMLElement) => () => void; updateOverflow: (padding?: number) => void; } @@ -19,6 +20,7 @@ const overflowContextDefaultValue: OverflowContextValue = { hasOverflow: false, registerItem: () => () => null, updateOverflow: () => null, + registerOverflowMenu: () => () => null, }; export const useOverflowContext = (selector: ContextSelector) => diff --git a/packages/react-components/react-overflow/src/stories/Overflow/ReverseDomOrder.stories.tsx b/packages/react-components/react-overflow/src/stories/Overflow/ReverseDomOrder.stories.tsx index d3a33b6e872f9b..711153093debd5 100644 --- a/packages/react-components/react-overflow/src/stories/Overflow/ReverseDomOrder.stories.tsx +++ b/packages/react-components/react-overflow/src/stories/Overflow/ReverseDomOrder.stories.tsx @@ -20,10 +20,10 @@ export const ReverseDomOrder = () => { return (
+ {itemIds.map(i => ( ))} -
); diff --git a/packages/react-components/react-overflow/src/types.ts b/packages/react-components/react-overflow/src/types.ts index 1b8698e72ae01f..45a60daa71415d 100644 --- a/packages/react-components/react-overflow/src/types.ts +++ b/packages/react-components/react-overflow/src/types.ts @@ -4,17 +4,10 @@ import { OverflowContextValue } from './overflowContext'; /** * @internal */ -export interface UseOverflowContainerReturn { +export interface UseOverflowContainerReturn + extends Pick { /** * Ref to apply to the container that will overflow */ containerRef: React.RefObject; - /** - * Registers and overflow item - */ - registerItem: OverflowContextValue['registerItem']; - /** - * Imperative function to trigger overflow update - */ - updateOverflow: OverflowContextValue['updateOverflow']; } diff --git a/packages/react-components/react-overflow/src/useOverflowContainer.ts b/packages/react-components/react-overflow/src/useOverflowContainer.ts index 49172f91155636..9eb55e4b5ac368 100644 --- a/packages/react-components/react-overflow/src/useOverflowContainer.ts +++ b/packages/react-components/react-overflow/src/useOverflowContainer.ts @@ -13,7 +13,7 @@ import type { } from '@fluentui/priority-overflow'; import { canUseDOM, useEventCallback, useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; import { UseOverflowContainerReturn } from './types'; -import { DATA_OVERFLOWING, DATA_OVERFLOW_ITEM } from './constants'; +import { DATA_OVERFLOWING, DATA_OVERFLOW_ITEM, DATA_OVERFLOW_MENU } from './constants'; /** * @internal @@ -82,10 +82,24 @@ export const useOverflowContainer = ( overflowManager?.update(); }, [overflowManager]); + const registerOverflowMenu = React.useCallback( + (el: HTMLElement) => { + overflowManager?.addOverflowMenu(el); + el.setAttribute(DATA_OVERFLOW_MENU, ''); + + return () => { + overflowManager?.removeOverflowMenu(); + el.removeAttribute(DATA_OVERFLOW_MENU); + }; + }, + [overflowManager], + ); + return { containerRef, registerItem, updateOverflow, + registerOverflowMenu, }; }; diff --git a/packages/react-components/react-overflow/src/useOverflowMenu.ts b/packages/react-components/react-overflow/src/useOverflowMenu.ts index d4620128fbfac7..9007489565f1b8 100644 --- a/packages/react-components/react-overflow/src/useOverflowMenu.ts +++ b/packages/react-components/react-overflow/src/useOverflowMenu.ts @@ -2,32 +2,27 @@ import * as React from 'react'; import { useId, useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; import { useOverflowContext } from './overflowContext'; import { useOverflowCount } from './useOverflowCount'; -import { DATA_OVERFLOW_MENU } from './constants'; export function useOverflowMenu(id?: string) { const elementId = useId('overflow-menu', id); const overflowCount = useOverflowCount(); - const registerItem = useOverflowContext(v => v.registerItem); + const registerOverflowMenu = useOverflowContext(v => v.registerOverflowMenu); const updateOverflow = useOverflowContext(v => v.updateOverflow); const ref = React.useRef(null); + const offsetRef = React.useRef(0); const isOverflowing = overflowCount > 0; useIsomorphicLayoutEffect(() => { const element = ref.current; - if (element) { - const deregisterItem = registerItem({ - element, - id: elementId, - priority: Infinity, - }); - element.setAttribute(DATA_OVERFLOW_MENU, ''); - - return () => { - deregisterItem(); - element?.removeAttribute(DATA_OVERFLOW_MENU); - }; + if (!element || !isOverflowing) { + offsetRef.current = 0; + return; } - }, [registerItem, isOverflowing, elementId]); + + offsetRef.current = 1; + + return registerOverflowMenu(element); + }, [registerOverflowMenu, isOverflowing, elementId]); useIsomorphicLayoutEffect(() => { if (isOverflowing) { From 9186714a2fff30829c791e553a873e02d9dfc92b Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 5 Oct 2022 14:38:28 +0200 Subject: [PATCH 2/7] changefile --- ...rity-overflow-8e5a994e-3ec2-4be9-ba5b-9e39c4a8edc6.json | 7 +++++++ ...eact-overflow-18c283fb-d822-40ba-bc1c-d6ff3844fc6b.json | 7 +++++++ 2 files changed, 14 insertions(+) create mode 100644 change/@fluentui-priority-overflow-8e5a994e-3ec2-4be9-ba5b-9e39c4a8edc6.json create mode 100644 change/@fluentui-react-overflow-18c283fb-d822-40ba-bc1c-d6ff3844fc6b.json diff --git a/change/@fluentui-priority-overflow-8e5a994e-3ec2-4be9-ba5b-9e39c4a8edc6.json b/change/@fluentui-priority-overflow-8e5a994e-3ec2-4be9-ba5b-9e39c4a8edc6.json new file mode 100644 index 00000000000000..1edf53fe40204c --- /dev/null +++ b/change/@fluentui-priority-overflow-8e5a994e-3ec2-4be9-ba5b-9e39c4a8edc6.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "feat: Adds API to register overflow menus for better available space calculation", + "packageName": "@fluentui/priority-overflow", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-overflow-18c283fb-d822-40ba-bc1c-d6ff3844fc6b.json b/change/@fluentui-react-overflow-18c283fb-d822-40ba-bc1c-d6ff3844fc6b.json new file mode 100644 index 00000000000000..16176e6d2d8f4d --- /dev/null +++ b/change/@fluentui-react-overflow-18c283fb-d822-40ba-bc1c-d6ff3844fc6b.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "fix: useOverflowMenu should register overflow menu", + "packageName": "@fluentui/react-overflow", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} From 66febd4488c46a030dcfc155d77abc4117a99d85 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 5 Oct 2022 14:39:15 +0200 Subject: [PATCH 3/7] update md --- .../priority-overflow/etc/priority-overflow.api.md | 2 ++ .../react-components/react-overflow/etc/react-overflow.api.md | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-components/priority-overflow/etc/priority-overflow.api.md b/packages/react-components/priority-overflow/etc/priority-overflow.api.md index 6ecdc4d372dcb8..a8a2ef84d50ce2 100644 --- a/packages/react-components/priority-overflow/etc/priority-overflow.api.md +++ b/packages/react-components/priority-overflow/etc/priority-overflow.api.md @@ -62,10 +62,12 @@ export interface OverflowItemEntry { // @internal (undocumented) export interface OverflowManager { addItem: (items: OverflowItemEntry) => void; + addOverflowMenu: (element: HTMLElement) => void; disconnect: () => void; forceUpdate: () => void; observe: (container: HTMLElement, options: ObserveOptions) => void; removeItem: (itemId: string) => void; + removeOverflowMenu: () => void; update: () => void; } 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 09fab788d7190b..37adf79a6dd5a3 100644 --- a/packages/react-components/react-overflow/etc/react-overflow.api.md +++ b/packages/react-components/react-overflow/etc/react-overflow.api.md @@ -50,10 +50,8 @@ export function useIsOverflowItemVisible(id: string): boolean; export const useOverflowContainer: (update: OnUpdateOverflow, options: Omit) => UseOverflowContainerReturn; // @internal (undocumented) -export interface UseOverflowContainerReturn { +export interface UseOverflowContainerReturn extends Pick { containerRef: React_2.RefObject; - registerItem: OverflowContextValue['registerItem']; - updateOverflow: OverflowContextValue['updateOverflow']; } // @public (undocumented) From 52b9c0dd934394cb3937b9bea18c45aaae9bc78b Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 7 Oct 2022 17:38:36 +0200 Subject: [PATCH 4/7] add test --- .../react-overflow/e2e/Overflow.e2e.tsx | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/react-components/react-overflow/e2e/Overflow.e2e.tsx b/packages/react-components/react-overflow/e2e/Overflow.e2e.tsx index baecc5bfb8be50..3b085a511be69f 100644 --- a/packages/react-components/react-overflow/e2e/Overflow.e2e.tsx +++ b/packages/react-components/react-overflow/e2e/Overflow.e2e.tsx @@ -48,7 +48,8 @@ const setContainerSize = (size: number) => { .then(container => { container.css('width', `${size}px`); }) - .log(`Setting container size to ${size}px`); + .log(`Setting container size to ${size}px`) + .wait(1); }; const Item: React.FC<{ children?: React.ReactNode; width?: number } & Omit> = ({ @@ -414,4 +415,24 @@ describe('Overflow', () => { cy.get(`[${selectors.divider}="1"]`).should('not.exist'); cy.get(`[${selectors.divider}]`).should('have.length', 1); }); + + it.only('should remove overflow menu if the last overflowed item can take its place', () => { + const mapHelper = new Array(10).fill(0).map((_, i) => i); + mount( + + {mapHelper.map(i => ( + + {i} + + ))} + + , + ); + + setContainerSize(497); + setContainerSize(498); + setContainerSize(499); + setContainerSize(500); + cy.get(`[${selectors.menu}]`).should('not.exist'); + }); }); From 123c955468e3d2721e235e6c6f145dde1cf5f76b Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 7 Oct 2022 17:42:10 +0200 Subject: [PATCH 5/7] revert --- .../src/components/Progress/Progress.types.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/react-components/react-progress/src/components/Progress/Progress.types.ts b/packages/react-components/react-progress/src/components/Progress/Progress.types.ts index 0c976d03ec16e7..c35c7e2d109f99 100644 --- a/packages/react-components/react-progress/src/components/Progress/Progress.types.ts +++ b/packages/react-components/react-progress/src/components/Progress/Progress.types.ts @@ -17,7 +17,7 @@ export type ProgressSlots = { export type ProgressProps = Omit, 'size'> & { /** * A decimal number between `0` and `1` (or between `0` and `max` if given), - * which specifies how much of the task has been completed. + * which specifies how much of the task has been completed. * * If `undefined` (default), the Progress will display an **indeterminate** state. */ @@ -38,6 +38,4 @@ export type ProgressProps = Omit, 'size'> & { /** * State used in rendering Progress */ -export type ProgressState = ComponentState & - Required> & - Pick; +export type ProgressState = ComponentState & Required> & Pick; From 7ecdcb7cb1f952c3019f30aeebb610803bf6e97c Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 7 Oct 2022 17:43:24 +0200 Subject: [PATCH 6/7] remove cruft --- .../react-overflow/src/useOverflowMenu.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/react-components/react-overflow/src/useOverflowMenu.ts b/packages/react-components/react-overflow/src/useOverflowMenu.ts index 9007489565f1b8..792ea3c7a0737b 100644 --- a/packages/react-components/react-overflow/src/useOverflowMenu.ts +++ b/packages/react-components/react-overflow/src/useOverflowMenu.ts @@ -9,19 +9,12 @@ export function useOverflowMenu(id?: string) { const registerOverflowMenu = useOverflowContext(v => v.registerOverflowMenu); const updateOverflow = useOverflowContext(v => v.updateOverflow); const ref = React.useRef(null); - const offsetRef = React.useRef(0); const isOverflowing = overflowCount > 0; useIsomorphicLayoutEffect(() => { - const element = ref.current; - if (!element || !isOverflowing) { - offsetRef.current = 0; - return; + if (ref.current) { + return registerOverflowMenu(ref.current); } - - offsetRef.current = 1; - - return registerOverflowMenu(element); }, [registerOverflowMenu, isOverflowing, elementId]); useIsomorphicLayoutEffect(() => { From c2a1a45ecf2cb791ddafe3bf04be371dbb0dc26e Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 7 Oct 2022 17:45:19 +0200 Subject: [PATCH 7/7] rename overflowMenuSize to overflowMenuOffset --- .../react-components/priority-overflow/src/overflowManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index 84013fd08d9810..f9a45f67ce0539 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -123,7 +123,7 @@ export function createOverflowManager(): OverflowManager { return; } - const overflowMenuSize = overflowMenu ? getOffsetSize(overflowMenu) : 0; + const overflowMenuOffset = overflowMenu ? getOffsetSize(overflowMenu) : 0; // Snapshot of the visible/invisible state to compare for updates const visibleTop = visibleItemQueue.peek(); @@ -147,7 +147,7 @@ export function createOverflowManager(): OverflowManager { currentWidth -= makeItemInvisible(); } - if (invisibleItemQueue.size() > 0 && currentWidth + overflowMenuSize > availableSize) { + if (invisibleItemQueue.size() > 0 && currentWidth + overflowMenuOffset > availableSize) { makeItemInvisible(); }