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] Support small size #16620

Merged
merged 2 commits into from
Jul 20, 2019
Merged

Conversation

darkowic
Copy link
Contributor

@darkowic darkowic commented Jul 17, 2019

I have added support for switch small size (Closes #16152).

  • Add docs and demo
  • fix tests

This is how it looks like:
switch

I need help with tests.

  • How to run a single test with watch mode? yarn test:watch Switch is not working...
  • Finding classes is not working as expected - see test should render the right class

What else should I do to merge this feature?

@mui-pr-bot
Copy link

mui-pr-bot commented Jul 17, 2019

Details of bundle changes.

Comparing: b6182ce...94422d1

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.07% 🔺 +0.08% 🔺 327,251 327,488 90,345 90,419
@material-ui/core/Paper 0.00% 0.00% 68,477 68,477 20,410 20,410
@material-ui/core/Paper.esm 0.00% 0.00% 61,761 61,761 19,177 19,177
@material-ui/core/Popper 0.00% 0.00% 28,896 28,896 10,394 10,394
@material-ui/core/Textarea 0.00% 0.00% 5,534 5,534 2,369 2,369
@material-ui/core/TrapFocus 0.00% 0.00% 3,753 3,753 1,577 1,577
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,156 16,156 5,816 5,816
@material-ui/core/useMediaQuery 0.00% 0.00% 3,098 3,098 1,310 1,310
@material-ui/lab 0.00% 0.00% 141,699 141,699 43,813 43,813
@material-ui/styles 0.00% 0.00% 51,886 51,886 15,380 15,380
@material-ui/system 0.00% 0.00% 15,576 15,576 4,445 4,445
Button 0.00% 0.00% 79,711 79,711 24,358 24,358
Modal 0.00% 0.00% 14,548 14,548 5,102 5,102
Portal 0.00% 0.00% 3,471 3,471 1,568 1,568
Rating 0.00% 0.00% 70,267 70,267 22,068 22,068
Slider 0.00% 0.00% 75,096 75,096 23,311 23,311
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 54,357 54,357 13,915 13,915
docs.main 0.00% 0.00% 646,774 646,774 203,034 203,034
packages/material-ui/build/umd/material-ui.production.min.js +0.08% 🔺 +0.08% 🔺 299,666 299,894 86,112 86,181

Generated by 🚫 dangerJS against 94422d1

@darkowic darkowic force-pushed the switch-size-small branch from a281a75 to c7de243 Compare July 17, 2019 09:41
@darkowic
Copy link
Contributor Author

Fixed problems with prettier, lint etc

@oliviertassinari oliviertassinari added component: switch This is the name of the generic UI component, not the React module! new feature New feature or request labels Jul 17, 2019
@oliviertassinari
Copy link
Member

Regarding the UI of the small size. What did you use as inspiration for coming up with the final values / look? For instance, we could make it really small. cc @VMasap.

@darkowic darkowic force-pushed the switch-size-small branch 2 times, most recently from 88049c1 to 0e5c533 Compare July 17, 2019 21:06
@darkowic
Copy link
Contributor Author

@oliviertassinari I based the size (height) on size of the small version of the Button component - https://material-ui.com/components/buttons/#sizes (variant contained). Then the width is simply visually looking good ;) I am open to discussion about it. There is no specification about what is the 'small' size.

@darkowic darkowic force-pushed the switch-size-small branch from 0e5c533 to 08d8fd7 Compare July 17, 2019 21:24
@oliviertassinari
Copy link
Member

@darkowic Ok, thanks for sharing the reflection. I have tried to give it a try. Here is the result I came up with.

1. Benchmarking

I'm looking for any design system that has solved this small problem in the past. I have found:

2. Average size

We have the following size: 32x16, 28x16, 28x16, 32x16. So I would propose a thumb size of 16px. Once we have the thumb size, the track size can be defined conserving the same ratio than with the normal size: 16 / 20 -> x0.8. I also want the slider dimensions to align on the 4px grid.

3. Result

Capture d’écran 2019-07-18 à 22 25 50

  sizeSmall: {
    width: 40,
    height: 24,
    padding: 7,
    '& $thumb': {
      width: 16,
      height: 16,
    },
    '& $switchBase': {
      padding: 4,
    },
  },

What do you think?

@oliviertassinari oliviertassinari changed the title WIP: [Switch] support small size [Switch] Support small size Jul 18, 2019
@oliviertassinari oliviertassinari force-pushed the switch-size-small branch 2 times, most recently from 17aeb74 to fe3987b Compare July 19, 2019 10:06
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.

I have added the TypeScript definitions and apply some of the review. Any more feedback is welcomed! Otherwise, I hope it's good enough.

@oliviertassinari
Copy link
Member

Rebased.

@darkowic
Copy link
Contributor Author

@oliviertassinari Thanks for the research! Great job as always! I am totally ok with this size. Customizing the small size is easy if anyone needs something else. LGTM ;)

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! new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Switch] size property isn't working
4 participants