Skip to content

Commit

Permalink
[Select] Fix touch devices bugs (#2939)
Browse files Browse the repository at this point in the history
  • Loading branch information
vladmoroz committed Aug 21, 2024
1 parent c8e3323 commit 8a007a5
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .yarn/versions/cc138cf5.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
releases:
"@radix-ui/react-select": patch

declined:
- primitives
10 changes: 8 additions & 2 deletions packages/react/select/src/Select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,9 @@ export const Cypress = () => {
</Select.Portal>
</Select.Root>
</Label>
<button type="submit">buy</button>
<button type="submit" style={{ width: 100, height: 50 }}>
buy
</button>
{data.size ? <p>You picked t-shirt size {data.size}</p> : null}
</form>

Expand Down Expand Up @@ -876,7 +878,7 @@ export const Cypress = () => {
</Select.Root>
</Label>

<button type="button" onClick={() => setModel('')}>
<button type="button" style={{ width: 100, height: 50 }} onClick={() => setModel('')}>
unset
</button>
</div>
Expand Down Expand Up @@ -1107,6 +1109,10 @@ const itemClass = css({
position: 'relative',
outline: 'none',

'&:active': {
backgroundColor: '$gray100',
},

'&[data-highlighted]': {
backgroundColor: '$black',
color: 'white',
Expand Down
46 changes: 36 additions & 10 deletions packages/react/select/src/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ const SelectTrigger = React.forwardRef<SelectTriggerElement, SelectTriggerProps>
const isDisabled = context.disabled || disabled;
const composedRefs = useComposedRefs(forwardedRef, context.onTriggerChange);
const getItems = useCollection(__scopeSelect);
const pointerTypeRef = React.useRef<React.PointerEvent['pointerType']>('touch');

const [searchRef, handleTypeaheadSearch, resetTypeahead] = useTypeaheadSearch((search) => {
const enabledItems = getItems().filter((item) => !item.disabled);
Expand All @@ -229,12 +230,19 @@ const SelectTrigger = React.forwardRef<SelectTriggerElement, SelectTriggerProps>
}
});

const handleOpen = () => {
const handleOpen = (pointerEvent?: React.MouseEvent | React.PointerEvent) => {
if (!isDisabled) {
context.onOpenChange(true);
// reset typeahead when we open
resetTypeahead();
}

if (pointerEvent) {
context.triggerPointerDownPosRef.current = {
x: Math.round(pointerEvent.pageX),
y: Math.round(pointerEvent.pageY),
};
}
};

return (
Expand All @@ -261,8 +269,15 @@ const SelectTrigger = React.forwardRef<SelectTriggerElement, SelectTriggerProps>
// because we are preventing default in `onPointerDown` so effectively
// this only runs for a label "click"
event.currentTarget.focus();

// Open on click when using a touch or pen device
if (pointerTypeRef.current !== 'mouse') {
handleOpen(event);
}
})}
onPointerDown={composeEventHandlers(triggerProps.onPointerDown, (event) => {
pointerTypeRef.current = event.pointerType;

// prevent implicit pointer capture
// https://www.w3.org/TR/pointerevents3/#implicit-pointer-capture
const target = event.target as HTMLElement;
Expand All @@ -271,13 +286,10 @@ const SelectTrigger = React.forwardRef<SelectTriggerElement, SelectTriggerProps>
}

// only call handler if it's the left button (mousedown gets triggered by all mouse buttons)
// but not when the control key is pressed (avoiding MacOS right click)
if (event.button === 0 && event.ctrlKey === false) {
handleOpen();
context.triggerPointerDownPosRef.current = {
x: Math.round(event.pageX),
y: Math.round(event.pageY),
};
// but not when the control key is pressed (avoiding MacOS right click); also not for touch
// devices because that would open the menu on scroll. (pen devices behave as touch on iOS).
if (event.button === 0 && event.ctrlKey === false && event.pointerType === 'mouse') {
handleOpen(event);
// prevent trigger from stealing focus from the active item after opening.
event.preventDefault();
}
Expand Down Expand Up @@ -1196,6 +1208,7 @@ const SelectItem = React.forwardRef<SelectItemElement, SelectItemProps>(
contentContext.itemRefCallback?.(node, value, disabled)
);
const textId = useId();
const pointerTypeRef = React.useRef<React.PointerEvent['pointerType']>('touch');

const handleSelect = () => {
if (!disabled) {
Expand Down Expand Up @@ -1241,11 +1254,24 @@ const SelectItem = React.forwardRef<SelectItemElement, SelectItemProps>(
ref={composedRefs}
onFocus={composeEventHandlers(itemProps.onFocus, () => setIsFocused(true))}
onBlur={composeEventHandlers(itemProps.onBlur, () => setIsFocused(false))}
onPointerUp={composeEventHandlers(itemProps.onPointerUp, handleSelect)}
onClick={composeEventHandlers(itemProps.onClick, () => {
// Open on click when using a touch or pen device
if (pointerTypeRef.current !== 'mouse') handleSelect();
})}
onPointerUp={composeEventHandlers(itemProps.onPointerUp, () => {
// Using a mouse you should be able to do pointer down, move through
// the list, and release the pointer over the item to select it.
if (pointerTypeRef.current === 'mouse') handleSelect();
})}
onPointerDown={composeEventHandlers(itemProps.onPointerDown, (event) => {
pointerTypeRef.current = event.pointerType;
})}
onPointerMove={composeEventHandlers(itemProps.onPointerMove, (event) => {
// Remember pointer type when sliding over to this item from another one
pointerTypeRef.current = event.pointerType;
if (disabled) {
contentContext.onItemLeave?.();
} else {
} else if (pointerTypeRef.current === 'mouse') {
// even though safari doesn't support this option, it's acceptable
// as it only means it might scroll a few pixels when using the pointer.
event.currentTarget.focus({ preventScroll: true });
Expand Down

1 comment on commit 8a007a5

@SindelarPetr
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

Please sign in to comment.