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": "prerelease",
"comment": "feat: Adds API to register overflow menus for better available space calculation",
"packageName": "@fluentui/priority-overflow",
"email": "lingfangao@hotmail.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "fix: useOverflowMenu should register overflow menu",
"packageName": "@fluentui/react-overflow",
"email": "lingfangao@hotmail.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type { OverflowGroupState, OverflowItemEntry, OverflowManager, ObserveOpt
*/
export function createOverflowManager(): OverflowManager {
let container: HTMLElement | undefined;
let overflowMenu: HTMLElement | undefined;
let observing = false;
const options: Required<ObserveOptions> = {
padding: 10,
Expand Down Expand Up @@ -122,6 +123,8 @@ export function createOverflowManager(): OverflowManager {
return;
}

const overflowMenuOffset = overflowMenu ? getOffsetSize(overflowMenu) : 0;

// Snapshot of the visible/invisible state to compare for updates
const visibleTop = visibleItemQueue.peek();
const invisibleTop = invisibleItemQueue.peek();
Expand All @@ -144,6 +147,10 @@ export function createOverflowManager(): OverflowManager {
currentWidth -= makeItemInvisible();
}

if (invisibleItemQueue.size() > 0 && currentWidth + overflowMenuOffset > availableSize) {
makeItemInvisible();
}

// only update when the state of visible/invisible items has changed
if (visibleItemQueue.peek() !== visibleTop || invisibleItemQueue.peek() !== invisibleTop) {
dispatchOverflowUpdate();
Expand Down Expand Up @@ -176,6 +183,10 @@ export function createOverflowManager(): OverflowManager {
};

const addItem: OverflowManager['addItem'] = item => {
if (overflowItems[item.id]) {
return;
}

overflowItems[item.id] = item;

// some options can affect priority which are only set on `observe`
Expand All @@ -197,7 +208,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);
Expand All @@ -218,5 +241,7 @@ export function createOverflowManager(): OverflowManager {
observe,
removeItem,
update,
addOverflowMenu,
removeOverflowMenu,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,14 @@ export function createPriorityQueue<T>(compare: PriorityQueueCompareFn<T>): 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) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these were absolutely awful bugs

Comment thread
ling1726 marked this conversation as resolved.
return;
}

Expand Down
11 changes: 11 additions & 0 deletions packages/react-components/priority-overflow/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
23 changes: 22 additions & 1 deletion packages/react-components/react-overflow/e2e/Overflow.e2e.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<OverflowItemProps, 'children'>> = ({
Expand Down Expand Up @@ -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(
<Container>
{mapHelper.map(i => (
<Item key={i} id={i.toString()}>
{i}
</Item>
))}
<Menu />
</Container>,
);

setContainerSize(497);
setContainerSize(498);
setContainerSize(499);
setContainerSize(500);
cy.get(`[${selectors.menu}]`).should('not.exist');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@ export function useIsOverflowItemVisible(id: string): boolean;
export const useOverflowContainer: <TElement extends HTMLElement>(update: OnUpdateOverflow, options: Omit<ObserveOptions, 'onUpdateOverflow'>) => UseOverflowContainerReturn<TElement>;

// @internal (undocumented)
export interface UseOverflowContainerReturn<TElement extends HTMLElement> {
export interface UseOverflowContainerReturn<TElement extends HTMLElement> extends Pick<OverflowContextValue, 'registerItem' | 'updateOverflow' | 'registerOverflowMenu'> {
containerRef: React_2.RefObject<TElement>;
registerItem: OverflowContextValue['registerItem'];
updateOverflow: OverflowContextValue['updateOverflow'];
}

// @public (undocumented)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -76,6 +76,7 @@ export const Overflow = React.forwardRef((props: OverflowProps, ref) => {
hasOverflow,
registerItem,
updateOverflow,
registerOverflowMenu,
}}
>
{clonedChild}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export interface OverflowContextValue {
groupVisibility: Record<string, OverflowGroupState>;
hasOverflow: boolean;
registerItem: (item: OverflowItemEntry) => () => void;
registerOverflowMenu: (el: HTMLElement) => () => void;
updateOverflow: (padding?: number) => void;
}

Expand All @@ -19,6 +20,7 @@ const overflowContextDefaultValue: OverflowContextValue = {
hasOverflow: false,
registerItem: () => () => null,
updateOverflow: () => null,
registerOverflowMenu: () => () => null,
};

export const useOverflowContext = <SelectedValue>(selector: ContextSelector<OverflowContextValue, SelectedValue>) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ export const ReverseDomOrder = () => {
return (
<Overflow overflowDirection="start">
<div className={styles.container}>
<OverflowMenu itemIds={itemIds} />
{itemIds.map(i => (
<TestOverflowItem key={i} id={i} />
))}
<OverflowMenu itemIds={itemIds} />
</div>
</Overflow>
);
Expand Down
11 changes: 2 additions & 9 deletions packages/react-components/react-overflow/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,10 @@ import { OverflowContextValue } from './overflowContext';
/**
* @internal
*/
export interface UseOverflowContainerReturn<TElement extends HTMLElement> {
export interface UseOverflowContainerReturn<TElement extends HTMLElement>
extends Pick<OverflowContextValue, 'registerItem' | 'updateOverflow' | 'registerOverflowMenu'> {
/**
* Ref to apply to the container that will overflow
*/
containerRef: React.RefObject<TElement>;
/**
* Registers and overflow item
*/
registerItem: OverflowContextValue['registerItem'];
/**
* Imperative function to trigger overflow update
*/
updateOverflow: OverflowContextValue['updateOverflow'];
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -82,10 +82,24 @@ export const useOverflowContainer = <TElement extends HTMLElement>(
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,
};
};

Expand Down
20 changes: 4 additions & 16 deletions packages/react-components/react-overflow/src/useOverflowMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,20 @@ 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<TElement extends HTMLElement>(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<TElement>(null);
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 (ref.current) {
return registerOverflowMenu(ref.current);
}
}, [registerItem, isOverflowing, elementId]);
}, [registerOverflowMenu, isOverflowing, elementId]);

useIsomorphicLayoutEffect(() => {
if (isOverflowing) {
Expand Down