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

[Accordion] Allow to disable gutter/spacing #24532

Conversation

TimonPllkrn
Copy link
Contributor

resolves #24492

I've added the disableGutters prop to the AccordionSummary and AccordionDetails Components.

@mui-pr-bot
Copy link

mui-pr-bot commented Jan 21, 2021

@material-ui/core: parsed: +0.08% , gzip: +0.07%

Details of bundle changes

Generated by 🚫 dangerJS against 22e5c58

@TimonPllkrn TimonPllkrn changed the title I24492 accordion allow to disable gutter/spacing [Accordion] Allow to disable gutter/spacing Jan 21, 2021
@oliviertassinari oliviertassinari added component: accordion This is the name of the generic UI component, not the React module! new feature New feature or request labels Jan 21, 2021
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that we are on the same page. The objective was to disable the very specific behavior of Material Design that adds a margin between to accordion items and that changes the accordion summary height when expanded:

e.g.

Capture d’écran 2021-01-21 à 20 04 35

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 22, 2021
@TimonPllkrn
Copy link
Contributor Author

Ok, I was confused because disableGutters in the other components means remove padding left and right.
Implemented it like this now:

<Accordion disableGutters>
accordion_disableGutters

<Accordion>
accordion

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 22, 2021

@TimonPllkrn Yes, I think that this is the behavior that makes the most sense :). If you have ideas for improving the name of the prop, I would love to hear them. I also agree that it's confusing.

The padding of the component should be easy enough to customize, I don't think that we need new dedicated props for them.

Mind that the component is being migrated to emotion, I would encourage us to hold on until these pull requests are merged:

Capture d’écran 2021-01-22 à 12 09 33

I think that we can use this prop for this demo: https://next.material-ui.com/components/accordion/#customized-accordion, it simplifies it.

@mnajdova
Copy link
Member

@TimonPllkrn @oliviertassinari all PRs for migrating the Accordion components are merged. This work can continue :)

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 23, 2021
@TimonPllkrn
Copy link
Contributor Author

TimonPllkrn commented Jan 23, 2021

@oliviertassinari I couldn't think of a better name either.

I have now adapted the changes to the migration and added disableGutters to the demo of Customized accordion.

Also added the classes gutters and contentGutters to customize the style.

@oliviertassinari oliviertassinari force-pushed the i24492-accordion-allow-to-disable-gutter/spacing branch 2 times, most recently from 35cee9f to 3002532 Compare January 24, 2021 19:49
@oliviertassinari oliviertassinari force-pushed the i24492-accordion-allow-to-disable-gutter/spacing branch 3 times, most recently from 30d4dc1 to c0fe861 Compare January 24, 2021 20:13
@oliviertassinari oliviertassinari self-assigned this Jan 24, 2021
@oliviertassinari oliviertassinari force-pushed the i24492-accordion-allow-to-disable-gutter/spacing branch from c0fe861 to d275807 Compare January 24, 2021 21:02
@oliviertassinari oliviertassinari force-pushed the i24492-accordion-allow-to-disable-gutter/spacing branch from d275807 to 22e5c58 Compare January 24, 2021 21:13
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have used the opportunity to polish the customization demo

Capture d’écran 2021-01-24 à 22 12 48

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@oliviertassinari oliviertassinari merged commit 6f3d0ac into mui:next Jan 25, 2021
natac13 pushed a commit to natac13/material-ui that referenced this pull request Jan 25, 2021
eps1lon pushed a commit to eps1lon/material-ui that referenced this pull request Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: accordion This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Accordion] Allow to disable gutter/spacing
4 participants