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

[CheckboxGroup] Implement components and hooks #458

Merged
merged 19 commits into from
Sep 4, 2024

Conversation

atomiks
Copy link
Contributor

@atomiks atomiks commented Jun 17, 2024

Closes #213

@atomiks atomiks marked this pull request as draft June 17, 2024 06:39
@atomiks atomiks added the component: checkbox This is the name of the generic UI component, not the React module! label Jun 17, 2024
@mui-bot
Copy link

mui-bot commented Jun 17, 2024

Netlify deploy preview

https://deploy-preview-458--base-ui.netlify.app/

Generated by 🚫 dangerJS against bc302e7

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 17, 2024
@atomiks atomiks force-pushed the feat/CheckboxGroup branch from 14e28ea to 64d67a9 Compare June 19, 2024 06:47
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 19, 2024
@atomiks atomiks marked this pull request as ready for review June 19, 2024 06:54
@atomiks atomiks force-pushed the feat/CheckboxGroup branch from af887e3 to de6e4f0 Compare June 26, 2024 05:00
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 27, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 21, 2024
Copy link
Member

Choose a reason for hiding this comment

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

When I select some items (such as Red and Green) and click on All Colors repeatedly, the selection goes from all to none to Red and Green again. This is quite unexpected (I can't recall seeing such behavior elsewhere). Usually, the parent checkbox toggles between all and none and the partially selected state is lost.
I would make it configurable if you want to keep this behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what the ARIA demo does, but yeah I can make it configurable

Copy link
Contributor

Choose a reason for hiding this comment

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

This surprised me too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added preserveChildStates prop

@atomiks atomiks merged commit 9c04f2c into mui:master Sep 4, 2024
17 of 18 checks passed
@atomiks atomiks deleted the feat/CheckboxGroup branch September 4, 2024 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: checkbox 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.

[CheckboxGroup] Implement CheckboxGroup
4 participants