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
9 changes: 5 additions & 4 deletions packages/core/src/Popover/Popover.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* @file Popover.test.tsx
* @input Uses vitest, Testing Library, node:fs, Popover, Dialog, and
* SegmentedControl
* @output Unit tests for Popover component behavior
* @output Unit and compile-time public-contract tests for Popover behavior
* @position Testing; validates Popover.tsx implementation
*
* SYNC: When Popover.tsx changes, update tests to match new behavior
Expand Down Expand Up @@ -66,9 +66,10 @@ describe('usePopover public return type', () => {
const hasDismissalGuard: 'wasJustDismissed' extends keyof UsePopoverReturn
? true
: false = false;
const hasInternalToggle: 'toggleWithOptions' extends keyof UsePopoverReturn
type PublicToggleParameters = Parameters<UsePopoverReturn['toggle']>;
const publicToggleTakesNoOptions: PublicToggleParameters extends []
? true
: false = false;
: false = true;
type PublicShowOptions = NonNullable<
Parameters<UsePopoverReturn['show']>[0]
>;
Expand All @@ -77,7 +78,7 @@ describe('usePopover public return type', () => {
: false = false;
expect(hasKeepOpenProps).toBe(false);
expect(hasDismissalGuard).toBe(false);
expect(hasInternalToggle).toBe(false);
expect(publicToggleTakesNoOptions).toBe(true);
expect(hasInternalFocusTarget).toBe(false);
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/Popover/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/**
* @file Popover.tsx
* @input Uses React layout measurement and the usePopover hook
* @output Exports Popover with viewport fitting and conditional overflow
* @output Exports Popover with viewport fitting, conditional overflow, and trigger-aware focus
* @position Layer component; declarative wrapper around usePopover hook
*
* For hover-triggered overlays, use HoverCard instead.
Expand Down Expand Up @@ -521,7 +521,7 @@ export function Popover({
// preselected. Keep focus inside the modal dialog by focusing its
// labeled container; keyboard and AT activation still focus the first
// content control and expose the expected focus ring.
popover.toggleWithOptions({
popover.toggle({
focusTarget:
role === 'dialog' && event != null && event.detail > 0
? 'container'
Expand Down
34 changes: 12 additions & 22 deletions packages/core/src/Popover/usePopover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/**
* @file usePopover.tsx
* @input Uses useLayer, useFocusTrap, React hooks
* @output Exports usePopover and a package-internal trigger-aware variant.
* @output Exports usePopover and a package-internal variant with refined open options.
* @position Higher-level layer utility; used by DatePicker, Combobox, etc.
*
* Combines popover layer behavior with focus trap for dialog-like popovers.
Expand Down Expand Up @@ -323,12 +323,14 @@ export interface UsePopoverReturn {
};
}

interface InternalPopoverOpenOptions {
skipAutoFocus?: boolean;
focusTarget?: 'first' | 'container';
}

interface InternalUsePopoverReturn extends UsePopoverReturn {
toggle: (options?: InternalPopoverOpenOptions) => void;
wasJustDismissed: () => boolean;
toggleWithOptions: (options?: {
skipAutoFocus?: boolean;
focusTarget?: 'first' | 'container';
}) => void;
}

/**
Expand Down Expand Up @@ -467,12 +469,9 @@ function usePopoverImplementation(
[layer],
);

// Show function with optional skipAutoFocus
// Show function with optional open preferences
const show = useCallback(
(showOptions?: {
skipAutoFocus?: boolean;
focusTarget?: 'first' | 'container';
}) => {
(showOptions?: InternalPopoverOpenOptions) => {
skipAutoFocusRef.current = showOptions?.skipAutoFocus ?? false;
focusTargetRef.current = showOptions?.focusTarget ?? 'first';
layer.show();
Expand All @@ -481,11 +480,8 @@ function usePopoverImplementation(
);

// Toggle function
const toggleWithOptions = useCallback(
(showOptions?: {
skipAutoFocus?: boolean;
focusTarget?: 'first' | 'container';
}) => {
const toggle = useCallback(
(showOptions?: InternalPopoverOpenOptions) => {
if (layer.wasJustDismissed()) {
return;
}
Expand All @@ -497,7 +493,6 @@ function usePopoverImplementation(
},
[layer, show],
);
const toggle = useCallback(() => toggleWithOptions(), [toggleWithOptions]);

// ARIA attributes for the trigger
const triggerProps = {
Expand Down Expand Up @@ -589,7 +584,6 @@ function usePopoverImplementation(
show,
hide: layer.hide,
toggle,
toggleWithOptions,
wasJustDismissed: layer.wasJustDismissed,
isOpen: layer.isOpen,
id: layer.id,
Expand All @@ -599,11 +593,7 @@ function usePopoverImplementation(
}

export function usePopover(options: UsePopoverOptions = {}): UsePopoverReturn {
const {
wasJustDismissed: _,
toggleWithOptions: __,
...popover
} = usePopoverImplementation(options);
const {wasJustDismissed: _, ...popover} = usePopoverImplementation(options);
return popover;
}

Expand Down
Loading