Skip to content

Commit

Permalink
[Popover] Disable focus management when opened via hover (#855)
Browse files Browse the repository at this point in the history
  • Loading branch information
atomiks authored Nov 25, 2024
1 parent 27ab5e4 commit fde72cf
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 14 deletions.
3 changes: 2 additions & 1 deletion packages/react/src/Popover/Popup/PopoverPopup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const PopoverPopup = React.forwardRef(function PopoverPopup(
descriptionId,
popupRef,
mounted,
openReason,
} = usePopoverRootContext();
const positioner = usePopoverPositionerContext();

Expand Down Expand Up @@ -91,7 +92,7 @@ const PopoverPopup = React.forwardRef(function PopoverPopup(
<FloatingFocusManager
context={positioner.positionerContext}
modal={false}
disabled={!mounted}
disabled={!mounted || openReason === 'hover'}
initialFocus={resolvedInitialFocus}
returnFocus={finalFocus}
>
Expand Down
27 changes: 26 additions & 1 deletion packages/react/src/Popover/Root/PopoverRoot.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { Popover } from '@base-ui-components/react/Popover';
import { fireEvent, flushMicrotasks, screen, waitFor } from '@mui/internal-test-utils';
import { act, fireEvent, flushMicrotasks, screen, waitFor } from '@mui/internal-test-utils';
import userEvent from '@testing-library/user-event';
import { expect } from 'chai';
import { spy } from 'sinon';
Expand Down Expand Up @@ -333,4 +333,29 @@ describe('<Popover.Root />', () => {
{ timeout: 1500 },
);
});

it('does not move focus to the popover when opened with hover', async () => {
await render(
<Popover.Root openOnHover delay={0}>
<Popover.Trigger>Toggle</Popover.Trigger>
<Popover.Positioner>
<Popover.Popup>
<Popover.Close>Close</Popover.Close>
</Popover.Popup>
</Popover.Positioner>
</Popover.Root>,
);

const toggle = screen.getByRole('button', { name: 'Toggle' });

act(() => toggle.focus());

await user.hover(toggle);
await flushMicrotasks();

const close = screen.getByRole('button', { name: 'Close' });

expect(close).not.to.equal(null);
expect(close).not.to.toHaveFocus();
});
});
17 changes: 5 additions & 12 deletions packages/react/src/Popover/Root/usePopoverRoot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export function usePopoverRoot(params: usePopoverRoot.Parameters): usePopoverRoo
open: externalOpen,
onOpenChange: onOpenChangeProp = () => {},
defaultOpen = false,
keepMounted = false,
delay,
closeDelay,
openOnHover = false,
Expand Down Expand Up @@ -69,22 +68,21 @@ export function usePopoverRoot(params: usePopoverRoot.Parameters): usePopoverRoo
(nextOpen: boolean, event?: Event, reason?: OpenChangeReason) => {
onOpenChange(nextOpen, event, reason);
setOpenUnwrapped(nextOpen);
if (!keepMounted && !nextOpen) {

if (!nextOpen) {
if (animated) {
runOnceAnimationsFinish(() => {
if (!openRef.current) {
setMounted(false);
setOpenReason(null);
}
});
} else {
setMounted(false);
setOpenReason(null);
}
}

if (nextOpen) {
setOpenReason(reason ?? null);
} else {
setOpenReason(null);
setOpenReason(reason ?? null);
}
},
);
Expand Down Expand Up @@ -209,11 +207,6 @@ export namespace usePopoverRoot {
* @default 0
*/
closeDelay?: number;
/**
* Whether the popover popup element stays mounted in the DOM when closed.
* @default false
*/
keepMounted?: boolean;
/**
* Whether the popover can animate, adding animation-related attributes and allowing for exit
* animations to play. Useful to disable in tests to remove async behavior.
Expand Down

0 comments on commit fde72cf

Please sign in to comment.