Skip to content

[Switch] simplify structure WIP#26303

Closed
DanielBretzigheimer wants to merge 1 commit intomui:nextfrom
DanielBretzigheimer:switch-improvements
Closed

[Switch] simplify structure WIP#26303
DanielBretzigheimer wants to merge 1 commit intomui:nextfrom
DanielBretzigheimer:switch-improvements

Conversation

@DanielBretzigheimer
Copy link
Contributor

@DanielBretzigheimer DanielBretzigheimer commented May 15, 2021

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 •
edited
Loading

Details of bundle changes (experimental)

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

Generated by 🚫 dangerJS against 853d63c

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

@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

oliviertassinari commented May 23, 2021

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

@DanielBretzigheimer
Copy link
Contributor Author

DanielBretzigheimer commented May 23, 2021

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
@oliviertassinari oliviertassinari added the breaking change Introduces changes that are not backward compatible. label May 24, 2021
@DanielBretzigheimer
Copy link
Contributor Author

DanielBretzigheimer commented May 24, 2021

@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.

@oliviertassinari oliviertassinari mentioned this pull request May 26, 2021
Merged
1 task
@siriwatknp
Copy link
Member

siriwatknp commented May 26, 2021

@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
@michaldudak michaldudak mentioned this pull request Jun 15, 2021
Merged
4 tasks
@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

Reviewers

@oliviertassinari oliviertassinari oliviertassinari left review comments

@siriwatknp siriwatknp Awaiting requested review from siriwatknp

Assignees

No one assigned

Labels

breaking change Introduces changes that are not backward compatible. PR: out-of-date The pull request has merge conflicts and can't be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DanielBretzigheimer @mui-pr-bot @oliviertassinari @siriwatknp