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

[Checkbox] Default Indicator to keepMounted=false #936

Merged
merged 13 commits into from
Dec 10, 2024

Conversation

atomiks
Copy link
Contributor

@atomiks atomiks commented Dec 2, 2024

Closes #896

@atomiks atomiks added the component: checkbox This is the name of the generic UI component, not the React module! label Dec 2, 2024
@mui-bot
Copy link

mui-bot commented Dec 2, 2024

Netlify deploy preview

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

Generated by 🚫 dangerJS against 61d1727

@atomiks atomiks marked this pull request as ready for review December 3, 2024 21:17
Comment on lines 77 to 80
extraProps: {
hidden,
...otherProps,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One consequence of hidden here is that you can't rely on the outer Root checkbox animation, because it looks for animations on the indicator itself: animatedElementRef: indicatorRef. So you need to duplicate styles to keep it mounted; hidden={false} of course can be used

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 4, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 5, 2024
@colmtuite colmtuite requested a review from vladmoroz December 5, 2024 01:34
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 6, 2024
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Dec 9, 2024
@colmtuite colmtuite added this to the 1.0.0-alpha.1 release milestone Dec 9, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 9, 2024
export const transitionStatusMapping = {
transitionStatus(value): Record<string, string> | null {
if (value === 'starting') {
return { 'data-starting-style': '' };
Copy link
Member

Choose a reason for hiding this comment

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

Nit: you could create the const object beforehand and reuse it in each call.

@atomiks atomiks merged commit 2fc05f1 into mui:master Dec 10, 2024
23 checks passed
@atomiks atomiks deleted the refactor/checkbox-indicator-keep-mounted branch December 10, 2024 04:33
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
Status: Done
Development

Successfully merging this pull request may close these issues.

[Checkbox] Unmount indicator
4 participants