Skip to content

[Accordion] Add new classes key#21920

Merged
oliviertassinari merged 7 commits intomui:nextfrom
natac13:accordion-add-new-classes-key
Jul 25, 2020
Merged

[Accordion] Add new classes key#21920
oliviertassinari merged 7 commits intomui:nextfrom
natac13:accordion-add-new-classes-key

Conversation

@natac13
Copy link
Contributor

@natac13 natac13 commented Jul 24, 2020 •
edited by oliviertassinari
Loading

Breaking change

  • The classes.container key was changed to match the convention of the other components.

    -<Collapse classes={{ container: 'collapse' }}>
+<Collapse classes={{ root: 'collapse' }}>

closes #21723

oliviertassinari
oliviertassinari reviewed Jul 24, 2020
View reviewed changes
@oliviertassinari oliviertassinari changed the title [Accordion] add new classes key [Accordion] Add new classes key Jul 24, 2020
@oliviertassinari oliviertassinari added the scope: accordion Changes related to the accordion. label Jul 24, 2020
@natac13
Copy link
Contributor Author

natac13 commented Jul 24, 2020

@oliviertassinari Should I be changing both the .js file and .tsx file with the changes?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 24, 2020 •
edited
Loading

@natac13 Run

yarn docs:api

Capture d’écran 2020-07-25 à 01 18 22

@mui-pr-bot
Copy link

mui-pr-bot commented Jul 24, 2020 •
edited
Loading

Details of bundle changes

Generated by 🚫 dangerJS against 0ebbadf

@natac13

This comment has been minimized.

Sign in to view
@oliviertassinari oliviertassinari added the breaking change Introduces changes that are not backward compatible. label Jul 25, 2020
@oliviertassinari oliviertassinari force-pushed the accordion-add-new-classes-key branch from 5ec2167 to 0ebbadf Compare July 25, 2020 10:31
@oliviertassinari oliviertassinari merged commit f082232 into mui:next Jul 25, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 25, 2020

@natac13 Well done

@eps1lon eps1lon added this to the v5 milestone Aug 4, 2020
@eps1lon eps1lon mentioned this pull request Aug 5, 2020
Closed
42 tasks
@eps1lon
Copy link
Member

eps1lon commented Aug 6, 2020

Could somebody elaborate why Collapse was also changed and why the region class was added? This is not apparent from the conversation in this PR or the linked issue.

@oliviertassinari oliviertassinari mentioned this pull request Dec 20, 2020
Merged
@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 20, 2020

@eps1lon The region class was added so all DOM nodes can be targeted in the document. Collapse was changed to fix one inconsistency in the naming.

@eps1lon
Copy link
Member

eps1lon commented Dec 21, 2020

@eps1lon The region class was added so all DOM nodes can be targeted in the document. Collapse was changed to fix one inconsistency in the naming.

Why was this part of this PR?

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 21, 2020 •
edited
Loading

Why was this part of this PR?

It was part of this PR to improve the customization story, the topic of the linked issue. It's a small inconsistency we didn't catch sooner (all elements should be customizable), following the boy scouts rule: leaving the forest cleaner than we found it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oliviertassinari oliviertassinari oliviertassinari approved these changes

+1 more reviewer

@joshwooding joshwooding joshwooding approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

breaking change Introduces changes that are not backward compatible. scope: accordion Changes related to the accordion.

Projects

None yet

Milestone

v5.0.0-beta.0

Development

Successfully merging this pull request may close these issues.

[Accordion] Add new classes key

5 participants

@natac13 @oliviertassinari @mui-pr-bot @eps1lon @joshwooding