Skip to content
Open
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 .changeset/select-scroll-button-jump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@radix-ui/react-select": patch
---

Fixed `Select` snapping the scroll position back to the selected item while scrolling with the pointer, wheel, or touch when `Select.ScrollUpButton` or `Select.ScrollDownButton` are present. The scroll-into-view that runs when a scroll button mounts now only fires when keyboard focus moves to a different item, so it no longer interferes with manual scrolling.
33 changes: 33 additions & 0 deletions apps/storybook/stories/select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,39 @@ export const ChromaticNoDefaultValue = () => (
);
ChromaticNoDefaultValue.parameters = { chromatic: { disable: false } };

export const CypressScrollButtons = () => (
<div style={{ display: 'flex', justifyContent: 'center', padding: 50 }}>
<Label>
Choose an item:
<Select.Root defaultValue="item-25">
<Select.Trigger className={styles.trigger}>
<Select.Value />
<Select.Icon />
</Select.Trigger>
<Select.Portal>
<Select.Content className={styles.content} position="popper" sideOffset={5}>
<Select.ScrollUpButton className={scrollUpButtonClass}>▲</Select.ScrollUpButton>
<Select.Viewport className={styles.viewport} style={{ maxHeight: 150 }}>
{Array.from({ length: 50 }, (_, i) => {
const value = `item-${i + 1}`;
return (
<Select.Item key={value} className={styles.item} value={value}>
<Select.ItemText>item {i + 1}</Select.ItemText>
<Select.ItemIndicator className={styles.indicator}>
<TickIcon />
</Select.ItemIndicator>
</Select.Item>
);
})}
</Select.Viewport>
<Select.ScrollDownButton className={scrollDownButtonClass}>▼</Select.ScrollDownButton>
</Select.Content>
</Select.Portal>
</Select.Root>
</Label>
</div>
);

export const Cypress = () => {
const [data, setData] = React.useState<{ size?: string; model?: string }>({});
const [model, setModel] = React.useState<string | undefined>('');
Expand Down
58 changes: 58 additions & 0 deletions cypress/e2e/Select.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,61 @@ describe('Select (shadow DOM)', () => {
});
});
});

describe('Select (scroll buttons)', () => {
beforeEach(() => {
cy.visitStory('select--cypress-scroll-buttons');
});

// https://github.com/radix-ui/primitives/issues/3686
it('does not snap the selected item back into view when a scroll button mounts mid-scroll', () => {
cy.findByLabelText(/choose an item/i).click();
cy.findByRole('listbox').should('be.visible');
cy.get('[data-radix-select-viewport]').as('viewport');

// Scroll to the very top so the up button unmounts and the selected item
// (item 25) sits well below the visible area.
cy.get('@viewport').scrollTo('top');

// Scroll down a little so the up button mounts again. Before the fix, the
// button's mount-time `scrollIntoView` yanked the viewport back to the
// selected item; now it should stay where the user scrolled.
cy.get('@viewport').scrollTo(0, 120);

cy.get('@viewport').then(($viewport) => {
const viewportRect = $viewport[0].getBoundingClientRect();
cy.findByRole('option', { name: /^item 25$/i }).then(($item) => {
const itemRect = $item[0].getBoundingClientRect();
expect(itemRect.top, 'selected item stays below the viewport').to.be.greaterThan(
viewportRect.bottom,
);
});
});
});

it('keeps the focused item visible while navigating with the keyboard', () => {
cy.findByLabelText(/choose an item/i).click();
cy.findByRole('listbox').should('be.visible');

// Walk focus up from the selected item; the scroll buttons mount/unmount as
// we move, and the focused item must remain within the viewport.
cy.realPress('ArrowUp');
cy.realPress('ArrowUp');
cy.realPress('ArrowUp');
cy.realPress('ArrowUp');
cy.realPress('ArrowUp');

cy.get('[data-radix-select-viewport]').then(($viewport) => {
const viewportRect = $viewport[0].getBoundingClientRect();
cy.focused().then(($item) => {
const itemRect = $item[0].getBoundingClientRect();
expect(itemRect.bottom, 'focused item is below the viewport top').to.be.greaterThan(
viewportRect.top,
);
expect(itemRect.top, 'focused item is above the viewport bottom').to.be.lessThan(
viewportRect.bottom,
);
});
});
});
});
21 changes: 19 additions & 2 deletions packages/react/select/src/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ type SelectContentContextValue = {
position?: SelectContentProps['position'];
isPositioned?: boolean;
searchRef?: React.RefObject<string>;
lastScrolledIntoViewItemRef?: React.RefObject<SelectItemElement | null>;
};

const [SelectContentProvider, useSelectContentContext] =
Expand Down Expand Up @@ -670,6 +671,9 @@ const SelectContentImpl = React.forwardRef<SelectContentImplElement, SelectConte
const getItems = useCollection(__scopeSelect);
const [isPositioned, setIsPositioned] = React.useState(false);
const firstValidItemFoundRef = React.useRef(false);
// Tracks the item the scroll buttons last scrolled into view, so the compensation
// effect only re-runs when keyboard focus moves to a different item (see #3686).
const lastScrolledIntoViewItemRef = React.useRef<SelectItemElement | null>(null);

// aria-hide everything except the content (better supported equivalent to setting aria-modal)
React.useEffect(() => {
Expand Down Expand Up @@ -835,6 +839,7 @@ const SelectContentImpl = React.forwardRef<SelectContentImplElement, SelectConte
position={position}
isPositioned={isPositioned}
searchRef={searchRef}
lastScrolledIntoViewItemRef={lastScrolledIntoViewItemRef}
>
<RemoveScroll as={Slot} allowPinchZoom>
<FocusScope
Expand Down Expand Up @@ -1666,10 +1671,22 @@ const SelectScrollButtonImpl = React.forwardRef<
// Because it is part of the normal flow, it will push down (top button) or shrink (bottom button)
// the viewport, potentially causing the active item to now be partially out of view.
// We re-run the `scrollIntoView` logic to make sure it stays within the viewport.
//
// The buttons also mount/unmount as the user scrolls across the top/bottom thresholds, so this
// effect re-runs mid-scroll. We only re-scroll when keyboard focus has moved to a *different*
// item (keyboard navigation moves `document.activeElement`); pointer/wheel/touch scrolling does
// not move focus, so it is no longer snapped back to the selected item. See #3686.
const { lastScrolledIntoViewItemRef } = contentContext;
useLayoutEffect(() => {
const activeItem = getItems().find((item) => item.ref.current === document.activeElement);
activeItem?.ref.current?.scrollIntoView({ block: 'nearest' });
}, [getItems]);
const activeNode = activeItem?.ref.current;
if (activeNode && activeNode !== lastScrolledIntoViewItemRef?.current) {
activeNode.scrollIntoView({ block: 'nearest' });
if (lastScrolledIntoViewItemRef) {
lastScrolledIntoViewItemRef.current = activeNode;
}
}
}, [getItems, lastScrolledIntoViewItemRef]);

return (
<Primitive.div
Expand Down