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] Add new classes key #21920

Merged
merged 7 commits into from
Jul 25, 2020

Conversation

natac13
Copy link
Contributor

@natac13 natac13 commented Jul 24, 2020

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 changed the title [Accordion] add new classes key [Accordion] Add new classes key Jul 24, 2020
@oliviertassinari oliviertassinari added the component: accordion This is the name of the generic UI component, not the React module! 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

@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

Details of bundle changes

Generated by 🚫 dangerJS against 0ebbadf

@natac13

This comment has been minimized.

@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

@natac13 Well done

@eps1lon eps1lon added this to the v5 milestone Aug 4, 2020
@eps1lon eps1lon mentioned this pull request Aug 5, 2020
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
Copy link
Member

@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

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
Labels
breaking change component: accordion This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Accordion] Add new classes key
5 participants