[Accordion] Add Joy UI Accordion components

[siriwatknp]

Preview: https://deploy-preview-38164--material-ui.netlify.app/joy-ui/react-accordion/

closes #36281. We have to roll a custom implementation as Base UI isn't available mui/base-ui#25 yet.

The Accordion family reuses styles from the List family.

AccordionGroup <-> List
Accordion <-> ListItem
AccordionSummary <-> ListItemButton

Joy UI Accordion uses CSS Grid for the accordion detail to let developers use CSS transition to animate the open/closed state.

---

[mui-bot]

Netlify deploy preview

• docs/data/joy/components/accordion/accordion.md

@mui/joy: parsed: +2.08% , gzip: +2.01%

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 69d7cfa

[oliviertassinari]

Great to see movement on the accordion 👍

I wonder about the order we create this vs. the one in Base UI.

More ideas to push it further:

1. For the animation duration, I think that getAutoHeightDuration could make a difference in how great it feels, Material UI is following this duration curve based on the height, to control the transition px/s

2. We will likely need overflowAnchor
3. I think that we will need to consider the performance implication of mounting the whole accordion details list.
4. This makes me think that we could start to support Support for prefers-reduced-motion #16367

adding @DiegoAndai to the loop, as the owner of the Accordion component (we are in draft mode now, so no need to review, I came here from https://twitter.com/siriwatknp/status/1684042706849505280).

[oliviertassinari]

It takes 277ms for the accordion to start moving, I don't think that we can't have this transition curve, per https://web.dev/fid/#what-is-a-good-fid-score. For comparison, with Easing, I measure this at 27ms.

Suggested change

<Radio value="0.8s cubic-bezier(0.68, -0.6, 0.32, 1.6)" label="Bouncy" />

Instead, I think that we could have a small bounce at the end of the animation like https://dribbble.com/shots/20711653-Side-panel-framework-for-Opkey-s-no-code-automation-tool?

[oliviertassinari]

Would it be clearer if this was customized with CSS and the sx prop? Developer would have direct access to their CSS animation design tokens, like duration, easing, etc.

What problem does the transition prop solve better?

[siriwatknp]

I thought about it as well. With transition prop, developers don't have to remember and specify both properties grid-template-rows and padding.

Also, if you want to customize the transition for both initial and expanded state in CSS, you will need to do this:

<AccordionGroup
  sx={{
    // actually you don't need to use CSS variables here but the amount of code to write is likely the same.
    '--AccordionDetails-transition': `grid-template-rows 0.2s, padding-block 0.2s`,
        [`& .${accordionDetailsClasses.root}.${accordionDetailsClasses.expanded}`]: {
          '--AccordionDetails-transition': `grid-template-rows 0.3s, padding-block 0.3s`,
        },
  }}
/>

whereas with transition prop:

<AccordionGroup
  transition={{
    initial: '0.2s',
    expanded: '0.3s',
  }}
/>

[oliviertassinari]

There can be no value selected, expected?

[siriwatknp]

How to reproduce it?

[oliviertassinari]

Screen.Recording.2023-08-05.at.19.16.24.mov

https://deploy-preview-38164--material-ui.netlify.app/joy-ui/react-accordion/#sizes

---

Oh, actually https://mui.com/material-ui/react-toggle-button/#exclusive-selection and https://mui.com/joy-ui/react-toggle-button-group/#editor-toolbar has the same bug? One of the text alignments must be active at all times. It's covered in https://mui.com/material-ui/react-toggle-button/#enforce-value-set. Maybe we need a prop for it.

[siriwatknp]

Hmm, I don't think so. We should let developers handle by themselve.

[oliviertassinari]

This easing makes me feel uncomfortable. What if instead, we were slowing the default animation (for illustration)? And/Or have it to be instant to illustrate prefers-reduced-motion?

[siriwatknp]

I think the default animation/transition is out of scope from this PR that's why the demo illustrates how to add transitions.

@zanivan mentioned the default transition for Joy as well and I think it should be done separately so that each component does not need to wait to roll out.

[zanivan]

I think adding default animations/transitions to Joy UI would be really helpful. It could improve the microinteractions on components and potentially become a standout feature that sets Joy UI apart from competitors. Also, if we incorporate the Joy UI design language into Base UI, it would further differentiate the two products.

All in all, I think we can polish out a bit these animations, the easing is indeed a bit off, mostly on the bouncing demo—at least for me. Are there any workarounds to improve these animations before coming up with the default transition?

[oliviertassinari]

I think the default animation/transition is out of scope from this PR that's why the demo illustrates how to add transitions.

@siriwatknp Ah sorry, I used the wrong terminology. By "default animation", I meant the one applied on the demo. I didn't consider the animation of the component bare.

But actually, this connects well to point 1. in #38164 (review). This feels OK, maybe a 10% faster would be good (to move to great, I think it would take a different easing IMHO):

Screen.Recording.2023-08-05.at.19.23.16.mov

and this feels too fast; 40% slower would be good (to move to great, I think it would take a different easing IMHO):

Screen.Recording.2023-08-05.at.19.25.07.mov

[oliviertassinari]

I think we can also see this bug in this animation: the children isn't stable, "The Accordion Group supports the…" moves during the transition. So to start reading the content, you have to wait for the whole transition to finish

Screen.Recording.2023-08-05.at.19.31.20.mov

To compare with how it feels stable, e.g. https://ui.shadcn.com/docs/components/accordion. I feel stable is better UX.

[siriwatknp]

I wonder about the order we create this vs. the one in Base UI.

For the Accordion to match WAI ARIA guide, using an internal hook from @mui/utils is enough.

We will likely need overflowAnchor

Got it.

I think that we will need to consider the performance implication of mounting the whole accordion details list.

Why? I think there will be a rare case to think in terms of performance which I think we can leave to the developer to decide.

[oliviertassinari]

Why? I think there will be a rare case to think in terms of performance which I think we can leave to the developer to decide.

@siriwatknp I think it's the same problem as in #21250. I think that the default of mounting everything is best, agree. The point I lead to is: How can developers unmount what's not visible with Joy UI without breaking animations?

Radix UI has the forceMount prop, by default it unmounts, and when applied to mounts: https://www.radix-ui.com/docs/primitives/components/accordion & https://codesandbox.io/p/sandbox/wizardly-zeh-dp5ng6?file=%2FApp.jsx%3A13%2C1.
Material UI does the opposite and allows developers to control mounting behavior: https://mui.com/material-ui/react-accordion/#performance.

We will likely need overflowAnchor

It fixed this strange bug: #22152.

For the Accordion to match WAI ARIA guide, using an internal hook from @mui/utils is enough.

I was thinking of it in terms of priority. The alternative route would be to open this PR on Base UI first, and then Joy UI.

[siriwatknp]

@zanivan Updates:

• The divider appears by default which can be hidden by <AccordionGroup disableDivider> prop
• The default transition is 0.2s ease.
• Adjusted padding-bottom to increase spacing between AccordionSummary and Details between accordions.
• I am hesitating to remove the solid variant at this point. I feel like it's the presenting issue more than the implementation so I decided to remove the Usage demo for now.
• I did not touch size-related properties. I would not add more CSS to it at this point because you can simply customize the font-size with just one line.
• Updated the last common example. The AccordionDetails should have only one for each accordion.

[zanivan]

• I don't know if we should remove the Usage demo, since we have it in all interactive components, it would be inconsistent.
  So this is a matter of choosing what tradeoff we will have: removing the Usage demo that'll result in an inconsistency with other pages; or, keeping the Usage demo and the solid variant.
  Personally, I lean towards keeping the Usage demo, since we have more components with problematic variants like this—and this issue can be addressed in a separate PR, where we select which variants each component will have.

• The https://deploy-preview-38164--material-ui.netlify.app/joy-ui/react-accordion/#styling-on-expansion doesn't seem to be rendering the border on the third item
  

[oliviertassinari]

A quick review:

• We could add the playground back, at least, why not.
• In the size demo, I was a bit surprised to see the typography change as well, I don't know, I feel should we want to control spacing and typography size independently.
• Rebasing on HEAD could be nice, to pull in the latest changes
• https://github.com/mui/material-ui/issues/38037#issuecomment-1678190180
• I linked this PR to the Base UI issue, so that the community, if they decide to take on Base UI accordion can more easily find the work we did here.
• I wish we had more demo previews. Maybe 3 accordion isn't compatible with this (too much vertical space), but it would work with 2.
• We could add ComponentLinkHeader
• https://github.com/mui/material-ui/issues/38037#issuecomment-1678200974

[oliviertassinari]

This React element could be hoisted.

[DiegoAndai]

The disabled and expanded classes are Accordion classes, why add them here as well?

[siriwatknp]

For styling purposes, without these classes, you will have to theme the AccordionSummary like this:

extendTheme({
  components: {
    JoyAccordion: {
      styleOverrides: {
        root: {
          [`.${accordionClasses.root}.${accordionClasses.expanded} &`]: {
             …styles
          }
        }
      }
    }
  }
})

whereas

extendTheme({
  components: {
    JoyAccordion: {
      styleOverrides: {
        root: {
          [`&.${accordionSummaryClasses.expanded}`]: {
             …styles
          }
        }
      }
    }
  }
})

[DiegoAndai]

I agree it's easier, but if we do it for the AccordionSummary component, shouldn't we do it for the other accordion components as well?

• AccordionDetails has expanded but not disabled
• AccordionGroup has neither

We should either add the classes to all or none. Otherwise, it might be confusing.

I would only have these on the root, I think the following is not a bad API:

extendTheme({
  components: {
    JoyAccordion: {
      styleOverrides: {
        root: {
          [`.${accordionClasses.expanded} &.${accordionSummaryClasses.root}`]: {
             …styles
          }
        }
      }
    }
  }
})

[siriwatknp]

• I don't know if we should remove the Usage demo, since we have it in all interactive components, it would be inconsistent.
  So this is a matter of choosing what tradeoff we will have: removing the Usage demo that'll result in an inconsistency with other pages; or, keeping the Usage demo and the solid variant.
  Personally, I lean towards keeping the Usage demo, since we have more components with problematic variants like this—and this issue can be addressed in a separate PR, where we select which variants each component will have.
• The https://deploy-preview-38164--material-ui.netlify.app/joy-ui/react-accordion/#styling-on-expansion doesn't seem to be rendering the border on the third item
  

I brought back the Usage demo and apply the variant to other components when it is solid. Also, fix the demo you mentioned.

[zanivan]

Added some small tweaks to a demo, but it looks good to me! 🚀