ariakit / ariakit

Toolkit for building accessible web apps with React
https://ariakit.org
MIT License
7.87k stars 370 forks source link

CompositeItem invoking events when disabled and unfocusable #2343

Open georgekaran opened 1 year ago

georgekaran commented 1 year ago

Current behavior

Hey there!

I noticed an issue with the CompositeItem component, where it dispatches events when disabled and not focusable.

Steps to reproduce the bug

  1. Go to https://codesandbox.io/s/jolly-napier-r6pylq?file=/index.js
  2. Once you're in, click on the CompositeItem option.
  3. Even if the component is disabled, you should still see the click event being dispatched.
  4. If you remove the "focusable" prop (or make it focusable by default), the click event won't be dispatched anymore.

Expected behavior

Events such as clicks must not be triggered when the CompositeItem is disabled and cannot receive focus.

Workaround

For now, in the userland, I'm doing something like this:

<CompositeItem
    disabled={disabled}
    focusable={disabled || focusable}
/>

Possible solutions

Something related to this line: https://github.com/ariakit/ariakit/blob/8381061f08afeb52a54719b04e9c198e30bc531a/packages/ariakit-react-core/src/focusable/focusable.ts#L212

diegohaz commented 1 year ago

Could you share your use case for setting the focusable prop to false on a button?

We should make this more evident in the JSDocs, but this prop works similarly to the composite prop, where setting it to false disables the composite features on that component as if it weren't using <Composite> underneath. Setting focusable to false basically disables all features from the underlying Focusable component: focus visible, focus on mouse down on Safari, auto focus, etc.

Internally, our only use case for disabling this prop is on the composite container itself. When virtualFocus is set to true, the composite element becomes focusable and inherits all the Focusable component features. When virtualFocus is set to false, it's just a normal div.

stale[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.