Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,10 @@ export function createOverflowManager(initialOptions: Partial<OverflowOptions> =
};

const addOverflowMenu: OverflowManager['addOverflowMenu'] = el => {
if (overflowMenu === el) {
return;
}

overflowMenu = el;

if (observing) {
Expand All @@ -357,9 +361,14 @@ export function createOverflowManager(initialOptions: Partial<OverflowOptions> =
};

const removeOverflowMenu: OverflowManager['removeOverflowMenu'] = () => {
if (!overflowMenu) {
return;
}

const hasInvisibleItems = invisibleItemQueue.size() > 0;
overflowMenu = undefined;

if (observing) {
if (observing && hasInvisibleItems) {
forceDispatch = true;
update();
}
Expand Down
Loading