primitives
primitives copied to clipboard
[Select][Mobile] Select became opened while scrolling page
Bug report
Current Behavior
When a mobile user scroll the page touching Select, Select became opened. But the user just want to scroll page down. Not open Select.
Expected behavior
Select should not be opened when user scroll the page event if touch was started on Select.
Reproducible example
Suggested solution
As mentioned in https://github.com/radix-ui/primitives/pull/1082#issuecomment-1230197898 we may use onPointerUp/onClick instead of onPointerDown in https://github.com/radix-ui/primitives/blob/main/packages/react/select/src/Select.tsx#L244
Additional context
Sorry for scaling...
https://user-images.githubusercontent.com/3282725/187749713-210f089f-e238-4773-a28e-8561ff20474b.mp4
Your environment
Software | Name(s) | Version |
---|---|---|
Radix Package(s) | @radix-ui/react-select | 1.0.0 |
Browser | Chrome 105, Safari 15.6 | Chrome was emulated via Devices, Safari was checked on iPhone |
We stumbled upon this too in our current project. Why is it marked as an enhancement? It feels more like a bug. It goes against the philosophy of doing it like in macOS and makes it very easy to accidentally trigger. The example page (https://www.radix-ui.com/docs/primitives/components/select) exhibits the same behaviour.
I have a workaround/fix of this issue, which I will make into a PR soon if possibile.
The idea here is to make it so that you can use onClick on the trigger instead of the onPointerDown event it uses by default. To do this you will have to navigate to the node-modules/@radix-ui/react-slider/dist folder and open the index.modules.js file. In that file you want to add this to the SelectTrigger portion of the code:
`/* -------------------------------------------------------------------------------------------------
-
SelectTrigger
-
-----------------------------------------------------------------------------------------------/ const $cc7e05a45900e73f$var$TRIGGER_NAME = 'SelectTrigger'; const $cc7e05a45900e73f$export$3ac1e88a1c0b9f1 = /#PURE*/ $01b9c$forwardRef((props, forwardedRef)=>{ const { __scopeSelect: __scopeSelect , disabled: disabled = false , 'aria-labelledby': ariaLabelledby , ...triggerProps } = props; const context = $cc7e05a45900e73f$var$useSelectContext($cc7e05a45900e73f$var$TRIGGER_NAME, __scopeSelect); const composedRefs = $01b9c$useComposedRefs(forwardedRef, context.onTriggerChange); const getItems = $cc7e05a45900e73f$var$useCollection(__scopeSelect); const labelId = $01b9c$useLabelContext(context.trigger); const labelledBy = ariaLabelledby || labelId; const [searchRef, handleTypeaheadSearch, resetTypeahead] = $cc7e05a45900e73f$var$useTypeaheadSearch((search)=>{ const enabledItems = getItems().filter((item)=>!item.disabled ); const currentItem = enabledItems.find((item)=>item.value === context.value ); const nextItem = $cc7e05a45900e73f$var$findNextItem(enabledItems, search, currentItem); if (nextItem !== undefined) context.onValueChange(nextItem.value); }); const handleOpen = ()=>{ if (!disabled) { context.onOpenChange(true); // reset typeahead when we open resetTypeahead(); } };
return /#PURE/ $01b9c$createElement($01b9c$Primitive.button, $01b9c$babelruntimehelpersesmextends({ type: "button", role: "combobox", "aria-controls": context.contentId, "aria-expanded": context.open, "aria-autocomplete": "none", "aria-labelledby": labelledBy, dir: context.dir, "data-state": context.open ? 'open' : 'closed', disabled: disabled, "data-disabled": disabled ? '' : undefined, "data-placeholder": context.value === undefined ? '' : undefined }, triggerProps, { ref: composedRefs, onPointerDown: $01b9c$composeEventHandlers(triggerProps.onPointerDown, (event)=>{ // prevent implicit pointer capture // https://www.w3.org/TR/pointerevents3/#implicit-pointer-capture event.target.releasePointerCapture(event.pointerId); // 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) }; // prevent trigger from stealing focus from the active item after opening. event.preventDefault(); } }), onKeyDown: $01b9c$composeEventHandlers(triggerProps.onKeyDown, (event)=>{ const isTypingAhead = searchRef.current !== ''; const isModifierKey = event.ctrlKey || event.altKey || event.metaKey; if (!isModifierKey && event.key.length === 1) handleTypeaheadSearch(event.key); if (isTypingAhead && event.key === ' ') return; if ($cc7e05a45900e73f$var$OPEN_KEYS.includes(event.key)) { handleOpen(); event.preventDefault(); } }), onClick: $01b9c$composeEventHandlers(triggerProps.onClick, (event)=>{ handleOpen(); }) })); });`
Once you have added this to your code, you want to tell the trigger to preventDefault onPointerDown if you are on mobile, which will then allow for onClick to work (onClick will trigger the select unless explicitly tapped).
Using patch-package you are able to use this fix in your own project.
@jlputter, whilst this might resolve the issue on the surface, it will break other behaviours (like the ability to select a value in one click lifecycle (down, move, up).
…which I will make into a PR soon if possibile.
I am mostly letting you know as I don't want you to waste some of your personal time working on a PR for a fix I know we would reject.
Hope that makes sense!
✌️
@benoitgrelard absolutely. If I were to make a PR it would be a fix to the onPointerDown, not by just adding an OnClick. I do think, however, that having the possibility for a simple onClick would not be bad either.
@benoitgrelard
it will break other behaviours (like the ability to select a value in one click lifecycle (down, move, up).
I understand but I do think that it's way better to force the user to click two times in order to select a value than having this blocking scroll issue on mobile.
Yes the issue needs to be fixed for sure. In the meantime you are welcome to modify the component to your needs.
Any updates for this issue, guys?
I did some digging on how react-aria does it. They have an event abstraction
https://github.com/adobe/react-spectrum/blob/b35d5c02fe900badccd0cf1a8f23bb593419f238/packages/%40react-aria/menu/src/useMenuTrigger.ts#L106-L118
let pressProps = {
onPressStart(e) {
// For consistency with native, open the menu on mouse/key down, but touch up.
if (e.pointerType !== 'touch' && e.pointerType !== 'keyboard' && !isDisabled) {
// If opened with a screen reader, auto focus the first item.
// Otherwise, the menu itself will be focused.
state.toggle(e.pointerType === 'virtual' ? 'first' : null);
}
},
onPress(e) {
if (e.pointerType === 'touch' && !isDisabled) {
state.toggle();
}
}
};
These non-native events are plumbed through useButton
https://react-spectrum.adobe.com/react-aria/useButton.html
It was not trivial to implement this to Radix UI Select. I was able to prevent opening the select when touch scrolling in https://github.com/radix-ui/primitives/commit/552650931a57b3316dd488bbe1543da80d9ba6d6
However, trying to open the select on touch devices was something I couldn't get to work reliably. I don't know when I would have the time to take a stab at this the next time, but hopefully someone else wants to pick this up in the meanwhile.
Bump.. This issue still exists..
Any update on this issue? Does it have an easy fix?
I used patch package to trigger on onClick and comment out onPointerDown:
diff --git a/node_modules/@radix-ui/react-select/dist/index.mjs b/node_modules/@radix-ui/react-select/dist/index.mjs
index 53fb4de..aaa1d08 100644
--- a/node_modules/@radix-ui/react-select/dist/index.mjs
+++ b/node_modules/@radix-ui/react-select/dist/index.mjs
@@ -194,23 +194,24 @@ const $cc7e05a45900e73f$export$3ac1e88a1c0b9f1 = /*#__PURE__*/ $01b9c$forwardRef
// because we are preventing default in `onPointerDown` so effectively
// this only runs for a label "click"
event.currentTarget.focus();
+ handleOpen()
}),
- onPointerDown: $01b9c$composeEventHandlers(triggerProps.onPointerDown, (event)=>{
- // prevent implicit pointer capture
- // https://www.w3.org/TR/pointerevents3/#implicit-pointer-capture
- const target = event.target;
- if (target.hasPointerCapture(event.pointerId)) target.releasePointerCapture(event.pointerId);
- // 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)
- }; // prevent trigger from stealing focus from the active item after opening.
- event.preventDefault();
- }
- }),
+ // onPointerDown: $01b9c$composeEventHandlers(triggerProps.onPointerDown, (event)=>{
+ // // prevent implicit pointer capture
+ // // https://www.w3.org/TR/pointerevents3/#implicit-pointer-capture
+ // const target = event.target;
+ // if (target.hasPointerCapture(event.pointerId)) target.releasePointerCapture(event.pointerId);
+ // // 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)
+ // }; // prevent trigger from stealing focus from the active item after opening.
+ // event.preventDefault();
+ // }
+ // }),
onKeyDown: $01b9c$composeEventHandlers(triggerProps.onKeyDown, (event)=>{
const isTypingAhead = searchRef.current !== '';
const isModifierKey = event.ctrlKey || event.altKey || event.metaKey;
Not sure about the downsides, but seems to be good
Any updates on this? Or any workarounds? The issue is still there
Hi. Is there any plan on fixing this? We would really like to have some solutions on preventing drop down opening during the scroll/touch Thanks!
Workaround
You can fix this by overwriting onPointerDown
to {(e) => e.preventDefault()}
on the <DropdownMenu.Trigger />
& adding an onClick
event & onOpenChange
.
Before:
return (
<DropdownMenu.Root>
<DropdownMenu.Trigger>
{/* trigger content */}
</DropdownMenu.Trigger>
{/* other content */}
</DropdownMenu.Root>
);
After:
const [isOpen, setIsOpen] = useState(false);
return (
<DropdownMenu.Root open={isOpen} onOpenChange={setIsOpen}>
<DropdownMenu.Trigger
onPointerDown={(e) => e.preventDefault()}
onClick={() => setIsOpen((prev) => !prev)}
>
{/* trigger content */}
</DropdownMenu.Trigger>
{/* other content */}
</DropdownMenu.Root>
);
This makes the Dropdown work onClick
and scrolling events do not trigger it. Try out the video example ↗
https://github.com/radix-ui/primitives/assets/35841182/617fe834-fd3d-440b-96d3-9f9e3679a1b7
Here's a trimmed down version of the Dropdown component from the video:
export const Dropdown = ({ children, text, id, ...props }: Props) => {
const [isOpen, setIsOpen] = useState(false);
return (
<DropdownMenuPrimitive.Root open={isOpen} onOpenChange={setIsOpen}>
<DropdownMenuPrimitive.Trigger asChild>
<button
className={}
onPointerDown={(e) => e.preventDefault()}
onClick={() => setIsOpen((prev) => !prev)}
id={id}
{...props}
>
{text}
</button>
</DropdownMenuPrimitive.Trigger>
<DropdownMenuPrimitive.Portal>
<DropdownMenuPrimitive.Content
{...props}
className={}
sideOffset={8}
align="end"
>
{children}
</DropdownMenuPrimitive.Content>
</DropdownMenuPrimitive.Portal>
</DropdownMenuPrimitive.Root>
);
};
In case you're coming from shadcn/ui, here is my workaround
- Wrap the
<Select />
component by a context provider
const SelectContext = React.createContext<
React.Dispatch<React.SetStateAction<boolean>>
>(() => {});
const Select: React.FC<SelectPrimitive.SelectProps> = (props) => {
const [isOpen, setIsOpen] = React.useState(false);
return (
<SelectContext.Provider value={setIsOpen}>
<SelectPrimitive.Root open={isOpen} onOpenChange={setIsOpen} {...props} />
</SelectContext.Provider>
);
};
- Override the default behavior of
<SelectTrigger />
const SelectTrigger = ... => {
const setIsOpen = React.useContext(SelectContext);
return (
<SelectPrimitive.Trigger
onPointerDown={(e) => {
if (e.pointerType === "touch") e.preventDefault(); // disable the default behavior in mobile
}}
onPointerUp={(e) => {
if (e.pointerType === "touch") {
setIsOpen((prevState) => !prevState); // use onPointerUp to simulate onClick in mobile
}
}}
...
>
...
</SelectPrimitive.Trigger>
);
});
Difference between Aug 31, 2022 and May 25, 2024: 1 years 8 months 25 days or 20 months 25 days or 90 weeks 3 days or 633 calendar days
In case you're coming from shadcn/ui, here is my workaround
- Wrap the
<Select />
component by a context providerconst SelectContext = React.createContext< React.Dispatch<React.SetStateAction<boolean>> >(() => {}); const Select: React.FC<SelectPrimitive.SelectProps> = (props) => { const [isOpen, setIsOpen] = React.useState(false); return ( <SelectContext.Provider value={setIsOpen}> <SelectPrimitive.Root open={isOpen} onOpenChange={setIsOpen} {...props} /> </SelectContext.Provider> ); };
- Override the default behavior of
<SelectTrigger />
const SelectTrigger = ... => { const setIsOpen = React.useContext(SelectContext); return ( <SelectPrimitive.Trigger onPointerDown={(e) => { if (e.pointerType === "touch") e.preventDefault(); // disable the default behavior in mobile }} onPointerUp={(e) => { if (e.pointerType === "touch") { setIsOpen((prevState) => !prevState); // use onPointerUp to simulate onClick in mobile } }} ... > ... </SelectPrimitive.Trigger> ); });
nailed it