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

[docs] Add new customized switch examples #26096

Merged

Conversation

DanielBretzigheimer
Copy link
Contributor

@DanielBretzigheimer DanielBretzigheimer commented May 2, 2021

Added Android 12 switch and custom icon in thumb example to the docs
Issue: #25919
Fix #25985

Preview: https://deploy-preview-26096--material-ui.netlify.app/components/switches/#customized-switches

Screenshot 2021-05-05 at 20 51 36

@mui-pr-bot
Copy link

mui-pr-bot commented May 2, 2021

No bundle size changes

Generated by 🚫 dangerJS against 32ff124

@mnajdova
Copy link
Member

mnajdova commented May 3, 2021

Looks great. A couple of suggestions on the iOS Switch.

  1. we should remove the border for the unchecked version
  2. we should make the background darker grey

@DanielBretzigheimer
Copy link
Contributor Author

I will update the iOS Example as well to more closely resemble the "real" iOS switch control.

Another question: I don't know how to fix the remaining failing test. What has to be done to fix it?

@mnajdova
Copy link
Member

mnajdova commented May 3, 2021

I will update the iOS Example as well to more closely resemble the "real" iOS switch control.

Great!

Another question: I don't know how to fix the remaining failing test. What has to be done to fix it?

We will just approve the new screenshot in the end, no need for fixing anything :)

@oliviertassinari
Copy link
Member

For iOS, see #25985 (comment)

@DanielBretzigheimer
Copy link
Contributor Author

I have updated the iOS Switch including proper disabled state. What do you think?

Images:
image
image

For reference screenshots from iOS:
image

@mnajdova
Copy link
Member

mnajdova commented May 5, 2021

I have updated the iOS Switch including proper disabled state. What do you think?

Looks awesome!

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Thanks @DanielBretzigheimer! It's a great first PR on Material-UI 👌

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label May 5, 2021
@oliviertassinari oliviertassinari added the component: switch This is the name of the generic UI component, not the React module! label May 5, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented May 5, 2021

I have pushed the effort one step further, mainly by adding the Material-UI Switch (MUI has its own branding which is not to be confused with Material Design).

Screenshot 2021-05-05 at 20 50 19

Screenshot 2021-05-05 at 20 50 14

The side improvements are to order the source in the same way as the demos, and to move the margin to userland.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 5, 2021

I have quite struggled to implement this. It took me more time than I had anticipated. @siriwatknp did his share too in https://mui-treasury.com/styles/switch/. @DanielBretzigheimer I wonder how we could simplify the customization of the component. A few thoughts:

  • the input catches the dev tools inspector, it's such a pain. I doubt we can fully solve this. One this that would help is to flatten the structure, have the input as a direct child of the root, so we can quickly understand where we are.
  • the DOM nodes are verbose, e.g. MuiButtonBase-root MuiIconButton-root MuiIconButton-sizeMedium MuiSwitch-switchBase MuiSwitch-colorSecondary PrivateSwitchBase-root Mui-checked css-1eusvr7-MuiButtonBase-root-MuiIconButton-root-MuiSwitch-switchBase). It makes it hard to understand DOM structure. I think that we should at the very least drop the IconButton Can't override IconButton styles in theme without affecting Checkbox and Switch #21503.
  • the style of MuiSwitch-thumb has to be moved into two different places simply to apply to leverage the check pseudo-class. Maybe it would be simpler if the thumb had its own pseudo-class. Not sure, because it's mostly disturbing by not being on the root. So I guess that if we move the pseudo-class to the root, we would be good. [Switch] ref attribute is not the root element #19613

@DanielBretzigheimer
Copy link
Contributor Author

I fully agree with you on the input part. Clearing up the DOM Structure seems like a good Idea, using a IconButton was confusing for me.

I will have a look into it and will try to find a better solution. Should I append the changes to this PR or create another one? Also, this will certainly be a breaking change, but this seems to be okay?

@oliviertassinari
Copy link
Member

oliviertassinari commented May 5, 2021

@DanielBretzigheimer If you can explore this in another pull-request, it would be awesome. I have linked the two relevant issues in my previous comment. As for this PR, no, it's already larger than it needs to be, no more :).

@oliviertassinari oliviertassinari changed the title [docs] Added new customized switch examples [docs] Add new customized switch examples May 6, 2021
@oliviertassinari oliviertassinari merged commit 215794b into mui:next May 6, 2021
@DanielBretzigheimer DanielBretzigheimer deleted the feature/switch_customization branch May 9, 2021 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: switch This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] Improve iOS style switch demo
4 participants