Skip to content

[Checkbox] Make color="primary" default#26002

Merged
mnajdova merged 11 commits intomui:nextfrom
vicasas:feature/change-color-checkbox
Apr 30, 2021
Merged

[Checkbox] Make color="primary" default#26002
mnajdova merged 11 commits intomui:nextfrom
vicasas:feature/change-color-checkbox

Conversation

@vicasas
Copy link
Member

@vicasas vicasas commented Apr 27, 2021 •
edited
Loading

  • [Checkbox] Make color="primary" the default over secondary. This better matches the material design guidelines.

    -<Checkbox />
+<Checkbox color="secondary />

Preview migrate guide: https://deploy-preview-26002--material-ui.netlify.app/guides/migration-v4/#checkbox

One of the material design section #20012

@mui-pr-bot
Copy link

mui-pr-bot commented Apr 27, 2021 •
edited
Loading

Details of bundle changes

Generated by 🚫 dangerJS against 9b4c4c8

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Apr 27, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Apr 27, 2021
@github-actions github-actions bot mentioned this pull request Apr 27, 2021
Closed
@vicasas
Copy link
Member Author

vicasas commented Apr 27, 2021

The difference of argos is given to the change of color of the component.

@oliviertassinari oliviertassinari added breaking change Introduces changes that are not backward compatible. scope: checkbox Changes related to the checkbox. labels Apr 27, 2021
oliviertassinari
oliviertassinari previously requested changes Apr 27, 2021
View reviewed changes
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.

It breaks the two demos in https://next.material-ui.com/customization/theming/#nesting-the-theme.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 27, 2021 •
edited
Loading

  • Could you update the migration guide to v5?
  • Did you check if there are cases where we can remove the new default prop?

@vicasas
Copy link
Member Author

vicasas commented Apr 27, 2021 •
edited
Loading

Ups! I will review it during the day.

@oliviertassinari oliviertassinari dismissed their stale review April 27, 2021 18:52

It looks good now

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 27, 2021

@bramick Is this the type of change that you did in your design system (the swap of secondary/primary you talked about during our last user interview)?

mnajdova
mnajdova approved these changes Apr 28, 2021
View reviewed changes
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.

👍 thanks!

@m4theushw m4theushw mentioned this pull request Apr 28, 2021
Closed
@mnajdova mnajdova merged commit 6a3b294 into mui:next Apr 30, 2021
@oliviertassinari oliviertassinari added the design: material This is about Material Design, please involve a visual or UX designer in the process label Apr 30, 2021
@vicasas vicasas deleted the feature/change-color-checkbox branch May 7, 2021 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mnajdova mnajdova mnajdova approved these changes

@oliviertassinari oliviertassinari oliviertassinari left review comments

Assignees

No one assigned

Labels

breaking change Introduces changes that are not backward compatible. design: material This is about Material Design, please involve a visual or UX designer in the process scope: checkbox Changes related to the checkbox.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vicasas @mui-pr-bot @oliviertassinari @mnajdova