Accordion Convergence

[jurokapsiar]

Preparation:

• Started prep
• Open UI Research complete: feat(Accordion): adds accordion research  openui/open-ui#263
• Comparison on v8 and v0 complete
• Gather open GitHub issues related to component
• react-* package scaffolded
  • link to package
• Component Spec authored and reviewed
  • link to spec in component package
• Deliverable: Reviewed component spec

Implementation

link to react-* package folder

• Started impl
• Implement component
• Add storybook stories
• Using hooks
• Using makeStyles
• Respects Figma tokens (and using provider)
• Respects API principles, shorthands and slots handling
• No dependency on v0/v7
• Add tests - Conformance, Unit, and VR
  • Conformance tests
  • Unit tests
  • VR tests
  • Accessibility behavior tests
• Write README.md covering basic usage
• Write initial MIGRATION.md guide (include v8 and v0)
• Deliverable: Experimental component ready for partner use

Validation

• Started validating
• Add and validate in UI Builder
• Add and validate in docs site
• Validate with token pipeline
• Validate in product
• Finalize migration guide
  • Author codemods
• Deliverable: Preview component ready for broader/3rd party use

[JustSlone]

Update:

• Spec in review

Next:

• Implementing base component

Need:

• Working with Design on visual design - ETA this week

[JustSlone]

Update:

• Contextual behavior is a challenge, need to figure out approach here. Spending more time on Debate here
• Technically in implementation phase

[brandonthomas]

Update:

• Contextual behavior is a challenge, need to figure out approach here. Spending more time on Debate here
• Technically in implementation phase

Can we just leverage existing patterns similar to Customizer to address in context needs without breaking encapsulation? I get really concerned when components try to reach out of themselves and make assumptions. Usually leads to unexpected behavior. Would love to understand the approach here.

[bsunderhus]

The approach I'm adopting at the moment (after some discussion) is to use @reach/descendants, an internal library from Reach UI design system that was developed to manage this exact problem of comunicatin between compound components.

Please let me know if you have any thoughts or advices on the matter @brandonthomas

[brandonthomas]

so is this being used to solve the problem of what is inside the Accordion or for the Accordion to understand what it is itself inside of?

[bsunderhus]

This solves the problem of knowing the order of child components inside a context. Which is a common problem for components such as Menu, Accordion, List, pretty much any components that normally follow some sort of compount pattern @brandonthomas

[JustSlone]

Update:

• PR in progress, wrapping up
• out of scope on PR - accessibility, icon property

[smhigley]

This can probably be part of the accessibility review, just adding a note here that I think it'd be worth using semantic heading and button elements rather than <div> + role (and I can add more detail for why that's helpful whenever it's needed).

The other thing I'd like to discuss at some point is the arrow key + home/end keyboard interaction. The drawback to implementing arrow key interactions in general is that they remove the default functionality of scrolling the page. On components like a combobox or a grid the benefits outweigh the drawbacks, but I don't think that's true for Accordion, since it's usually used primarily as a container for content.

I think it'd be good to have arrow keys + home/end turned off by default, then perhaps have a property to turn them on with some docs language around when it'd be a good idea to do so.

I know we'll have an a11y review later, just wanted to dump thoughts in here for future reference 😄

[bsunderhus]

Perfect! thx @smhigley! I totally agree with the semantic heading and the button element as default value, and let the user opt-out if wanted.

But since there're more opinions on the table, let's wait for the review!

[brandonthomas]

@smhigley I think now is definitely the right time to go over these things. Way cheaper to call out now. To your point, I don't think there's so many accordions in a page where tabbing becomes unmanageable so I think that default makes sense.

@bsunderhus so this is basically to replace needing to pass the count of items through to children via props then? Is that necessary when we have slots and can pass in native props through the root of the component? Either way the children have to either opt into the context here or support props to the root through the component props. Am I understanding this right?

[bsunderhus]

Uh no, I think I might have missguided you there @brandonthomas . The documentation on reach-descendants is quite a good description actually.

Let's take the case of Accordion for example. The state of an AccordionItem being opened is controlled by an Accordion, and can behave in 3 distincts behaviors: multiple (where 1 or more AccordionItem can be opened), collapsible (where 1 or 0 AccordionItem can be opened) and accordion (where only 1 AccordionItem can be opened). To ensure those behaviors the Accordion component has to mantain a count of the number of internal AccordionItem . The descendants API does exactly that, it informs a parent element about properties and index about it's children elements, so that the parent can take behavioral decisions.

The same line of thinking from the Accordion behavior can be applied to any compound component, such as Menu, List, etc.

[bsunderhus]

Here's the sum up of the A11y review

1. Avoid using button inside heading
   • This is actually the way that is described in ARIA documentation, but apparently VoiceOver has some problem with that
2. Use semantic elements by default
   • Some environments behave better when semantic elements are used over ARIA, (VoiceControl, HighContrast Mode)
3. Make arrow navigation optional and disabled by default.
4. Make cyclic navigation optional and disabled by default.

[JustSlone]

Update:

• No progress this week (work on other item)

Next up:

• Solve Accessibility issues and other small additions

[scottaohara]

Hi there.

I had come to Fluent's issues to file a request for an accordion component, as I've seen many teams misusing the tree grid component as a stand-in for more appropriate disclosure widgets / accordion components. So I'm very happy to see this in the works and (hopefully?) coming soon!

I do have one comment, having read through the thread here, concerning the following:

Avoid using button inside heading
This is actually the way that is described in ARIA documentation, but apparently VoiceOver has some problem with that

First, I would submit that not all accordion components require headings - so optionally being able to specify headings would be the ideal here. But per the comment's reasoning not to implement, Voice Over doesn't have a bug with how buttons within headings work, as much as VO has a very particular behavior which is intentional on their end, for how elements are navigated. To that end, I would request that this not be treated as a reason to not implement support for headings.

Thank you!

[bsunderhus]

Thx for the feedback @scottaohara! The implementation will follow WAI-ARIA spec as recommended and use a header with a button by default, but opting-out mechanisms will be provided to ensure that this behavior can be addressed

[scottaohara]

Thank you for the response Look forward to the component's release.

[msft-fluent-ui-bot]

Because this issue has not had activity for over 180 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.

[msft-fluent-ui-bot]

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.