Skip to content
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.

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

Select: Prevent negative starting position #3149

Merged
merged 1 commit into from
Sep 27, 2024
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
5 changes: 5 additions & 0 deletions .yarn/versions/9cbffb59.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
releases:
"@radix-ui/react-select": patch

declined:
- primitives
39 changes: 39 additions & 0 deletions packages/react/select/src/Select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,44 @@ export const WithinDialog = () => (
</div>
);

export const WithVeryLongSelectItems = () => (
<div style={{ paddingLeft: 300 }}>
<Label>
What is the meaning of life?
<Select.Root defaultValue="1">
<Select.Trigger className={triggerClass()}>
<Select.Value />
<Select.Icon />
</Select.Trigger>
<Select.Portal>
<Select.Content className={contentClass()}>
<Select.ScrollUpButton className={scrollUpButtonClass()}>▲</Select.ScrollUpButton>
<Select.Viewport className={viewportClass()}>
{[
'The meaning of life is a complex topic that has been the subject of much philosophical, scientific, and theological speculation, with no definitive answer. The meaning of life can be interpreted in many different ways, depending on individual beliefs, values, and experiences.',
'42',
].map((opt, i) => (
<Select.Item
key={opt}
className={itemClass()}
value={String(i)}
// style={{ maxWidth: 500 }}
>
<Select.ItemText>{opt}</Select.ItemText>
<Select.ItemIndicator className={indicatorClass()}>
<TickIcon />
</Select.ItemIndicator>
</Select.Item>
))}
</Select.Viewport>
<Select.ScrollDownButton className={scrollDownButtonClass()}>▼</Select.ScrollDownButton>
</Select.Content>
</Select.Portal>
</Select.Root>
</Label>
</div>
);

export const ChromaticShortOptionsPaddedContent = () => (
<ChromaticStoryShortOptions paddedElement="content" />
);
Expand Down Expand Up @@ -1055,6 +1093,7 @@ const triggerClass = css({
fontFamily: '-apple-system, BlinkMacSystemFont, helvetica, arial, sans-serif',
fontSize: 13,
lineHeight: 1,
overflow: 'hidden',

'&:focus': {
outline: 'none',
Expand Down
21 changes: 18 additions & 3 deletions packages/react/select/src/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,15 @@ const SelectItemAlignedPosition = React.forwardRef<
const minContentWidth = triggerRect.width + leftDelta;
const contentWidth = Math.max(minContentWidth, contentRect.width);
const rightEdge = window.innerWidth - CONTENT_MARGIN;
const clampedLeft = clamp(left, [CONTENT_MARGIN, rightEdge - contentWidth]);
const clampedLeft = clamp(left, [
CONTENT_MARGIN,
// Prevents the content from going off the starting edge of the
// viewport. It may still go off the ending edge, but this can be
// controlled by the user since they may want to manage overflow in a
// specific way.
// https://github.com/radix-ui/primitives/issues/2049
Math.max(CONTENT_MARGIN, rightEdge - contentWidth),
]);

contentWrapper.style.minWidth = minContentWidth + 'px';
contentWrapper.style.left = clampedLeft + 'px';
Expand All @@ -835,7 +843,10 @@ const SelectItemAlignedPosition = React.forwardRef<
const minContentWidth = triggerRect.width + rightDelta;
const contentWidth = Math.max(minContentWidth, contentRect.width);
const leftEdge = window.innerWidth - CONTENT_MARGIN;
const clampedRight = clamp(right, [CONTENT_MARGIN, leftEdge - contentWidth]);
const clampedRight = clamp(right, [
CONTENT_MARGIN,
Math.max(CONTENT_MARGIN, leftEdge - contentWidth),
]);

contentWrapper.style.minWidth = minContentWidth + 'px';
contentWrapper.style.right = clampedRight + 'px';
Expand Down Expand Up @@ -1080,7 +1091,11 @@ const SelectViewport = React.forwardRef<SelectViewportElement, SelectViewportPro
// (independent of the scrollUpButton).
position: 'relative',
flex: 1,
overflow: 'auto',
// Viewport should only be scrollable in the vertical direction.
// This won't work in vertical writing modes, so we'll need to
// revisit this if/when that is supported
// https://developer.chrome.com/blog/vertical-form-controls
overflow: 'hidden auto',
...viewportProps.style,
}}
onScroll={composeEventHandlers(viewportProps.onScroll, (event) => {
Expand Down
Loading