Select: Prevent negative starting position (#3149)

radix-ui · Sep 27, 2024 · a555a40 · a555a40
1 parent 53287e0
commit a555a40
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 3 deletions.
diff --git a/.yarn/versions/9cbffb59.yml b/.yarn/versions/9cbffb59.yml
@@ -0,0 +1,5 @@
+releases:
+  "@radix-ui/react-select": patch
+
+declined:
+  - primitives
diff --git a/packages/react/select/src/Select.stories.tsx b/packages/react/select/src/Select.stories.tsx
@@ -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" />
 );
@@ -1055,6 +1093,7 @@ const triggerClass = css({
   fontFamily: '-apple-system, BlinkMacSystemFont, helvetica, arial, sans-serif',
   fontSize: 13,
   lineHeight: 1,
+  overflow: 'hidden',
 
   '&:focus': {
     outline: 'none',

diff --git a/packages/react/select/src/Select.tsx b/packages/react/select/src/Select.tsx
@@ -827,7 +827,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';
@@ -838,7 +846,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';
@@ -1083,7 +1094,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) => {