Skip to content

nav-menu: Fix auto-focus resetting submenu default value #3935

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.

Sign up for GitHub

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

Open
chaance wants to merge 1 commit into
base: main
Choose a base branch
from
Open

nav-menu: Fix auto-focus resetting submenu default value #3935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/fine-kids-jam.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@radix-ui/react-navigation-menu": patch
---

Fixed a bug where a submenu's `defaultValue` was reset when external element was focused before menu is opened.
5 changes: 5 additions & 0 deletions .changeset/navigation-menu-focus-outside.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@radix-ui/react-navigation-menu': patch
---

Fixed an open `NavigationMenu` (e.g. via `defaultValue`) being dismissed when focus moved between two elements outside the menu, such as a `Dialog` auto-focusing its close button on open.
200 changes: 199 additions & 1 deletion apps/storybook/stories/navigation-menu.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import { NavigationMenu, Direction } from 'radix-ui';
import { Dialog, NavigationMenu, Direction } from 'radix-ui';
import styles from './navigation-menu.stories.module.css';
import dialogStyles from './dialog.stories.module.css';

export default { title: 'Components/NavigationMenu' };

Expand Down Expand Up @@ -368,6 +369,203 @@ export const Submenus = () => {
);
};

export const InsideDialog = () => {
return (
<Dialog.Root>
<Dialog.Trigger className={dialogStyles.trigger}>open</Dialog.Trigger>
<Dialog.Portal>
<Dialog.Overlay className={dialogStyles.overlay} />
<Dialog.Content className={dialogStyles.contentDefault}>
<Dialog.Title>Booking info</Dialog.Title>
<Dialog.Description>Please enter the info for your booking below.</Dialog.Description>
<div>
<NavigationMenu.Root>
<NavigationMenu.List className={styles.mainList}>
<NavigationMenu.Item>
<TriggerWithIndicator>Products</TriggerWithIndicator>
<NavigationMenu.Content className={styles.submenusContent}>
<NavigationMenu.Sub className={styles.submenusRoot} defaultValue="security">
<NavigationMenu.List className={styles.mainList}>
<NavigationMenu.Item value="extensibility">
<NavigationMenu.Trigger className={styles.submenusSubTrigger}>
Extensibility
</NavigationMenu.Trigger>

<NavigationMenu.Content
className={styles.submenusSubContent}
style={{
gridTemplateColumns: '1.5fr 1fr 1fr',
}}
>
<LinkGroup items={['Donec quis dui', 'Vestibulum', 'Nunc dignissim']} />
<LinkGroup
items={['Fusce pellentesque', 'Aliquam porttitor', 'Pellentesque']}
/>
<LinkGroup
items={['Fusce pellentesque', 'Aliquam porttitor', 'Pellentesque']}
/>
</NavigationMenu.Content>
</NavigationMenu.Item>

<NavigationMenu.Item value="security">
<NavigationMenu.Trigger className={styles.submenusSubTrigger}>
Security
</NavigationMenu.Trigger>
<NavigationMenu.Content
className={styles.submenusSubContent}
style={{
gridTemplateColumns: '1fr 1fr 1fr',
}}
>
<LinkGroup
items={[
'Fusce pellentesque',
'Aliquam porttitor',
'Pellentesque',
'Vestibulum',
]}
/>
<LinkGroup
items={['Fusce pellentesque', 'Aliquam porttitor', 'Pellentesque']}
/>
<LinkGroup items={['Fusce pellentesque', 'Aliquam porttitor']} />
</NavigationMenu.Content>
</NavigationMenu.Item>

<NavigationMenu.Item value="authentication">
<NavigationMenu.Trigger className={styles.submenusSubTrigger}>
Authentication
</NavigationMenu.Trigger>

<NavigationMenu.Content
className={styles.submenusSubContent}
style={{
gridTemplateColumns: '1.5fr 1fr 1fr',
}}
>
<LinkGroup items={['Donec quis dui', 'Vestibulum', 'Nunc dignissim']} />
<LinkGroup
items={['Fusce pellentesque', 'Aliquam porttitor', 'Pellentesque']}
/>
<LinkGroup
items={['Fusce pellentesque', 'Aliquam porttitor', 'Pellentesque']}
/>
</NavigationMenu.Content>
</NavigationMenu.Item>

<NavigationMenu.Indicator className={styles.submenusSubIndicator} />
</NavigationMenu.List>

<NavigationMenu.Viewport className={styles.submenusSubViewport} />
</NavigationMenu.Sub>
</NavigationMenu.Content>
</NavigationMenu.Item>

<NavigationMenu.Item>
<TriggerWithIndicator>Company</TriggerWithIndicator>
<NavigationMenu.Content className={styles.submenusContent}>
<NavigationMenu.Sub
className={styles.submenusRoot}
orientation="vertical"
defaultValue="customers"
>
<NavigationMenu.List className={styles.mainList}>
<NavigationMenu.Item value="customers">
<NavigationMenu.Trigger className={styles.submenusSubTrigger}>
Customers
</NavigationMenu.Trigger>

<NavigationMenu.Content
className={styles.submenusSubContent}
style={{
gridTemplateColumns: '1.5fr 1fr',
}}
>
<LinkGroup items={['Donec quis dui', 'Vestibulum', 'Nunc dignissim']} />
<LinkGroup
items={['Fusce pellentesque', 'Aliquam porttitor', 'Pellentesque']}
/>
</NavigationMenu.Content>
</NavigationMenu.Item>

<NavigationMenu.Item value="partners">
<NavigationMenu.Trigger className={styles.submenusSubTrigger}>
Partners
</NavigationMenu.Trigger>
<NavigationMenu.Content
className={styles.submenusSubContent}
style={{
gridTemplateColumns: '1fr 1fr',
}}
>
<LinkGroup
items={[
'Fusce pellentesque',
'Aliquam porttitor',
'Pellentesque',
'Vestibulum',
]}
/>
<LinkGroup
items={['Fusce pellentesque', 'Aliquam porttitor', 'Pellentesque']}
/>
</NavigationMenu.Content>
</NavigationMenu.Item>

<NavigationMenu.Item value="enterprise">
<NavigationMenu.Trigger className={styles.submenusSubTrigger}>
Enterprise
</NavigationMenu.Trigger>

<NavigationMenu.Content
className={styles.submenusSubContent}
style={{
gridTemplateColumns: '1.5fr 1fr',
}}
>
<LinkGroup items={['Donec quis dui', 'Vestibulum', 'Nunc dignissim']} />
<LinkGroup
items={['Fusce pellentesque', 'Aliquam porttitor', 'Pellentesque']}
/>
</NavigationMenu.Content>
</NavigationMenu.Item>

<NavigationMenu.Indicator className={styles.submenusSubIndicator} />
</NavigationMenu.List>

<NavigationMenu.Viewport className={styles.submenusSubViewport} />
</NavigationMenu.Sub>
</NavigationMenu.Content>
</NavigationMenu.Item>

<NavigationMenu.Item>
<TriggerWithIndicator disabled>Developers</TriggerWithIndicator>
<NavigationMenu.Content
className={styles.submenusSubContent}
style={{ gridTemplateColumns: '1fr 1fr' }}
>
<LinkGroup items={['Donec quis dui', 'Vestibulum']} />
<LinkGroup items={['Fusce pellentesque', 'Aliquam porttitor']} />
</NavigationMenu.Content>
</NavigationMenu.Item>

<NavigationMenu.Item>
<NavigationMenu.Link href="#example" className={styles.link}>
Link
</NavigationMenu.Link>
</NavigationMenu.Item>
</NavigationMenu.List>

<NavigationMenu.Viewport className={styles.submenusViewport} />
</NavigationMenu.Root>
</div>
<Dialog.Close className={dialogStyles.close}>close</Dialog.Close>
</Dialog.Content>
</Dialog.Portal>
</Dialog.Root>
);
};

/* -----------------------------------------------------------------------------------------------*/

const StoryFrame: React.FC<{ children: React.ReactNode }> = ({ children }) => {
Expand Down
51 changes: 49 additions & 2 deletions packages/react/navigation-menu/src/navigation-menu.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { cleanup, render, screen, waitFor } from '@testing-library/react';
import { afterEach, describe, it, expect } from 'vitest';
import { cleanup, render, screen, waitFor, fireEvent } from '@testing-library/react';
import { afterEach, describe, it, expect, vi } from 'vitest';
import * as NavigationMenu from './navigation-menu';

const TRIGGER_TEXT = 'Item One';
Expand Down Expand Up @@ -44,3 +44,50 @@ describe('aria-controls', () => {
expect(content).toContainElement(screen.getByText(CONTENT_TEXT));
});
});

// See: https://github.com/radix-ui/primitives/issues/3473
describe('focus outside', () => {
afterEach(cleanup);

it('should not dismiss an open menu when focus moves between elements outside the menu', async () => {
const onValueChange = vi.fn();
render(
<div>
<NavigationMenuTest defaultValue="one" onValueChange={onValueChange} />
<button type="button" data-testid="outside">
Outside
</button>
</div>,
);
await waitFor(() => expect(screen.getByText(CONTENT_TEXT)).toBeInTheDocument());

// Mimics an external layer (e.g. a Dialog) auto-focusing an element on open.
// Focus never originated from within the menu, so it should stay open.
const outside = screen.getByTestId('outside');
outside.focus();
fireEvent.focusIn(outside);

expect(onValueChange).not.toHaveBeenCalledWith('');
expect(screen.getByText(CONTENT_TEXT)).toBeInTheDocument();
});

it('should dismiss an open menu when focus actually leaves the menu', async () => {
const onValueChange = vi.fn();
render(
<div>
<NavigationMenuTest defaultValue="one" onValueChange={onValueChange} />
<button type="button" data-testid="outside">
Outside
</button>
</div>,
);
await waitFor(() => expect(screen.getByText(CONTENT_TEXT)).toBeInTheDocument());

// Focus leaving the menu content for an outside element should dismiss it.
const link = screen.getByText(CONTENT_TEXT);
const outside = screen.getByTestId('outside');
fireEvent.focusIn(outside, { relatedTarget: link });

expect(onValueChange).toHaveBeenCalledWith('');
});
});
15 changes: 12 additions & 3 deletions packages/react/navigation-menu/src/navigation-menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -936,9 +936,18 @@ const NavigationMenuContentImpl = React.forwardRef<
}}
onFocusOutside={composeEventHandlers(props.onFocusOutside, (event) => {
onContentFocusOutside();
const target = event.target as HTMLElement;
// Only dismiss content when focus moves outside of the menu
if (context.rootNavigationMenu?.contains(target)) event.preventDefault();
const target = event.target;
const relatedTarget = event.detail.originalEvent.relatedTarget;
const focusMovedIntoMenu = context.rootNavigationMenu?.contains(target as Node);
const focusCameFromMenu = context.rootNavigationMenu?.contains(relatedTarget as Node);
// Only dismiss content when focus actually leaves the menu. If focus
// moves into the menu, or it never originated from within the menu
// (e.g. an external layer such as a Dialog auto-focusing on open),
// keep the content open.
// See https://github.com/radix-ui/primitives/issues/3473
if (focusMovedIntoMenu || !focusCameFromMenu) {
event.preventDefault();
}
})}
onPointerDownOutside={composeEventHandlers(props.onPointerDownOutside, (event) => {
const target = event.target as HTMLElement;
Expand Down
Loading