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

[Rating] Fix a few issues #17270

Merged
merged 1 commit into from Sep 2, 2019
Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Sep 1, 2019

  • Fix hover logic. This is a recent regression ([Rating] Improve rendering of arbitrary precision  #17013), I have added a test for it.
  • Display inline instead of block. I have used the component in https://themes.material-ui.com, I quickly realized that we could have a better default. This change is breaking. I have looked at a couple of the generic rating components, the benchmark confirms that display inline is likely a better default.
  • Fix warning in the demos. This is a recent regression.
  • Remove the need for ownerWindow(). As it turns out, we can do the computation with the viewport as the origin (getBoundingClientRect + clientX). This should help with the bundle size.

@oliviertassinari oliviertassinari added the component: rating This is the name of the generic UI component, not the React module! label Sep 1, 2019
@mui-pr-bot
Copy link

Details of bundle changes.

Comparing: 0720bfa...65ab8aa

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core -0.02% -0.03% 331,074 331,013 90,395 90,367
@material-ui/core/Paper 0.00% 0.00% 68,774 68,774 20,485 20,485
@material-ui/core/Paper.esm 0.00% +0.01% 🔺 62,148 62,148 19,212 19,214
@material-ui/core/Popper 0.00% 0.00% 28,466 28,466 10,190 10,190
@material-ui/core/Textarea 0.00% 0.00% 5,094 5,094 2,137 2,137
@material-ui/core/TrapFocus 0.00% 0.00% 3,834 3,834 1,613 1,613
@material-ui/core/styles/createMuiTheme -0.01% -0.02% 16,386 16,385 5,828 5,827
@material-ui/core/useMediaQuery 0.00% 0.00% 2,541 2,541 1,058 1,058
@material-ui/lab -0.19% -0.17% 153,503 153,213 46,734 46,655
@material-ui/styles 0.00% -0.01% 51,492 51,492 15,304 15,303
@material-ui/system 0.00% +0.02% 🔺 15,668 15,668 4,358 4,359
Button 0.00% -0.02% 78,727 78,727 24,066 24,061
Modal 0.00% 0.00% 14,344 14,344 5,019 5,019
Portal 0.00% 0.00% 2,907 2,907 1,318 1,318
Rating -0.33% -0.36% 70,243 70,014 21,945 21,866
Slider -0.33% -0.29% 74,483 74,240 23,062 22,994
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 52,253 52,253 13,780 13,780
docs.main 0.00% 0.00% 597,413 597,413 190,804 190,804
packages/material-ui/build/umd/material-ui.production.min.js -0.01% -0.02% 301,963 301,931 86,702 86,682

Generated by 🚫 dangerJS against 65ab8aa

@oliviertassinari oliviertassinari merged commit e3df147 into mui:master Sep 2, 2019
@oliviertassinari oliviertassinari deleted the rating-fixes branch February 8, 2020 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change bug 🐛 Something doesn't work component: rating This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants