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

[Switch] simplify structure WIP #26303

Closed

Conversation

DanielBretzigheimer
Copy link
Contributor

I created this PR to address the complications from this comment. The PR isn't nearly finished, but I wanted to receive early feedback before changing too much in the code.

One thing I'm not sure about is if the SwitchBase should be removed. It clearly cleans the structure but will duplicate some code.

@oliviertassinari What do you think?

Comparison of the DOM structures:
Old
image

New
image

@mui-pr-bot
Copy link

mui-pr-bot commented May 15, 2021

Details of bundle changes (experimental)

@material-ui/core: parsed: +0.24% , gzip: +0.28%

Generated by 🚫 dangerJS against 853d63c

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 seems to go in the right direction. What should we do with the useFormControl?

packages/material-ui/src/Switch/Switch.js Outdated Show resolved Hide resolved
packages/material-ui/src/Switch/Switch.js Outdated Show resolved Hide resolved
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 20, 2021
@oliviertassinari
Copy link
Member

@DanielBretzigheimer What do you think the next step should be?

@DanielBretzigheimer
Copy link
Contributor Author

Sorry, had no time this week, but I will look into it tomorrow

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 24, 2021
@DanielBretzigheimer
Copy link
Contributor Author

@oliviertassinari I have added the ripple effect and some other UI improvements today. I also applied your suggestions.

The useFormControl is still on my agenda. I will look into it in the following days when I have time.

@siriwatknp
Copy link
Member

@DanielBretzigheimer can you take a look at #26460. The PR simplify the structure of SwitchBase by replacing IconButton with ButtonBase (remove 1 level of DOM). If it is enough, we can hold this PR and improve later.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 1, 2021
@DanielBretzigheimer DanielBretzigheimer deleted the switch-improvements branch June 20, 2021 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants