[EuiCollapsibleNavItem] Add a non-accordion group component

[cee-chen]

Summary

Screenshots:

Kibana is currently doing mildly hacky shenanigans to accomplish the above (using the accordion component and forcibly hiding the arrow via CSS).

Instantiating an accordion when one isn't needed is a lot of extra unnecessary logic, and is also potentially an accessibility issue. So I've added an isCollapsible API next to the items array that can be used to conditionally not render an accordion but instead what's essentially a plain div. Visually, it should look the same as an open accordion with no arrow, but semantically it's different to screen readers, and a lot more lightweight.

I strongly recommend following along by commit and hiding white space changes. The last commit touches a lot of files but is technically optional - I wanted to see what would happened if I tried adding the group, list, and listitem roles to try adding more meaningful information/relationships between groups and group headings. (reverted)

QA

• Go to https://eui.elastic.co/pr_7315/storybook/?path=/story/euicollapsiblenavitem--edge-case-testing

General checklist

• Browser QA
  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Checked for accessibility including keyboard-only and screenreader modes
• Docs site QA - N/A, beta component
• Code quality checklist
  • Added or updated jest and cypress tests
• Release checklist - N/A, beta component
• Designer checklist - N/A, beta component

[cee-chen]

@1Copenut I'm so sorry to keep pinging you 🥲 But I'd really love your expert a11y opinion on the last commit in this PR! I'm not totally sure if adding the group/list/listitem roles is helpful to screen reader users, or if we should just leave them as free-floating content since there's already a <nav> group. I might be visually conflating the indented hierarchies/groups with the idea of <ul> type elements in my head - if so, I'm happy to backtrack on this.

[cee-chen]

Also FYI @sebelga and @tsullivan that this work is coming to Kibana in the next week or so

[1Copenut]

@cee-chen I ran this through VoiceOver + Safari after dinner on Oct 25 (Wednesday) and here's what I found:

• The list items are announcing themselves as list items, and their index in the list, but there's a region sitting between the div[role="list"] and elem[role="listitem"] in several cases that was causing a small level of confusion. I'd have gone ahead and tested with NVDA and JAWS, but for point two.
• This structure with elements between the list and listItem roles was throwing critical errors in axe-core. That'll give us grief in automated tests as well as the reduced experience for assistive technology.

I agree with the reasoning you've applied here; thank you for taking a run at it. I'd like to keep this general structure, but it'll have to be refactored to simpler markup to pass validation and honor the roles you've assigned.

Feel free to holler at me in the morning if you want to discuss ways this could be refactored or if it's work there's time and energy to take on.

[cee-chen]

but there's a region [...] causing a small level of confusion

This is #7316 FWIW, and would be fixed by removing that role from our EuiAccordion component

This structure with elements between the list and listItem roles was throwing critical errors in axe-core

This is weird because I tried moving the role="list" parent a little lower in the nav and axe is still complaining about there being [role=group] elements (link to rule) despite the fact that MDN lists group as a valid companion to list 🤷

It sounds like this may not be a clear win, so I'm going to take a revert the commit in this PR for now (so Kibana still gets the new API earlier) and open up a separate draft PR just for screen reader list behavior.

[cee-chen]

Alright I spiked out removing all role="group" and role="region" children in my local and unfortunately there's still just no way axe is apparently going to pass this, because it yells about almost literally any aria attribute and we have a lot of those on our accordion component that simply cannot be removed

I'm honestly baffled - I don't feel like there's anything that specifically says lists can't have interactive elements in them, and screen readers seem to do just fine (correctly read out the number of list items, etc) with them. It's frustrating when an automated tool gets in the way of an arguably real world screen reader improvement, but c'est la vie.

[kibanamachine]

Preview staging links for this PR:

• Docs site: https://eui.elastic.co/pr_7315/
• Storybook: https://eui.elastic.co/pr_7315/storybook

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 0a3fd0f

History

• 💚 Build #899 succeeded 95654e7

[1Copenut]

👍 LGTM! This feels like the best approach given the difficulty refactoring for lists and list items just now. I tested with VO, NVDA, and JAWS in their preferred browsers. All elements were announced in order and hierarchy maintained.

I think there's a future opportunity for lists and list items, but it'll be a bigger refactor as we've seen and probably a big effort.