-
Notifications
You must be signed in to change notification settings - Fork 894
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] Can't differentiate if select trigger was focused from mouse or keyboard #1803
Comments
I'm facing the same issue with |
Hey @dlacaille,
As you correctly put it, unfortunately the browser doesn't expose a primitive for us to tell it whether it's a keyboard or pointer focus. This is really the issue here.
This was introduced for a different reason that has nothing to do with focus-visible. The reason is because we didn't want users to rely on I will close the issue as there isn't much we can do about this technically but feel free to discuss here if you have ideas you think we could explore! |
Just started using Radix and loving it so far (thank you!). The one issue that's annoying me (relative to other frameworks I've used) is there seems to be no way to make focus rings appear only for keyboard (not mouse) input. For example, React Aria does this by default: https://react-spectrum.adobe.com/react-aria/FocusRing.html Mantine makes it a theme option, and keyboard-only by default: https://mantine.dev/theming/theme-object/#focusring This flexibility is really nice from a design perspective, as you can have prominent focus rings to aid keyboard navigation without requiring them to appear for every mouse click. Many sites do this (such as this Github comment box :) ). Adding an option for this would also make it easier to incrementally adopt Radix alongside other component libraries. For example, I'd like to use Radix for Dropdown/Select components but already have other Mantine components in my project, and want to make focus ring behavior consistent across them. (thanks for considering!) |
I also have been really enjoying radix, but find this issue is leading me to remove |
This issue drove me nuts and caused me to abandon radix (and shadcn) for react-aria/rac, but I recently picked up another project that has radix and thankfully came across this post, which works well in conjunction w/ focus-visible.
credit: Thomas Stock |
That's not a very accessible solution @rolanday. The trigger should be focused on close, but it shouldn't be "visibly focused". |
@benface , agreed, which is precisely why I've moved to react-aria for my main work (i.e., I'm unwilling to compromise on accessibility). That said, hiding the focus outline full stop, which is a common pattern from ppl trying to resolve this radix issue is also not accessible (I'm calling it a radix issue because despite above comment, this issue is not present in other UI frameworks so it's solvable despite browser limitations). The focus ring not appearing on deselect via mouse interactions is table stakes. If you know of an accessible workaround , please please do share -- simply stating that the workarounds to address the issue are not accessibly friendly is not useful. Thanks. |
@rolanday – I actually do know and use a workaround, which I should have posted here before (thanks for asking me). I am using the |
@benoitgrelard – This issue should be re-opened. It's a real problem, which I believe you'll be able to fix with the new |
@benface , this is very helpful! Thanks you :-) +1 for request to reopen and +100 that this is a real problem. To be frank, I'm surprised radix-ui and derivatives like shadcn-ui are so popular if devs need to always hide the focus ring in order to ensure proper mouse/touch behavior at the expense of keyboard interaction/accessibility. |
@benface , thanks again for the pointer -- works well. Super appreciate. |
I'm not sure how Headless UI does it, but it doesn't have this issue: https://headlessui.com/react/listbox |
Completely understand this is open source, but just to gauge interest, this is also somewhat of a blocker for our use of Radix Select. |
I was frustrated with this behavior to say the least. As @benface pointed out, @rolanday's solution was not very accessible. While using a polyfill is an option, it's not ideal for those of us who prefer to avoid adding more JS and CSS to the frontend. I've found a solution that prevents autofocus only when the user uses a mouse. When using the keyboard, the focus-visible property will still work. This approach should save at least 1KB when gzipped, CSS will be a bit smaller, and it avoids adding several event listeners the polyfill would add. Here's a snippet: const Content = forwardRef<HTMLDivElement, DropdownMenuContentProps>(
({ onPointerDownOutside, onCloseAutoFocus, ...props }, ref) => {
const isCloseFromMouse = useRef<boolean>(false);
const handlePointerDownOutside = useCallback(
(e: unknown) => {
isCloseFromMouse.current = true;
// @ts-expect-error - they don't export the PointerDownOutsideEvent
onPointerDownOutside?.(e);
},
[onPointerDownOutside]
);
const handleCloseAutoFocus = useCallback(
(e: Event) => {
if (onCloseAutoFocus) {
return onCloseAutoFocus(e);
}
if (!isCloseFromMouse.current) {
return;
}
e.preventDefault();
isCloseFromMouse.current = false;
},
[onCloseAutoFocus]
);
return (
<Portal>
<RadixContent
ref={ref}
{...props}
onPointerDownOutside={handlePointerDownOutside}
onCloseAutoFocus={handleCloseAutoFocus}
/>
</Portal>
);
}
);
Content.displayName = 'DropdownMenu.Content'; |
This should be reopened. @benoitgrelard |
Why this issue keeps getting closed without actually fixing the issue? |
@felipedeboni thanks for the snippet! It works well in our scenario and I think it's a fair workaround to achieve that behavior. By the way JFYI, you can avoid disabling the TS check by getting the actual type of the type PointerDownOutsideEvent = Parameters<NonNullable<DropdownMenuContentProps['onPointerDownOutside']>>[0]; |
@CarlosLleraMartin @felipedeboni Sadly i think even that solution is not perfect. It only prevents the outline when clicked outside. Yet if an item is selected via a mouse it will still trigger the outline which is annoying, it should be there only when the items is selected via keyboard. Unfortunately I wasn't able to find a good way to implement that. Radix seems abandoned now so I switched to react-aria-components and I am using radix only for select few specific elements which I will probably replace in the future anyway. |
Although it is not perfect you can create a // ...
const handlePointerDownOutside = useCallback(
(e: unknown) => {
isCloseFromMouse.current = true;
// @ts-expect-error - they don't export the PointerDownOutsideEvent
onPointerDownOutside?.(e);
},
[onPointerDownOutside]
);
const handlePointerDown = useCallback(
(e: unknown) => {
isCloseFromMouse.current = true;
// @ts-expect-error - they don't export the PointerDownEvent
onPointerDown?.(e);
},
[onPointerDown]
);
// ... And // ...
<SelectPrimitive.Content
onPointerDown={handlePointerDown}
onPointerDownOutside={handlePointerDownOutside}
// ... Here is the edited version: const Content = forwardRef<HTMLDivElement, DropdownMenuContentProps>(
({ onPointerDownOutside, onCloseAutoFocus, ...props }, ref) => {
const isCloseFromMouse = useRef<boolean>(false);
const handlePointerDownOutside = useCallback(
(e: unknown) => {
isCloseFromMouse.current = true;
// @ts-expect-error - they don't export the PointerDownOutsideEvent
onPointerDownOutside?.(e);
},
[onPointerDownOutside]
);
const handlePointerDown = useCallback(
(e: unknown) => {
isCloseFromMouse.current = true;
// @ts-expect-error - they don't export the PointerDownEvent
onPointerDown?.(e);
},
[onPointerDown]
);
const handleCloseAutoFocus = useCallback(
(e: Event) => {
if (onCloseAutoFocus) {
return onCloseAutoFocus(e);
}
if (!isCloseFromMouse.current) {
return;
}
e.preventDefault();
isCloseFromMouse.current = false;
},
[onCloseAutoFocus]
);
return (
<Portal>
<RadixContent
ref={ref}
{...props}
onPointerDownOutside={handlePointerDownOutside}
onPointerDown={handlePointerDown}
onCloseAutoFocus={handleCloseAutoFocus}
/>
</Portal>
);
}
);
Content.displayName = 'DropdownMenu.Content'; |
Bug report
Current Behavior
There is currently no way to prevent highlighting a
<Select.Trigger>
when using the mouse.Expected behavior
Using
:focus-visible
we should be able to only show a<Select.Trigger>
element as focused when it is actually selected using the keyboard. Currently radix programmatically focuses the element, which prevents the browser from differentiating a keyboard focus from a mouse focus.Reproducible example
Code Sandbox
This is a slightly modified version of the Radix UI Select example where I changed the
:focus
selector to.SelectTrigger:focus-visible
to show that the mouse still triggers the browser's:focus-visible
property.Suggested solution
In other radix-ui components such as
<Select.Item>
we can use[data-highlighted]
, but it is not available for this component.Additional context
An old issue resolved by the data-highlighted attribute:
#1164
Your environment
The text was updated successfully, but these errors were encountered: