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

[Slider] Increase hover target size #18074

Merged
merged 1 commit into from
Nov 5, 2019

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Oct 28, 2019

Use touch target size of 48px from Material guidelines (https://material.io/design/usability/accessibility.html#layout-typography)

The actual touch target size was already good since we use the full rail. But we could get some toggle flicker when hovering from label to thumb

master
slider-master-hitbox

vs.

branch
slider-pr-hitbox

@eps1lon eps1lon added new feature New feature or request component: slider This is the name of the generic UI component, not the React module! labels Oct 28, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Oct 28, 2019

Details of bundle changes.

Comparing: e049ad8...4a8de83

bundle Size Change Size Gzip Change Gzip
@material-ui/core[umd] ▲ +84 B (+0.03% ) 306 kB ▲ +35 B (+0.04% ) 88.1 kB
Slider ▲ +84 B (+0.11% ) 73.8 kB ▲ +33 B (+0.14% ) 23.3 kB
@material-ui/core ▲ +84 B (+0.02% ) 347 kB ▲ +26 B (+0.03% ) 95 kB
@material-ui/lab -- 143 kB -- 44.6 kB
@material-ui/styles -- 50.8 kB -- 15.4 kB
@material-ui/system -- 14.8 kB -- 4.07 kB
AppBar -- 62.3 kB -- 19.5 kB
Avatar -- 61.1 kB -- 19.2 kB
Backdrop -- 66.2 kB -- 20.4 kB
Badge -- 63.8 kB -- 19.7 kB
BottomNavigation -- 60.8 kB -- 19 kB
BottomNavigationAction -- 73.8 kB -- 23.3 kB
Box -- 69.2 kB -- 20.9 kB
Breadcrumbs -- 66.4 kB -- 20.8 kB
Button -- 77.7 kB -- 24.1 kB
ButtonBase -- 72.2 kB -- 22.6 kB
ButtonGroup -- 62.6 kB -- 19.5 kB
Card -- 61.3 kB -- 19.2 kB
CardActionArea -- 73.3 kB -- 23.1 kB
CardActions -- 60.5 kB -- 18.9 kB
CardContent -- 60.4 kB -- 18.9 kB
CardHeader -- 63.5 kB -- 20 kB
CardMedia -- 60.8 kB -- 19.1 kB
Checkbox -- 80 kB -- 25.1 kB
Chip -- 69 kB -- 21.3 kB
CircularProgress -- 62.5 kB -- 19.7 kB
ClickAwayListener -- 3.85 kB -- 1.55 kB
Collapse -- 66.3 kB -- 20.5 kB
colorManipulator -- 3.83 kB -- 1.52 kB
Container -- 61.6 kB -- 19.2 kB
CssBaseline -- 56 kB -- 17.5 kB
Dialog -- 80.9 kB -- 25.1 kB
DialogActions -- 60.5 kB -- 18.9 kB
DialogContent -- 60.6 kB -- 19 kB
DialogContentText -- 62.5 kB -- 19.6 kB
DialogTitle -- 62.7 kB -- 19.7 kB
Divider -- 61 kB -- 19.1 kB
docs.landing -- 55.5 kB -- 14.5 kB
docs.main -- 599 kB -- 191 kB
Drawer -- 82.7 kB -- 25.6 kB
ExpansionPanel -- 69.5 kB -- 21.7 kB
ExpansionPanelActions -- 60.5 kB -- 18.9 kB
ExpansionPanelDetails -- 60.4 kB -- 18.9 kB
ExpansionPanelSummary -- 76.3 kB -- 24.1 kB
Fab -- 75.1 kB -- 23.3 kB
Fade -- 22 kB -- 7.6 kB
FilledInput -- 71.5 kB -- 22.2 kB
FormControl -- 62.7 kB -- 19.4 kB
FormControlLabel -- 63.9 kB -- 20.1 kB
FormGroup -- 60.4 kB -- 18.9 kB
FormHelperText -- 61.7 kB -- 19.3 kB
FormLabel -- 61.7 kB -- 19.1 kB
Grid -- 63.5 kB -- 19.9 kB
GridList -- 60.9 kB -- 19.1 kB
GridListTile -- 62.1 kB -- 19.5 kB
GridListTileBar -- 61.6 kB -- 19.3 kB
Grow -- 22.6 kB -- 7.72 kB
Hidden -- 64.5 kB -- 20.2 kB
Icon -- 61.2 kB -- 19.1 kB
IconButton -- 74.4 kB -- 23.2 kB
Input -- 70.5 kB -- 22 kB
InputAdornment -- 63.5 kB -- 20 kB
InputBase -- 68.6 kB -- 21.5 kB
InputLabel -- 63.5 kB -- 19.8 kB
LinearProgress -- 63.7 kB -- 19.8 kB
Link -- 65 kB -- 20.6 kB
List -- 60.8 kB -- 18.9 kB
ListItem -- 75.4 kB -- 23.5 kB
ListItemAvatar -- 60.5 kB -- 18.9 kB
ListItemIcon -- 60.6 kB -- 19 kB
ListItemSecondaryAction -- 60.4 kB -- 18.9 kB
ListItemText -- 63.4 kB -- 19.9 kB
ListSubheader -- 61.2 kB -- 19.2 kB
Menu -- 86.6 kB -- 27.2 kB
MenuItem -- 76.4 kB -- 23.8 kB
MenuList -- 64.4 kB -- 20.1 kB
MobileStepper -- 66.2 kB -- 20.6 kB
Modal -- 14.2 kB -- 4.97 kB
NativeSelect -- 74.8 kB -- 23.5 kB
NoSsr -- 2.19 kB -- 1.04 kB
OutlinedInput -- 72.1 kB -- 22.4 kB
Paper -- 60.8 kB -- 18.9 kB
Popover -- 81 kB -- 25 kB
Popper -- 28.4 kB -- 10.2 kB
Portal -- 2.87 kB -- 1.29 kB
Radio -- 80.9 kB -- 25.4 kB
RadioGroup -- 61.7 kB -- 19.3 kB
Rating -- 68.3 kB -- 21.8 kB
RootRef -- 4.43 kB -- 1.67 kB
Select -- 112 kB -- 33.2 kB
Skeleton -- 60.9 kB -- 19.1 kB
Slide -- 24.1 kB -- 8.21 kB
Snackbar -- 75.6 kB -- 23.5 kB
SnackbarContent -- 64.2 kB -- 20.1 kB
SpeedDial -- 84.3 kB -- 26.5 kB
SpeedDialAction -- 114 kB -- 36 kB
SpeedDialIcon -- 63 kB -- 19.8 kB
Step -- 61 kB -- 19.1 kB
StepButton -- 80.6 kB -- 25.3 kB
StepConnector -- 61.1 kB -- 19.2 kB
StepContent -- 67.4 kB -- 21 kB
StepIcon -- 63.1 kB -- 19.6 kB
StepLabel -- 67 kB -- 21 kB
Stepper -- 63.3 kB -- 19.9 kB
styles/createMuiTheme -- 15.2 kB -- 5.36 kB
SvgIcon -- 61.5 kB -- 19.1 kB
SwipeableDrawer -- 89 kB -- 27.6 kB
Switch -- 79.3 kB -- 24.7 kB
Tab -- 74.6 kB -- 23.6 kB
Table -- 61 kB -- 19.1 kB
TableBody -- 60.5 kB -- 18.9 kB
TableCell -- 62.5 kB -- 19.6 kB
TableFooter -- 60.5 kB -- 18.9 kB
TableHead -- 60.5 kB -- 18.9 kB
TablePagination -- 138 kB -- 40.4 kB
TableRow -- 60.9 kB -- 19.1 kB
TableSortLabel -- 75.6 kB -- 23.9 kB
Tabs -- 83.7 kB -- 26.7 kB
TextareaAutosize -- 5.06 kB -- 2.11 kB
TextField -- 120 kB -- 35.2 kB
ToggleButton -- 74.4 kB -- 23.5 kB
ToggleButtonGroup -- 61.6 kB -- 19.3 kB
Toolbar -- 60.7 kB -- 19 kB
Tooltip -- 97.6 kB -- 30.9 kB
TreeItem -- 71.8 kB -- 22.6 kB
TreeView -- 64.4 kB -- 20.2 kB
Typography -- 62.1 kB -- 19.3 kB
useMediaQuery -- 2.49 kB -- 1.05 kB
Zoom -- 22.1 kB -- 7.61 kB

Generated by 🚫 dangerJS against 4a8de83

@oliviertassinari
Copy link
Member

Given it's the first time (I believe?) we apply this approach to improve the touch zone area on mobile, do you know if we have different components to handle? If we do, does the support scale to these other components?

For instance, the Slider's rail padding was a result of a tradeoff to allow compact display while maintaining a not too bad interaction zone. As we handle the thumb in this pull request, should we do something for the rail zone?

@eps1lon
Copy link
Member Author

eps1lon commented Oct 28, 2019

It is a regression from the previous implementation.

What should we do for the rail?

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 28, 2019

@eps1lon Benchmarking alternative implementations, I have found the following:

  • Should we discriminate the desktop vs the mobile version? It seems that mobile needs a larger touch target while desktop should be more precise.
  • The rails touch zone seems to be as important as the thumb to improve the UX:

Capture d’écran 2019-10-28 à 21 19 49o
blueprint

Capture d’écran 2019-10-28 à 21 14 24
ionic

Capture d’écran 2019-10-28 à 21 15 39
MDC

Capture d’écran 2019-10-28 à 21 16 41
BaseUI

I would propose to increase the whole slider interactive zone, only on "mobile". What do you think about something like this?:

diff --git a/packages/material-ui/src/Slider/Slider.js b/packages/material-ui/src/Slider/Slider.js
index 01d73ad0e..c0a303b48 100644
--- a/packages/material-ui/src/Slider/Slider.js
+++ b/packages/material-ui/src/Slider/Slider.js
@@ -154,6 +154,18 @@ export const styles = theme => ({
       height: '100%',
       padding: '0 11px',
     },
+    // The primary input mechanism of the device includes a pointing device of limited accuracy.
+    '@media (pointer: coarse)': {
+      '&::after': {
+        position: 'absolute',
+        content: '""',
+        // Reach 42px touch target, about ~8mm on screen.
+        left: -9,
+        top: -9,
+        right: -9,
+        bottom: -9,
+      },
+    },
   },

With 9/48*42 = 7.875mm, we should be within the Material Design recommended range:

The recommended target size for touchscreen elements is 7-10mm.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 28, 2019

It is a regression from the previous implementation.

@eps1lon Thanks for looking at this regression! I hadn't realized it.

@eps1lon
Copy link
Member Author

eps1lon commented Oct 28, 2019

It seems that mobile needs a larger touch target while desktop should be more precise.

We don't branch in the core for other components. Why start specifically here?

With 9/48*42 = 7.875mm, we should be within the Material Design recommended range:

I have no idea where these numbers come from.

Why start deviating here but not in the other components? If you have a general issue with touch hit size please open an issue. I tried to follow the general core approach. If I didn't please point to the instances where we do it differently. Otherwise we can address this across the core in a separate PR. Same applies to the rail. The primary concern is fixing a regression.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 28, 2019

I have no idea where these numbers come from.

Sorry, in https://material.io/design/usability/accessibility.html#layout-typography, the spec mention that 48px on screens translate, on average to 9mm. From this correspondence, we can estimate the size on the screen of 42px.

We don't branch in the core for other components. Why start specifically here?

It's a great question!

  • Ant Design, Rsuite and Element (3 Chineses lib) branch for the Slider component, similarly to the proposed diff.
  • It seems that the Slider component is the only one we have that has a long and slim interaction surface area. Do you see another component that has the same characteristics that we could share the same solution with? I was wondering about the same in my initial comment.
  • On a touch device, the current size of the rails makes it hard to interact with. I think that we both agree that it needs to be increased. However, if we increase its size on a pointer device, it might take too much space and create unintended interactions. YouTube slider is 22px tall, ours is 24px, we are above.

The primary concern is fixing a regression.

It seems that the rail is an integral part of the regression, at least, if we compare it with v3.9.3. Should we include it?

Capture d’écran 2019-10-28 à 22 22 35

@oliviertassinari
Copy link
Member

I'm wondering about something with the &:after approach. What happens if there is another interactive element it overflows with, like a button?

@oliviertassinari
Copy link
Member

I'm wondering about something with the &:after approach. What happens if there is another interactive element it overflows with, like a button?

I have tested it, it prevents the interaction with the other elements. Having a different behavior based on the precision of the pointer device might create unpredictable interaction zone overlaps. So I don't think that my proposed approach is great. Maybe something like this instead to avoid conflicts?

--- a/packages/material-ui/src/Slider/Slider.js
+++ b/packages/material-ui/src/Slider/Slider.js
@@ -154,6 +154,18 @@ export const styles = theme => ({
       height: '100%',
       padding: '0 11px',
     },
+    // The primary input mechanism of the device includes a pointing device of limited accuracy.
+    '@media (pointer: coarse)': {
+      // Reach 42px touch target, about ~8mm on screen.
+      padding: '20px 0',
+      '&$vertical': {
+        padding: '0 20px',
+      },
+    },
   },

@eps1lon
Copy link
Member Author

eps1lon commented Nov 4, 2019

I have tested it, it prevents the interaction with the other elements.

Please include this test.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 4, 2019

@eps1lon Sure: https://codesandbox.io/s/material-demo-u9y9b would be my only concern with the change.

@eps1lon
Copy link
Member Author

eps1lon commented Nov 4, 2019

@eps1lon Sure: https://codesandbox.io/s/material-demo-u9y9b.

In general this UI should be discouraged (and is so by Material guidelines) since the space between interactive elements is very small.

[...] with at least 8dp of space between them.

-- https://material.io/design/layout/spacing-methods.html#touch-targets

Beyond that I can't find an issue with your demo (using latest) or my fork (using this PR: https://codesandbox.io/s/material-demo-vwilb)

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 4, 2019

Beyond that I can't find an issue with your demo (using latest) or my fork (using this PR:

codesandbox-ci to the rescue 👌! Right, it doesn't reproduce, I had to inverse the order to make it noticeable:
https://codesandbox.io/s/material-demo-hzimd.

In general this UI should be discouraged (and is so by Material guidelines) since the space between interactive elements is very small.

Yeah, there are some points to consider in this land:

  • I think that there is a good balance to find between too small and too big elements. Ultimately, if we pick wrong, I believe our users will complain, so I wouldn't worry too much about it.
  • I see two main issues with a too large interaction zone: 1. users might not expect an element to be interactive too far from this visibility footprint. For instance, I personally use blank space to clear my text selection or close a popup, somewhere I feel safe not to trigger any action. 2. it can prevent users to implement dense UI.

What do you think of reducing the interaction zone width and height from 48px to 24px? It's 12px on master, we would still x4 the interaction surface with 24px.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y component: slider 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.

3 participants