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

fix(Accordion): close other items in circular order #735

Merged
merged 2 commits into from
Sep 24, 2023

Conversation

Haythamasalama
Copy link
Contributor

@Haythamasalama Haythamasalama commented Sep 24, 2023

πŸ”— Linked issue

#626

❓ Type of change

  • 🐞 Bug fix (a non-breaking change that fixes an issue)

πŸ“š Description

The closeOther function, which closes items by index in ascending order (1, 2, 3, 4, etc.), can lead to a bug when users try to close items using the keyboard. To solve this issue, we need to close open items in a circular order. Here's an example:

  1. When the currentIndex is 0, close items in the order 2, 3, 4, 1.
  2. When the currentIndex is 1, close items in the order 3, 4, 0, 2.
  3. When the currentIndex is 2, close items in the order 4, 0, 1, 3.
  4. When the currentIndex is 3, close items in the order 0, 1, 2, 4.
  5. When the currentIndex is 4, close items in the order 1, 2, 3, 0.

Befor :

Screen.Recording.2023-09-24.at.3.13.52.AM.mov

After:

Screen.Recording.2023-09-24.at.3.14.16.AM.mov

πŸ“ Checklist

  • I have linked an issue or discussion.

Resolves #626

@vercel
Copy link

vercel bot commented Sep 24, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
ui βœ… Ready (Inspect) Visit Preview Sep 24, 2023 9:15am

@benjamincanac benjamincanac changed the title fix(Accordion): close other items oepn in circular order fix(Accordion): close other items in circular order Sep 24, 2023
@benjamincanac benjamincanac merged commit 6887f73 into nuxt:dev Sep 24, 2023
1 check passed
@Haythamasalama Haythamasalama deleted the fix/Accordion-close-others branch September 24, 2023 09:37
@matteason
Copy link

matteason commented Sep 25, 2023

Thanks for picking this up @Haythamasalama ! Unfortunately the behaviour is still wrong - when an accordion section is expanded or collapsed, focus should remain on that section header (there's an example of this in WAI). With this change, it still jumps to the next section header. If someone is using a screen reader, for example, they'll expect focus to remain on the header for the section they've just opened so they can trigger the screen reader to read the content inside it. If focus has jumped to the next header that becomes harder because they'll need to tab back (potentially through focusable content in the accordion section, such as links) before being able to read the content from the top.

@Haythamasalama
Copy link
Contributor Author

Unfortunately, at this time, when clicking on any items that should close other items, we have to handle it manually. There is no API provided by the headless UI to do this, so we are managing it in a tricky way. Additionally, we cannot focus on the current item that is open.

You can enable the multiple props, which will give you the desired behavior, but it will not automatically close the other items.

Feel free to open an issue about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keyboard focus jumps when opening accordion section
4 participants