adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.71k stars 1.09k forks source link

Remix Link with useLink causes FocusRing to appear when clicked #6982

Closed guygit closed 4 weeks ago

guygit commented 4 weeks ago

Provide a general summary of the issue here

I need to create my own Link component for preserving the features Remix Link component offers and at the same time benefitting from what React-Aria's useLink contributes.

Everything works fine when I am using my Link component - however when I click it - it has this FocusRing around it. As far as I understood the docs - this behaviour is not intended as that FocusRing should only appear when the Link was 'tabbed' to (by keyboard navigation).

πŸ€” Expected Behavior?

The wrapped component should NOT show a FocusRing upon MouseClick

😯 Current Behavior

The wrapped component SHOWs a FocusRing upon MouseClick

πŸ’ Possible Solution

No response

πŸ”¦ Context

No response

πŸ–₯️ Steps to Reproduce

` // Use React Aria's useLink to enhance the accessibility const { linkProps } = useLink(ariaLinkProps, ref); if (typeof className === 'function') { return ( <RemixNavLink to={toWithLocale} className={className} {...restOfProps} {...linkProps} // Spread linkProps from useLink ref={ref} aria-disabled={isDisabled} // Optionally add aria-disabled if the link is disabled /> ); }

return ( <RemixLink to={toWithLocale} className={className} {...restOfProps} {...linkProps} // Spread linkProps from useLink ref={ref} aria-disabled={isDisabled} // Optionally add aria-disabled if the link is disabled /> );`

Version

1.33

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

mac

🧒 Your Company/Team

No response

πŸ•· Tracking Issue

No response

reidbarber commented 4 weeks ago

Are you seeing the browser's default focus outline style? If so, you can use the outline: none style, and add your own focus style that uses the isFocusVisible state from useFocusRing().

guygit commented 4 weeks ago

Not sure if it is the browers's default focus outline style - but when I omit the linkProps returned from useLink() - I mean if I don't spread them into the <RemixNavLink {...linkProps} then I don't see a FocusRing. So I assume, it is definitely coming from useLink() hook.

guygit commented 4 weeks ago

If I understand the docs right - the default behaviour is showing a FocusRing on Keyboard Navigation only. If that's the case, then this would be a bug I guess and I could workaround it I think - but I am not sure.

reidbarber commented 4 weeks ago

It isn't necessarily a bug, you'll just want to add outline: none. You'll notice that we do this in all our docs examples, starter kits, and in React Spectrum.

From @devongovett in https://github.com/adobe/react-spectrum/discussions/5318#discussioncomment-8259310:

native :focus-visible will show the focus ring when an element is programmatically focused whereas useFocusVisible does not. We use programmatic focus in a bunch of places and it's not always desirable to show the focus ring.

guygit commented 4 weeks ago

ok - thx, I am going to stick with that