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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 11 commits into from
Apr 30, 2021
Merged

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

merged 11 commits into from
Apr 30, 2021

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 component: checkbox This is the name of the generic UI component, not the React module! 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

@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 component: checkbox This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vicasas @mui-pr-bot @oliviertassinari @mnajdova