Skip to content
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

[joy-ui][Accordion] Expanding breaks the AccordionDetails children tab index #40116

Closed
2 tasks done
vanessag opened this issue Dec 6, 2023 · 7 comments · Fixed by #43246
Closed
2 tasks done

[joy-ui][Accordion] Expanding breaks the AccordionDetails children tab index #40116

vanessag opened this issue Dec 6, 2023 · 7 comments · Fixed by #43246
Assignees
Labels
bug 🐛 Something doesn't work component: accordion This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy

Comments

@vanessag
Copy link

vanessag commented Dec 6, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Steps:

  1. Create an Accordion component with 2 inputs inside of the AccordionDetails
  2. Expand the Accordion
  3. Click the first input inside of the accordion and then press tab
  4. Observe that the focus does not go to the next input

Current behavior 😯

When I click the Accordion to expand it and then click on the first Input within my AccordionDetails then press tab it does not focus the next Input.

Expected behavior 🤔

I expect that when I click on the Accordion to expand it and then click on an Input that is inside of AccordionDetails that pressing tab goes to the next Input.

Context 🔦

I am trying to accomplish an Accordion component that has form elements inside of it. These are advanced settings that the user can expand/collapse.

Your environment 🌎

npx @mui/envinfo
  System:
    OS: macOS 14.1.1
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    Yarn: Not Found
    npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
  Browsers:
    Chrome: 119.0.6045.199
  npmPackages:
    @emotion/react: ^11.11.1 => 11.11.1 
    @emotion/styled: ^11.11.0 => 11.11.0 
    @mui/base: ^5.0.0-beta.22 => 5.0.0-beta.22 
    @mui/core-downloads-tracker:  5.14.16 
    @mui/icons-material: ^5.14.16 => 5.14.16 
    @mui/joy: ^5.0.0-beta.13 => 5.0.0-beta.13 
    @mui/material:  5.14.16 
    @mui/private-theming:  5.14.16 
    @mui/styled-engine:  5.14.16 
    @mui/system:  5.14.16 
    @mui/types:  7.2.8 
    @mui/utils:  5.14.16 
    @types/react: ^18.2.34 => 18.2.34 
    react: ^18.2.0 => 18.2.0 
    react-dom: ^18.2.0 => 18.2.0 
    typescript: ^5.2.2 => 5.2.2 
@vanessag vanessag added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 6, 2023
@danilo-leal danilo-leal changed the title [Joy UI] [Accordion] Expanding accordion breaks AccordionDetails children tab index [joy-ui][Accordion] Expanding breaks the AccordionDetails children tab index Dec 6, 2023
@danilo-leal danilo-leal added component: accordion This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy labels Dec 6, 2023
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Dec 11, 2023

Why do you need inputs inside the AccordionDetails? I think I have never seen that. What is your use case? Please provide a CodeSandbox reproducing the issue.

@ZeeshanTamboli ZeeshanTamboli added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 11, 2023
@JessiPio4cronn
Copy link

Why do you need inputs inside the AccordionDetails? I think I have never seen that. What is your use case? Please provide a CodeSandbox reproducing the issue.

See e.g. this example of the Joy UI documentation.

Furthermore, this would be the recommended behaviour according to the ARIA Authoring Practices Guide.

Tab: Moves focus to the next focusable element; all focusable elements in the accordion are included in the page Tab sequence.

@basti4242
Copy link

The issue seems to be, that certain elements (a, button, input, textarea, select, details) get a tabIndex of -1 when inside an AccordionDetails component. The implementation implies that this should only be the case when the accordion is closed and the regular tabIndex should be restored when the accordion is expanded again.

When I explicitly set a tabIndex of 0 for a component inside an AccordionDetails component (e.g. for a Textarea: <Textarea slotProps={{ textarea: { tabIndex: 0 } }} />, the mechanism is working. When no tabIndex is explicitly set, the tabIndex is -1 even if the accordion is expanded. I think this is a bug that should be fixed.

@JessiPio4cronn
Copy link

JessiPio4cronn commented Apr 3, 2024

The solution of @basti4242 only seems to work, if the accordion is initially expanded (defaultExpanded is set to true). Otherwise the tabIndex and data-prev-tabindex will be set to -1 during initialisation:

joy-ui-accordion-tab-index-bug.mov

It would be beneficial to add two more test for these cases.

  1. Test that the tab index is 0 when defaultExpanded is set to false and the accordion is expanded for the first time.
  2. Test that the tab index is 0 when the accordion is collapsed and expanded again.

@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work ready to take Help wanted. Guidance available. There is a high chance the change will be accepted and removed status: waiting for author Issue with insufficient information labels Apr 6, 2024
@jacobmoshipco
Copy link
Contributor

jacobmoshipco commented May 31, 2024

Took a look at this because I also ran into this problem.

I'm pretty sure this line needs to have the && !currentTabIndex condition removed. currentTabIndex will always be "-1" after the first render, so this will fail to remove the attribute unless it already doesn't exist.

The portion that adds the tab-index="-1" will mistakingly set data-prev-tabindex to "-1" when double-rendered, such as what happens when in react development mode. When testing this, making that previous change fixed things in production but not development.

The other issue with this partial fix is that elements with an explicit tab-index in a default expanded details will get their tab index cleared. It would probably be better to use an explicit data-prev-tabindex="unset" or something to keep track.

@Shreypatel13ll
Copy link
Contributor

If no one is working can I take this up.

Shreypatel13ll added a commit to Shreypatel13ll/material-ui that referenced this issue Aug 9, 2024
Shreypatel13ll added a commit to Shreypatel13ll/material-ui that referenced this issue Aug 9, 2024
@aarongarciah aarongarciah self-assigned this Aug 12, 2024
Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

We value your feedback @vanessag! How was your experience with our support team?
We'd love to hear your thoughts in this brief Support Satisfaction survey. Your insights help us improve!

@aarongarciah aarongarciah removed the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: accordion This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants