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

[TableSortLabel] Increase size and show on hover #14650

Merged
merged 5 commits into from
Feb 26, 2019

Conversation

leMaik
Copy link
Member

@leMaik leMaik commented Feb 24, 2019

This PR updates the table sort label to be spec-compliant again. 🎉

As seen here, the icon size is 18px now (with 3px padding, simmilar to <IconButton size="small" /> (not merged yet):
image

As seen here, the arrow is now visible (in a less opaque color) when hovering the label. This improves the visibility of the sorting feature and reduces surprises when clicking on it.

image

@leMaik leMaik added design: material This is about Material Design, please involve a visual or UX designer in the process component: table This is the name of the generic UI component, not the React module! labels Feb 24, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Feb 24, 2019

Details of bundle changes.

Comparing: 1ca7af1...f5cf175

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.03% 🔺 +0.01% 🔺 368,525 368,622 91,360 91,372
@material-ui/core/Paper 0.00% 0.00% 76,647 76,647 19,297 19,297
@material-ui/core/Paper.esm 0.00% 0.00% 71,595 71,595 18,771 18,771
@material-ui/core/Popper 0.00% 0.00% 30,462 30,462 10,584 10,584
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 17,286 17,286 5,720 5,720
@material-ui/core/useMediaQuery 0.00% 0.00% 2,486 2,486 1,054 1,054
@material-ui/lab 0.00% 0.00% 182,793 182,793 50,217 50,217
@material-ui/styles 0.00% 0.00% 57,708 57,708 16,237 16,237
@material-ui/system 0.00% 0.00% 17,062 17,062 4,486 4,486
Button 0.00% 0.00% 99,068 99,068 26,484 26,484
Modal 0.00% 0.00% 98,649 98,649 26,152 26,152
colorManipulator 0.00% 0.00% 3,232 3,232 1,297 1,297
docs.landing 0.00% 0.00% 49,899 49,899 10,728 10,728
docs.main 0.00% 0.00% 677,542 677,542 205,746 205,746
packages/material-ui/build/umd/material-ui.production.min.js +0.03% 🔺 +0.02% 🔺 319,754 319,851 84,598 84,613

Generated by 🚫 dangerJS against f5cf175

@mbrookes
Copy link
Member

Looks good. We don't need the "Sort" tooltip any more.

(The spec says to use the tooltip to show the full text of truncated headings, but the screenshots show it being used for a more verbose description of the column content... 🤔 )

@leMaik
Copy link
Member Author

leMaik commented Feb 24, 2019

We don't need the "Sort" tooltip any more.

I like UIs that don't need tooltips to explain how they work. 🎉

The spec says [...], but the screenshots [...]

Material Design specs in a nutshell…

@oliviertassinari oliviertassinari merged commit 730e873 into mui:next Feb 26, 2019
@leMaik leMaik deleted the table-sort-label branch February 26, 2019 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: table This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants