Skip to content
Draft
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
59 changes: 59 additions & 0 deletions src/menu/menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -395,4 +395,63 @@ describe('Menu', () => {
})
})
})

describe('persistent elements', () => {
it('keeps elements marked with data-reactist-persist interactive while a modal menu is open', async () => {
render(
<>
<div data-testid="drag-region" data-reactist-persist />
<div data-testid="other-content" />
<Menu>
<MenuButton>Options menu</MenuButton>
<MenuList aria-label="Some options">
<MenuItem>First option</MenuItem>
</MenuList>
</Menu>
</>,
)
const user = userEvent.setup()

await user.click(screen.getByRole('button', { name: 'Options menu' }))
await flushMicrotasks()
expect(screen.getByRole('menu')).toBeInTheDocument()

// Ariakit disables the tree outside a modal menu (via `inert` where supported, and
// `aria-hidden` + `pointer-events: none` as a fallback in jsdom). The persistent
// element is exempted, so it stays interactive while a sibling without the attribute
// does not.
await waitFor(() => {
expect(screen.getByTestId('other-content')).toHaveAttribute('aria-hidden', 'true')
})
expect(screen.getByTestId('drag-region')).not.toHaveAttribute('aria-hidden')
})

it('still honors a consumer-provided getPersistentElements callback', async () => {
render(
<>
<div data-testid="custom-persist" />
<div data-testid="other-content" />
<Menu>
<MenuButton>Options menu</MenuButton>
<MenuList
aria-label="Some options"
getPersistentElements={() => [screen.getByTestId('custom-persist')]}
>
<MenuItem>First option</MenuItem>
</MenuList>
</Menu>
</>,
)
const user = userEvent.setup()

await user.click(screen.getByRole('button', { name: 'Options menu' }))
await flushMicrotasks()
expect(screen.getByRole('menu')).toBeInTheDocument()

await waitFor(() => {
expect(screen.getByTestId('other-content')).toHaveAttribute('aria-hidden', 'true')
})
expect(screen.getByTestId('custom-persist')).not.toHaveAttribute('aria-hidden')
})
})
})
31 changes: 30 additions & 1 deletion src/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,29 @@ interface MenuListProps
extends Omit<AriakitMenuProps, 'store' | 'className'>,
ObfuscatedClassName {}

/**
* Selector for elements that should remain interactive while a modal menu is open.
*
* A modal `MenuList` marks the entire element tree outside of it as `inert`, which removes those
* elements from hit-testing. On desktop apps (e.g. Electron) this breaks the native window
* drag region (`-webkit-app-region: drag`) of the title bar, since `inert` elements no longer
* receive the OS drag gesture. Marking such elements with `data-reactist-persist` keeps them
* interactive while the menu stays modal in every other respect.
*/
const PERSISTENT_ELEMENTS_SELECTOR = '[data-reactist-persist]'

function getPersistentElementsDefault(): Element[] {
if (typeof document === 'undefined') {
return []
}
return Array.from(document.querySelectorAll(PERSISTENT_ELEMENTS_SELECTOR))
}

/**
* The dropdown menu itself, containing a list of menu items.
*/
const MenuList = React.forwardRef<HTMLDivElement, MenuListProps>(function MenuList(
{ exceptionallySetClassName, modal = true, flip, ...props },
{ exceptionallySetClassName, modal = true, flip, getPersistentElements, ...props },
ref,
) {
const { menuStore, getAnchorRect } = React.useContext(MenuContext)
Expand All @@ -164,6 +182,16 @@ const MenuList = React.forwardRef<HTMLDivElement, MenuListProps>(function MenuLi

const isOpen = menuStore.useState('open')

const mergedGetPersistentElements = React.useCallback(
function mergedGetPersistentElements(): Element[] {
const consumerElements = getPersistentElements
? Array.from(getPersistentElements())
: []
return [...getPersistentElementsDefault(), ...consumerElements]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🟡 P2 MenuListProps inherits Ariakit’s getPersistentElements, so callers will expect this prop to define the complete persistent-element set. Merging the default selector here turns it into an append-only hook: once an app adds data-reactist-persist anywhere, every MenuList will keep those elements interactive and a caller can’t opt back into strict modal behavior for a specific menu. That’s a surprising contract change for a pass-through Ariakit prop. Consider applying the default only when getPersistentElements is unset, or introducing a separate Reactist prop for the title-bar escape hatch.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🟡 P2 This makes every modal MenuList pay for document.querySelectorAll('[data-reactist-persist]') whenever Ariakit resolves persistent elements, even in apps that never use this feature. Since menus are a repeated interaction path, walking the whole document and allocating fresh arrays here is avoidable overhead. Please gate the default lookup behind an explicit opt-in or cache the selector results for the lifetime of the open menu instead of rescanning the full DOM on each callback.

},
[getPersistentElements],
)

return isOpen ? (
<Portal preserveTabOrder>
<AriakitMenu
Expand All @@ -175,6 +203,7 @@ const MenuList = React.forwardRef<HTMLDivElement, MenuListProps>(function MenuLi
className={classNames('reactist_menulist', exceptionallySetClassName)}
getAnchorRect={getAnchorRect ?? undefined}
modal={modal}
getPersistentElements={mergedGetPersistentElements}
flip={flip ?? (isSubMenu ? 'left bottom' : undefined)}
/>
</Portal>
Expand Down
Loading