Skip to content

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

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

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

Conversation

@vicasas

@vicasas vicasas commented Apr 27, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

  • [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

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

Copy link
Copy Markdown

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

vicasas commented Apr 27, 2021

Copy link
Copy Markdown
Member Author

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

@oliviertassinari oliviertassinari left a comment

Copy link
Copy Markdown
Member

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

oliviertassinari commented Apr 27, 2021 •
edited
Loading

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

@vicasas

vicasas commented Apr 27, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

Ups! I will review it during the day.

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

It looks good now

@oliviertassinari

oliviertassinari commented Apr 27, 2021

Copy link
Copy Markdown
Member

@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

@mnajdova mnajdova left a comment

Copy link
Copy Markdown
Member

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