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

[Joy] Add Checkbox component #31273

Merged
merged 15 commits into from
Mar 7, 2022
Merged

[Joy] Add Checkbox component #31273

merged 15 commits into from
Mar 7, 2022

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Mar 2, 2022 •
edited

🖥 Preview

🎠 Variables Playground

A simple checkbox that leverages useSwitch from MUI Base and has APIs similar to @mui/material.

@siriwatknp siriwatknp added the package: joy-ui Specific to @mui/joy label Mar 2, 2022
@mui-bot
Copy link

mui-bot commented Mar 2, 2022 •
edited

Details of bundle changes

@mui/joy: parsed: +3.40% , gzip: +2.63%

Generated by 🚫 dangerJS against 1da9f6d

@siriwatknp siriwatknp requested a review from hbjORbj March 2, 2022 14:05
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 3, 2022
hbjORbj
hbjORbj reviewed Mar 3, 2022
View reviewed changes
Copy link
Member

@hbjORbj hbjORbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Not important, but some components in the demo are slightly misaligned.

packages/mui-joy/src/Checkbox/checkboxClasses.ts Show resolved Hide resolved
packages/mui-joy/src/Checkbox/Checkbox.tsx Show resolved Hide resolved
packages/mui-joy/src/Checkbox/Checkbox.test.js Show resolved Hide resolved
packages/mui-joy/src/Checkbox/Checkbox.tsx Outdated Show resolved Hide resolved
@hbjORbj hbjORbj added component: checkbox This is the name of the generic UI component, not the React module! and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Mar 3, 2022
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 3, 2022
@hbjORbj
Copy link
Member

hbjORbj commented Mar 3, 2022 •
edited

Hey, Jun, I pushed a commit where I added iconSize (consisting of values from xs to xl) to theme.vars and then used it in your Checkbox component.

I pushed it to minimise our merge conflicts in the future because I need this theme variable for Avatar component, which I am currently working on, as well. I hope you don't mind!

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 4, 2022
@hbjORbj hbjORbj mentioned this pull request Mar 4, 2022
Merged
1 task
@siriwatknp siriwatknp requested a review from hbjORbj March 7, 2022 03:42
hbjORbj
hbjORbj approved these changes Mar 7, 2022
View reviewed changes
Copy link
Member

@hbjORbj hbjORbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect!

@siriwatknp siriwatknp merged commit 0a8f91f into mui:master Mar 7, 2022
@oliviertassinari oliviertassinari mentioned this pull request Sep 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hbjORbj hbjORbj hbjORbj approved these changes

Assignees
No one assigned
Labels
component: checkbox This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@siriwatknp @mui-bot @hbjORbj