Skip to content
Merged
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
6 changes: 6 additions & 0 deletions .changeset/famous-results-bow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@radix-ui/react-popper": patch
"radix-ui": patch
---

Fix regression in popper that caused submenu misalignment when using custom portals
6 changes: 6 additions & 0 deletions .changeset/select-clearable-value.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@radix-ui/react-select": patch
"radix-ui": patch
---

Allow a `Select.Item` with an empty string value to act as a "clear" option. Selecting it resets the selection back to the placeholder, restoring the native `<select>` behavior for optional selects.
92 changes: 92 additions & 0 deletions apps/storybook/stories/popper.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from 'react';
import * as ReactDOM from 'react-dom';
import { Portal } from 'radix-ui';
import cx from 'clsx';
import { Popper } from 'radix-ui/internal';
Expand Down Expand Up @@ -195,6 +196,97 @@ export const WithUpdatePositionStrategyAlways = () => {
);
};

// Regression test for https://github.com/radix-ui/primitives/pull/3237

// When `hideWhenDetached` is set and no explicit `collisionBoundary` is passed,
// the `hide` middleware must detect the anchor being clipped by its scroll
// container, not just the viewport. The anchor below is scrolled out of its
// container while remaining inside the viewport, so the content should hide.
export const HideWhenDetachedInScrollContainer = () => {
const [container, setContainer] = React.useState<HTMLDivElement | null>(null);
return (
<div style={{ paddingTop: 300, paddingLeft: 50 }}>
<div
ref={setContainer}
data-testid="scroll-container"
style={{ width: 300, height: 200, overflow: 'auto', border: '1px solid black' }}
>
<div style={{ height: 900, paddingTop: 20 }}>
<Popper.Root>
<Popper.Anchor
data-testid="anchor"
className={styles.anchor}
style={{ width: 80, height: 40 }}
>
anchor
</Popper.Anchor>
{container && (
<Portal.Root asChild>
<Popper.Content
data-testid="content"
className={styles.content}
side="right"
sideOffset={5}
hideWhenDetached
>
content
</Popper.Content>
</Portal.Root>
)}
</Popper.Root>
</div>
</div>
</div>
);
};

// When content is rendered via a custom portal into a transformed +
// overflow-clipping container and no explicit `collisionBoundary` is passed,
// the available size must be measured against the viewport (the documented
// default), NOT clamped to the portal host. Here the host is only 120px tall,
// so a correct `--radix-popper-available-height` should be far larger than
// that.
export const AvailableSizeWithCustomPortal = () => {
const [host, setHost] = React.useState<HTMLDivElement | null>(null);
return (
<div style={{ padding: 50 }}>
<Popper.Root>
<Popper.Anchor
data-testid="anchor"
className={styles.anchor}
style={{ width: 80, height: 40 }}
>
anchor
</Popper.Anchor>
{host &&
ReactDOM.createPortal(
<Popper.Content data-testid="content" className={styles.content} sideOffset={5}>
content
</Popper.Content>,
host,
)}
</Popper.Root>

{/* A custom portal host that both establishes a containing block (via
`transform`) and clips its children (via `overflow: hidden`), which is
what makes Floating UI treat it as a clipping ancestor. */}
<div
ref={setHost}
data-testid="portal-host"
style={{
position: 'fixed',
top: 0,
left: 0,
width: 120,
height: 120,
overflow: 'hidden',
transform: 'translateZ(0)',
}}
/>
</div>
);
};

export const Chromatic = () => {
const [scrollContainer1, setScrollContainer1] = React.useState<HTMLDivElement | null>(null);
const [scrollContainer2, setScrollContainer2] = React.useState<HTMLDivElement | null>(null);
Expand Down
54 changes: 54 additions & 0 deletions apps/storybook/stories/select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,60 @@ export const NoDefaultValue = () => (
</div>
);

export const Clearable = () => (
<div
style={{
display: 'flex',
gap: 20,
alignItems: 'center',
justifyContent: 'center',
height: '100vh',
}}
>
<Label>
Choose a number (optional):
<Select.Root defaultValue="two">
<Select.Trigger className={styles.trigger}>
<Select.Value placeholder="Pick an option" />
<Select.Icon />
</Select.Trigger>
<Select.Portal>
<Select.Content className={styles.content} sideOffset={5}>
<Select.Viewport className={styles.viewport}>
{/* An item with an empty value resets the selection back to the placeholder */}
<Select.Item className={styles.item} value="">
<Select.ItemText>None</Select.ItemText>
<Select.ItemIndicator className={styles.indicator}>
<TickIcon />
</Select.ItemIndicator>
</Select.Item>
<Select.Item className={styles.item} value="one">
<Select.ItemText>One</Select.ItemText>
<Select.ItemIndicator className={styles.indicator}>
<TickIcon />
</Select.ItemIndicator>
</Select.Item>
<Select.Item className={styles.item} value="two">
<Select.ItemText>Two</Select.ItemText>
<Select.ItemIndicator className={styles.indicator}>
<TickIcon />
</Select.ItemIndicator>
</Select.Item>
<Select.Item className={styles.item} value="three">
<Select.ItemText>Three</Select.ItemText>
<Select.ItemIndicator className={styles.indicator}>
<TickIcon />
</Select.ItemIndicator>
</Select.Item>
</Select.Viewport>
<Select.Arrow />
</Select.Content>
</Select.Portal>
</Select.Root>
</Label>
</div>
);

export const Typeahead = () => (
<div
style={{
Expand Down
43 changes: 43 additions & 0 deletions cypress/e2e/Popper.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// `Popper` stories live under the `Utilities/*` namespace, so they can't be
// reached through the `components-` prefixed `cy.visitStory` helper.
function visitPopperStory(storyName: string) {
return cy.visit(`iframe.html?id=utilities-popper--${storyName}`);
}

describe('Popper', () => {
// Regression test for https://github.com/radix-ui/primitives/pull/3237
describe('given hideWhenDetached and no explicit collisionBoundary', () => {
beforeEach(() => {
visitPopperStory('hide-when-detached-in-scroll-container');
});

it('hides the content when the anchor is clipped by its scroll container but still within the viewport', () => {
cy.findByTestId('content').should('be.visible');

// Scroll the anchor out of the (200px tall) scroll container while keeping
// it on screen. Detach detection must fall back to clipping ancestors, not
// the viewport, otherwise the content would incorrectly stay visible.
cy.findByTestId('scroll-container').scrollTo(0, 200);

cy.findByTestId('content').should('not.be.visible');
});
});

// Regression test for the 1.3.0 custom-portal regression.
describe('given content in a transformed, overflow-clipping custom portal host', () => {
beforeEach(() => {
visitPopperStory('available-size-with-custom-portal');
});

it('measures available size against the viewport, not the portal host', () => {
// The portal host is only 120px tall. If the collision/size middlewares
// clamped to it (the regression), available height would be <= ~120px.
cy.get('[data-radix-popper-content-wrapper]').should(($wrapper) => {
const availableHeight = parseFloat(
getComputedStyle($wrapper[0]).getPropertyValue('--radix-popper-available-height'),
);
expect(availableHeight).to.be.greaterThan(300);
});
});
});
});
26 changes: 17 additions & 9 deletions packages/react/popper/src/popper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ const PopperContent = React.forwardRef<PopperContentElement, PopperContentProps>
alignOffset = 0,
arrowPadding = 0,
avoidCollisions = true,
collisionBoundary,
collisionBoundary = [],
collisionPadding: collisionPaddingProp = 0,
sticky = 'partial',
hideWhenDetached = false,
Expand All @@ -206,16 +206,12 @@ const PopperContent = React.forwardRef<PopperContentElement, PopperContentProps>
? collisionPaddingProp
: { top: 0, right: 0, bottom: 0, left: 0, ...collisionPaddingProp };

const boundary = collisionBoundary
? Array.isArray(collisionBoundary)
? collisionBoundary
: [collisionBoundary]
: undefined;
const hasExplicitBoundaries = boundary !== undefined && boundary.length > 0;
const boundary = Array.isArray(collisionBoundary) ? collisionBoundary : [collisionBoundary];
const hasExplicitBoundaries = boundary.length > 0;

const detectOverflowOptions = {
padding: collisionPadding,
boundary: boundary?.filter(isNotNull),
boundary: boundary.filter(isNotNull),
// with `strategy: 'fixed'`, this is the only way to get it to respect boundaries
altBoundary: hasExplicitBoundaries,
};
Expand Down Expand Up @@ -256,7 +252,19 @@ const PopperContent = React.forwardRef<PopperContentElement, PopperContentProps>
}),
arrow && floatingUIarrow({ element: arrow, padding: arrowPadding }),
transformOrigin({ arrowWidth, arrowHeight }),
hideWhenDetached && hide({ strategy: 'referenceHidden', ...detectOverflowOptions }),
hideWhenDetached &&
hide({
strategy: 'referenceHidden',
...detectOverflowOptions,
// `hide` detects whether the anchor (reference) is clipped, so when
// no explicit `collisionBoundary` is set we fall back to Floating
// UI's default clipping ancestors (e.g. a scrollable menu). This
// lets an occluded submenu hide once its anchor scrolls out of view
// (#3237). The collision/size middlewares deliberately keep the
// viewport-based default to avoid clamping content rendered inside
// transformed or overflow-clipping portal containers.
boundary: hasExplicitBoundaries ? detectOverflowOptions.boundary : undefined,
}),
],
});

Expand Down
95 changes: 94 additions & 1 deletion packages/react/select/src/select.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import * as React from 'react';
import { cleanup, render, screen, waitFor } from '@testing-library/react';
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { afterEach, describe, it, expect } from 'vitest';
import * as Select from './select';

const PLACEHOLDER_TEXT = 'Pick one';
const CLEAR_TEXT = 'None';

const SelectTest = (props: React.ComponentProps<typeof Select.Root>) => (
<Select.Root {...props}>
Expand All @@ -25,6 +26,29 @@ const SelectTest = (props: React.ComponentProps<typeof Select.Root>) => (
</Select.Root>
);

const SelectClearableTest = (props: React.ComponentProps<typeof Select.Root>) => (
<Select.Root {...props}>
<Select.Trigger aria-label="Choice">
<Select.Value placeholder={PLACEHOLDER_TEXT} />
</Select.Trigger>
<Select.Portal>
<Select.Content position="popper">
<Select.Viewport>
<Select.Item value="">
<Select.ItemText>{CLEAR_TEXT}</Select.ItemText>
</Select.Item>
<Select.Item value="apple">
<Select.ItemText>Apple</Select.ItemText>
</Select.Item>
<Select.Item value="banana">
<Select.ItemText>Banana</Select.ItemText>
</Select.Item>
</Select.Viewport>
</Select.Content>
</Select.Portal>
</Select.Root>
);

describe('aria-controls', () => {
afterEach(cleanup);

Expand All @@ -47,3 +71,72 @@ describe('aria-controls', () => {
expect(document.getElementById(content.id)).toBe(content);
});
});

describe('clearing an optional value (#2706)', () => {
afterEach(cleanup);

it('allows a `Select.Item` with an empty string value to be rendered', async () => {
expect(() => render(<SelectClearableTest defaultOpen />)).not.toThrow();
const clearItem = await waitFor(() => screen.getByText(CLEAR_TEXT));
expect(clearItem).toBeInTheDocument();
});

it('marks the empty value item as selected when the value is empty', async () => {
render(<SelectClearableTest defaultOpen value="" />);
const clearItem = await waitFor(() =>
screen.getByRole('option', { name: CLEAR_TEXT, hidden: true }),
);
expect(clearItem).toHaveAttribute('data-state', 'checked');
});

it('shows the placeholder (not the item text) when the empty value item is selected', async () => {
render(<SelectClearableTest value="" />);
const trigger = screen.getByRole('combobox');
expect(trigger).toHaveTextContent(PLACEHOLDER_TEXT);
expect(trigger).not.toHaveTextContent(CLEAR_TEXT);
expect(trigger).toHaveAttribute('data-placeholder');
});

it('lets the user reset a previously selected value back to the placeholder', async () => {
function ControlledSelect() {
const [value, setValue] = React.useState<string | undefined>('apple');
return <SelectClearableTest value={value} onValueChange={setValue} defaultOpen />;
}

render(<ControlledSelect />);
const trigger = screen.getByRole('combobox', { hidden: true });

// Value starts as "apple" and the trigger reflects it.
await waitFor(() => expect(trigger).toHaveTextContent('Apple'));
expect(trigger).not.toHaveAttribute('data-placeholder');

// Selecting the empty value item clears the selection.
const clearItem = screen.getByRole('option', { name: CLEAR_TEXT, hidden: true });
fireEvent.click(clearItem);

await waitFor(() => expect(trigger).toHaveTextContent(PLACEHOLDER_TEXT));
expect(trigger).not.toHaveTextContent('Apple');
expect(trigger).toHaveAttribute('data-placeholder');
});

it('renders a single empty native option when a clear item is provided', async () => {
const { container } = render(
<form>
<SelectClearableTest name="fruit" value="" defaultOpen />
</form>,
);
// Wait for items to register their native options.
await waitFor(() => {
const nativeSelect = container.querySelector('select');
const optionValues = Array.from(nativeSelect?.querySelectorAll('option') ?? []).map(
(o) => o.value,
);
expect(optionValues).toContain('apple');
});
const nativeSelect = container.querySelector('select');
const emptyOptions = Array.from(nativeSelect?.querySelectorAll('option') ?? []).filter(
(option) => option.value === '',
);
expect(emptyOptions).toHaveLength(1);
});
});
Loading
Loading