Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabling hide middleware in DropdownMenu by default #2233

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions .changeset/green-dogs-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@itwin/itwinui-react': patch
---

`Popover`'s `middleware.hide` prop is now respected.
7 changes: 7 additions & 0 deletions .changeset/slimy-vans-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@itwin/itwinui-react': minor
---

`Popover` now enables the [`hide` middleware](https://floating-ui.com/docs/hide) to hide the floating content when the trigger is hidden.
* This also affects other popover-like components (e.g. `Select`, `ComboBox`, `DropdownMenu`).
* If the floating content gets hidden even when it shouldn't (e.g. due to some custom styles interfering with the trigger's hide detection), consider disabling the `hide` middleware.
21 changes: 21 additions & 0 deletions apps/website/src/content/docs/dropdownmenu.mdx
mayank99 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,27 @@ The menu can contain extra content, including both text and additional selectabl
<AllExamples.DropdownMenuContentExample client:load />
</LiveExample>

### `hide` middleware

By default, the menu is hidden when the trigger is hidden (e.g. out of the scrollport/viewport). This is useful when the trigger element is scrolled out of view.

<LiveExample src='DropdownMenu.hidemiddleware.jsx'>
<AllExamples.DropdownMenuHideMiddlewareExample client:load />
</LiveExample>

If the menu gets hidden even when it shouldn't (e.g. due to some custom styles interfering with the trigger's hide detection), consider disabling the `hide` middleware.

```jsx {4}
<DropdownMenu
middleware={{
hide: false,
}}
>
</DropdownMenu>
```

## Props

<PropsTable path='@itwin/itwinui-react/esm/core/DropdownMenu/DropdownMenu.d.ts' />
3 changes: 2 additions & 1 deletion apps/website/src/content/docs/popover.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ Popover handles positioning using an external library called [Floating UI](float
There are some advanced positioning options available.

- The `positionReference` prop can be used to position the popover relative to a different element than the trigger. This can be useful, for example, when the trigger is part of a larger group of elements that should all be considered together.
- The `middleware` prop exposes more granular control over the positioning logic. By default, `Popover` enables the [`flip`](https://floating-ui.com/docs/flip), [`shift`](https://floating-ui.com/docs/shift) and [`size`](https://floating-ui.com/docs/size) middlewares. You might also find the [`offset`](https://floating-ui.com/docs/offset) middleware useful for adding a gap between the trigger and the popover.
- The `middleware` prop exposes more granular control over the positioning logic. By default, `Popover` enables the [`flip`](https://floating-ui.com/docs/flip), [`shift`](https://floating-ui.com/docs/shift), [`size`](https://floating-ui.com/docs/size), and [`hide`](https://floating-ui.com/docs/hide) middlewares. You might also find the [`offset`](https://floating-ui.com/docs/offset) middleware useful for adding a gap between the trigger and the popover.
- If the floating content gets hidden even when it shouldn't (e.g. due to some custom styles interfering with the trigger's hide detection), consider disabling the `hide` middleware.

### Portals

Expand Down
8 changes: 8 additions & 0 deletions examples/DropdownMenu.hidemiddleware.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.demo-container {
width: 50%;
}

.list {
overflow-y: auto;
max-height: 200px;
}
51 changes: 51 additions & 0 deletions examples/DropdownMenu.hidemiddleware.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import * as React from 'react';
import {
Surface,
List,
ListItem,
DropdownMenu,
IconButton,
MenuItem,
} from '@itwin/itwinui-react';
import { SvgMore } from '@itwin/itwinui-icons-react';

export default () => {
const dropdownMenuItems = (close) => [
<MenuItem key={1} onClick={() => close()}>
Option #1
</MenuItem>,
<MenuItem key={2} onClick={() => close()}>
Option #2
</MenuItem>,
<MenuItem key={3} onClick={() => close()} disabled>
Option #3
</MenuItem>,
];

const items = new Array(30).fill(0);

return (
<Surface className='demo-container'>
<Surface.Body as={List} className='list'>
{items.map((_, i) => (
<ListItem key={i}>
<ListItem.Content>Item {i}</ListItem.Content>
<DropdownMenu menuItems={dropdownMenuItems}>
<IconButton
styleType='borderless'
label='More options'
size='small'
>
<SvgMore />
</IconButton>
</DropdownMenu>
</ListItem>
))}
</Surface.Body>
</Surface>
);
};
6 changes: 6 additions & 0 deletions examples/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,12 @@ const DropdownMenuContentExample = withThemeProvider(
);
export { DropdownMenuContentExample };

import { default as DropdownMenuHideMiddlewareExampleRaw } from './DropdownMenu.hidemiddleware';
const DropdownMenuHideMiddlewareExample = withThemeProvider(
DropdownMenuHideMiddlewareExampleRaw,
);
export { DropdownMenuHideMiddlewareExample };

// ----------------------------------------------------------------------------

import { default as ExpandableBlockMainExampleRaw } from './ExpandableBlock.main';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import * as React from 'react';
import { render } from '@testing-library/react';
import { render, waitFor } from '@testing-library/react';
Copy link
Contributor

@smmr-dn smmr-dn Sep 19, 2024

Choose a reason for hiding this comment

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

Is waitFor being used in this unit test? I think it might be a stray import.


import { DropdownButton } from './DropdownButton.js';
import { MenuItem } from '../Menu/MenuItem.js';
Expand Down Expand Up @@ -83,28 +83,6 @@ it('should update when menu opens or closes', async () => {
expect(svg).toEqual(downArrow);
});

it('should work with menu items', async () => {
const { container } = renderComponent();

const button = container.querySelector('.iui-button') as HTMLButtonElement;
expect(button).toBeTruthy();

let menu = document.querySelector('.iui-menu') as HTMLElement;
expect(menu).toBeFalsy();

await userEvent.click(button);
menu = document.querySelector('[role=menu]') as HTMLElement;
expect(menu).toBeVisible();

expect(document.querySelectorAll('[role=menuitem]')).toHaveLength(3);

const menuItem = menu.querySelector('[role=menuitem]') as HTMLElement;
expect(menuItem).toBeTruthy();
await userEvent.click(menuItem);

expect(menu).not.toBeVisible();
});

it('should render borderless button correctly', () => {
const { container } = renderComponent({ styleType: 'borderless' });
const button = container.querySelector('.iui-button') as HTMLButtonElement;
Expand Down
17 changes: 16 additions & 1 deletion packages/itwinui-react/src/core/ComboBox/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,17 @@ export type ComboBoxProps<T> = {
* Callback fired when dropdown menu is closed.
*/
onHide?: () => void;
/**
* Middleware options.
*
* By default, `hide` is enabled. If the floating options get hidden even when they shouldn't (e.g. some custom styles
* interfering with the trigger's hide detection) consider disabling the `hide` middleware.
*
* @see https://floating-ui.com/docs/middleware
*/
middleware?: {
hide?: boolean;
};
} & ComboboxMultipleTypeProps<T> &
Pick<InputContainerProps, 'status'> &
CommonProps;
Expand Down Expand Up @@ -206,6 +217,7 @@ export const ComboBox = React.forwardRef(
onHide: onHideProp,
id = inputProps?.id ? `iui-${inputProps.id}-cb` : idPrefix,
defaultValue,
middleware,
...rest
} = props;

Expand Down Expand Up @@ -572,7 +584,10 @@ export const ComboBox = React.forwardRef(
visible: isOpen,
onVisibleChange: (open) => (open ? show() : hide()),
matchWidth: true,
middleware: { size: { maxHeight: 'var(--iui-menu-max-height)' } },
middleware: {
size: { maxHeight: 'var(--iui-menu-max-height)' },
...middleware,
},
closeOnOutsideClick: true,
interactions: { click: false, focus: true },
});
Expand Down
56 changes: 35 additions & 21 deletions packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ export type DropdownMenuProps = {
* Child element to wrap dropdown with.
*/
children: React.ReactNode;
/**
* Middleware options.
*
* By default, `hide` is enabled. If the menu gets hidden even when it shouldn't (e.g. some custom styles interfering
* with the trigger's hide detection) consider disabling the `hide` middleware.
*
* @see https://floating-ui.com/docs/middleware
*/
middleware?: {
hide?: boolean;
Copy link
Contributor

@mayank99 mayank99 Sep 19, 2024

Choose a reason for hiding this comment

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

Building upon #2233 (comment), I think middleware.hide should be exposed in all places that use usePopover (directly or indirectly).

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be under dropdownMenuProps in ComboBox and under popoverProps in Select.

};
} & Pick<
Parameters<typeof usePopover>[0],
'visible' | 'onVisibleChange' | 'placement' | 'matchWidth'
Expand Down Expand Up @@ -78,6 +89,7 @@ const DropdownMenuContent = React.forwardRef((props, forwardedRef) => {
matchWidth = false,
onVisibleChange,
portal = true,
middleware,
...rest
} = props;

Expand All @@ -95,30 +107,32 @@ const DropdownMenuContent = React.forwardRef((props, forwardedRef) => {
}, [menuItems, setVisible]);

return (
<>
<Menu
trigger={children}
onKeyDown={mergeEventHandlers(props.onKeyDown, (e) => {
if (e.defaultPrevented) {
return;
}
if (e.key === 'Tab') {
setVisible(false);
}
})}
role={role}
ref={forwardedRef}
portal={portal}
popoverProps={{
<Menu
trigger={children}
onKeyDown={mergeEventHandlers(props.onKeyDown, (e) => {
if (e.defaultPrevented) {
return;
}
if (e.key === 'Tab') {
setVisible(false);
}
})}
role={role}
ref={forwardedRef}
portal={portal}
popoverProps={React.useMemo(
() => ({
placement,
matchWidth,
visible,
onVisibleChange: setVisible,
}}
{...rest}
>
{menuContent}
</Menu>
</>
middleware,
}),
[matchWidth, middleware, placement, setVisible, visible],
)}
{...rest}
>
{menuContent}
</Menu>
);
}) as PolymorphicForwardRefComponent<'div', DropdownMenuProps>;
49 changes: 0 additions & 49 deletions packages/itwinui-react/src/core/Header/Header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,55 +102,6 @@ it('renders avatar alone correctly', () => {
expect(avatar).toBeTruthy();
expect(avatar?.textContent).toEqual('AvatarContent');
});
it('renders moreMenu alone correctly', async () => {
// Summarized, as this is partly based on DropdownMenu, which is tested independently.
const itemOneOnClick = vi.fn();

const { container } = render(
<Header
appLogo={<div>AppTitle</div>}
menuItems={(close) => [
<MenuItem
key={0}
onClick={() => {
itemOneOnClick();
close();
}}
>
Test0
</MenuItem>,
<MenuItem key={1} onClick={close}>
Test1
</MenuItem>,
<MenuItem key={2} onClick={close}>
Test2
</MenuItem>,
]}
/>,
);
const button = container.querySelector(
'.iui-page-header-right > .iui-button[data-iui-variant="borderless"]:last-child',
) as HTMLButtonElement;
expect(button).toBeTruthy();
expect(button.getAttribute('aria-label')).toEqual('More options');

let menu = document.querySelector('.iui-menu') as HTMLElement;
expect(menu).toBeFalsy();

await userEvent.click(button);
menu = document.querySelector('.iui-menu') as HTMLElement;
expect(menu).toBeVisible();

expect(document.querySelectorAll('[role=menuitem]')).toHaveLength(3);

const menuItem = menu.querySelector('[role=menuitem]') as HTMLElement;
expect(menuItem).toBeTruthy();
await userEvent.click(menuItem);

expect(menu).not.toBeVisible();

expect(itemOneOnClick).toHaveBeenCalled();
});

it('renders translatedStrings correctly', () => {
const { container } = render(
Expand Down
30 changes: 21 additions & 9 deletions packages/itwinui-react/src/core/Popover/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,27 @@ export const usePopover = (options: PopoverOptions & PopoverInternalProps) => {

const mergedInteractions = React.useMemo(
() => ({
...interactionsProp,
...{
click: true,
dismiss: true,
hover: false,
focus: false,
click: interactionsProp?.click ?? true,
dismiss: interactionsProp?.dismiss ?? true,
hover: interactionsProp?.hover ?? false,
focus: interactionsProp?.focus ?? false,
},
...interactionsProp,
mayank99 marked this conversation as resolved.
Show resolved Hide resolved
}),
[interactionsProp],
);

const tree = useFloatingTree();

const middleware = React.useMemo(
() => ({ flip: true, shift: true, size: true, ...options.middleware }),
() => ({
...options.middleware,
flip: options.middleware?.flip ?? true,
shift: options.middleware?.shift ?? true,
size: options.middleware?.size ?? true,
hide: options.middleware?.hide ?? true,
}),
[options.middleware],
);

Expand Down Expand Up @@ -287,17 +293,23 @@ export const usePopover = (options: PopoverOptions & PopoverInternalProps) => {
maxInlineSize: `min(${referenceWidth * 2}px, 90vw)`,
}
: {}),
...(middleware.hide &&
floating.middlewareData.hide?.referenceHidden && {
visibility: 'hidden',
}),
...userProps?.style,
},
}),
[
floating.floatingStyles,
interactions,
matchWidth,
referenceWidth,
floating.floatingStyles,
floating.middlewareData.hide?.referenceHidden,
middleware.size,
middleware.hide,
availableHeight,
maxHeight,
matchWidth,
referenceWidth,
],
);

Expand Down
Loading
Loading