From b5ec47fc035849b21b35d6f6054d60c0a64ff3db Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Fri, 21 Aug 2026 12:29:35 +0200 Subject: [PATCH] Fix overflow menu teardown convergence (#36602) --- ...-7b5f4bd5-6726-4b0f-b54f-ff6cbeb538b4.json | 7 ++ .../src/overflowManager.test.ts | 102 ++++++++++++++++++ .../priority-overflow/src/overflowManager.ts | 11 +- 3 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 change/@fluentui-priority-overflow-7b5f4bd5-6726-4b0f-b54f-ff6cbeb538b4.json diff --git a/change/@fluentui-priority-overflow-7b5f4bd5-6726-4b0f-b54f-ff6cbeb538b4.json b/change/@fluentui-priority-overflow-7b5f4bd5-6726-4b0f-b54f-ff6cbeb538b4.json new file mode 100644 index 00000000000000..ae438b65e62c97 --- /dev/null +++ b/change/@fluentui-priority-overflow-7b5f4bd5-6726-4b0f-b54f-ff6cbeb538b4.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: avoid redundant overflow recomputation after conditional menu removal", + "packageName": "@fluentui/priority-overflow", + "email": "bsunderhus@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/priority-overflow/src/overflowManager.test.ts b/packages/react-components/priority-overflow/src/overflowManager.test.ts index 6fdc7bcb2a0731..70299140d146c3 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.test.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.test.ts @@ -224,6 +224,108 @@ describe('overflowManager', () => { expect(getClientWidth).toHaveBeenCalledTimes(1); }); + it('should not recompute when the same overflow menu is added twice', () => { + const manager = createOverflowManager(createObserveOptions()); + const container = createContainer(100); + const getClientWidth = jest.fn(() => 100); + Object.defineProperty(container, 'clientWidth', { configurable: true, get: getClientWidth }); + const menu = createElementWithSize('button', 30); + + manager.addItem({ element: createElementWithSize('button', 60), id: 'a', priority: 1 }); + manager.addItem({ element: createElementWithSize('button', 60), id: 'b', priority: 0 }); + manager.observe(container); + manager.forceUpdate(); + manager.addOverflowMenu(menu); + getClientWidth.mockClear(); + + const listener = jest.fn(); + manager.subscribe(listener); + manager.addOverflowMenu(menu); + + expect(listener).not.toHaveBeenCalled(); + expect(getClientWidth).not.toHaveBeenCalled(); + }); + + it('should not recompute when no overflow menu is registered', () => { + const manager = createOverflowManager(createObserveOptions()); + const container = createContainer(100); + const getClientWidth = jest.fn(() => 100); + Object.defineProperty(container, 'clientWidth', { configurable: true, get: getClientWidth }); + + manager.addItem({ element: createElementWithSize('button', 60), id: 'a', priority: 1 }); + manager.addItem({ element: createElementWithSize('button', 60), id: 'b', priority: 0 }); + manager.observe(container); + manager.forceUpdate(); + getClientWidth.mockClear(); + + const listener = jest.fn(); + manager.subscribe(listener); + manager.removeOverflowMenu(); + + expect(listener).not.toHaveBeenCalled(); + expect(getClientWidth).not.toHaveBeenCalled(); + }); + + it('should not recompute when the overflow menu is removed after all items become visible', () => { + const manager = createOverflowManager(createObserveOptions()); + const container = createContainer(110); + const getClientWidth = jest.fn(() => 110); + Object.defineProperty(container, 'clientWidth', { configurable: true, get: getClientWidth }); + const menu = createElementWithSize('button', 30); + let menuAttached = false; + + const createResponsiveItem = () => { + const item = document.createElement('button'); + Object.defineProperty(item, 'offsetWidth', { + configurable: true, + get: () => (menuAttached ? 35 : 60), + }); + return item; + }; + + manager.addItem({ element: createResponsiveItem(), id: 'a', priority: 1 }); + manager.addItem({ element: createResponsiveItem(), id: 'b', priority: 0 }); + manager.observe(container); + manager.forceUpdate(); + expect(getInvisibleIds(manager)).toEqual(['b']); + + menuAttached = true; + manager.addOverflowMenu(menu); + expect(getInvisibleIds(manager)).toEqual([]); + getClientWidth.mockClear(); + + const listener = jest.fn(); + manager.subscribe(listener); + menuAttached = false; + manager.removeOverflowMenu(); + + expect(listener).not.toHaveBeenCalled(); + expect(getClientWidth).not.toHaveBeenCalled(); + expect(getVisibleIds(manager)).toEqual(['a', 'b']); + }); + + it('should recompute when the overflow menu is removed with hidden items', () => { + const manager = createOverflowManager(createObserveOptions()); + const container = createContainer(140); + const getClientWidth = jest.fn(() => 140); + Object.defineProperty(container, 'clientWidth', { configurable: true, get: getClientWidth }); + + manager.addItem({ element: createElementWithSize('button', 60), id: 'a', priority: 1 }); + manager.addItem({ element: createElementWithSize('button', 60), id: 'b', priority: 0 }); + manager.addItem({ element: createElementWithSize('button', 60), id: 'c', priority: -1 }); + manager.addOverflowMenu(createElementWithSize('button', 30)); + manager.observe(container); + manager.forceUpdate(); + expect(getInvisibleIds(manager)).toHaveLength(2); + getClientWidth.mockClear(); + + manager.removeOverflowMenu(); + + expect(getClientWidth).toHaveBeenCalledTimes(1); + expect(getVisibleIds(manager)).toHaveLength(2); + expect(getInvisibleIds(manager)).toHaveLength(1); + }); + it('should remove items through removeItem', () => { const manager = createOverflowManager(createObserveOptions()); const container = createContainer(100); diff --git a/packages/react-components/priority-overflow/src/overflowManager.ts b/packages/react-components/priority-overflow/src/overflowManager.ts index 22498f92a082eb..3a5ce0cc054967 100644 --- a/packages/react-components/priority-overflow/src/overflowManager.ts +++ b/packages/react-components/priority-overflow/src/overflowManager.ts @@ -335,6 +335,10 @@ export function createOverflowManager(initialOptions: Partial = }; const addOverflowMenu: OverflowManager['addOverflowMenu'] = el => { + if (overflowMenu === el) { + return; + } + overflowMenu = el; if (observing) { @@ -357,9 +361,14 @@ export function createOverflowManager(initialOptions: Partial = }; const removeOverflowMenu: OverflowManager['removeOverflowMenu'] = () => { + if (!overflowMenu) { + return; + } + + const hasInvisibleItems = invisibleItemQueue.size() > 0; overflowMenu = undefined; - if (observing) { + if (observing && hasInvisibleItems) { forceDispatch = true; update(); }