[material-ui][Chip] The skipFocusWhenDisabled prop skips the focus when set to false and when disabled is true

[TLSSter]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Steps to reproduce 🕹

Link to live example:

Steps:

1. Go to the Mui chip link https://mui.com/material-ui/react-chip/
2. Goto the clickable section
3. update the example with the following code
   <Chip label="Clickable" disabled skipFocusWhenDisabled={false} onClick={handleClick} /> <Chip label="Clickable" variant="outlined" onClick={handleClick} />

Current behavior 😯

If you inspect the first chip component that is disabled if you set skipFocusWhenDisabled={true} the tabIndex is set to -1 which I think is correct if you then set skipFocusWhenDisabled={false} the tabIndex is still -1 which I think is incorrect.

Expected behavior 🤔

When disabled is set to true and skipFocusWhenDisabled={false} the Index should not be -1 it should allow it to have focus.

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo

  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

[aman44444]

@danilo-leal i want to work on this issue, can you please assign this to ?

[brijeshb42]

@aman44444 Assigned. Please let us know if you are facing any issues.

[aman44444]

@brijeshb42 i attempted to change the value of tabIndex both directly and indirectly, but it's not working. after debugging, i discovered that the value only changes when 'disabled' is applied. there is no effect of 'skipFocusWhenDisabled,' and i am sorry I couldn't determine how 'disabled' is changing the value. also sorry for the late reply.

[ZeeshanTamboli]

There's a bug in the skipFocusWhenDisabled prop added in #35065. We overlooked cases where the chip is both clickable and disabled when rendered as ButtonBase.

[aacevski]

Hey @ZeeshanTamboli, I want to take on this task but I'm a little confused. Accessibility wise, shouldn't a disabled Chip always have a tabIndex of -1? That is how it is in the ButtonBase component which imo is correct.

I have removed the usage of this prop in my playground and it works as expected, the Chip is not focused, I did this to recreate the issue/PR you have sent. I feel like I'm missing an important piece.

[ZeeshanTamboli]

@aacevski When the skipFocusWhenDisabled prop is set to false (either as its default value or when not explicitly provided to the component), the disabled Chip should be focusable. Conversely, when set to true, the disabled Chip should be non-focusable. The inclusion of this prop aimed to prevent a breaking change in #35065, as discussed in that thread. However, its current functionality falls short of expectations, working only when a disabled Chip has an onDelete prop, as shown in the example:

<Chip label="Chip Filled" onDelete={() => {}} disabled />

Considering the issues with its implementation, and ideal solution is to consider removing this prop and address #35038 properly, which the PR #35065 attempts to resolve. It's worth noting that such a change would introduce a breaking change.

[aacevski]

Okay thank you @ZeeshanTamboli, I'll try to take on this and update you soon. 🙌🏼