Sebastian review

Co-Authored-By: Sebastian Silbermann <[email protected]>

Author: oliviertassinari

docs/src/pages/components/slider/ContinuousSlider.js

@@ -14,6 +14,11 @@ const useStyles = makeStyles({
 
 export default function ContinuousSlider() {
   const classes = useStyles();
+  const [value, setValue] = React.useState(30);
+
+  const handleChange = (event, newValue) => {
+    setValue(newValue);
+  };
 
   return (
     <div className={classes.root}>
@@ -25,7 +30,7 @@ export default function ContinuousSlider() {
           <VolumeDown />
         </Grid>
         <Grid item xs>
-          <Slider defaultValue={30} aria-labelledby="continuous-slider" />
+          <Slider value={value} onChange={handleChange} aria-labelledby="continuous-slider" />
         </Grid>
         <Grid item>
           <VolumeUp />

docs/src/pages/components/slider/ContinuousSlider.tsx

@@ -14,6 +14,11 @@ const useStyles = makeStyles({
 
 export default function ContinuousSlider() {
   const classes = useStyles();
+  const [value, setValue] = React.useState<number | number[]>(30);
+
+  const handleChange = (event: any, newValue: number | number[]) => {
+    setValue(newValue);
+  };
 
   return (
     <div className={classes.root}>
@@ -25,7 +30,7 @@ export default function ContinuousSlider() {
           <VolumeDown />
         </Grid>
         <Grid item xs>
-          <Slider defaultValue={30} aria-labelledby="continuous-slider" />
+          <Slider value={value} onChange={handleChange} aria-labelledby="continuous-slider" />
         </Grid>
         <Grid item>
           <VolumeUp />

docs/src/pages/components/slider/CustomizedSlider.js

@@ -78,14 +78,15 @@ const IOSSlider = withStyles({
     boxShadow: iOSBoxShadow,
     marginTop: -14,
     marginLeft: -14,
-    '&:focus,&:hover': {
+    '&:focus,&:hover,&$active': {
       boxShadow: '0 3px 1px rgba(0,0,0,0.1),0 4px 8px rgba(0,0,0,0.3),0 0 0 1px rgba(0,0,0,0.02)',
       // Reset on touch devices, it doesn't add specificity
       '@media (hover: none)': {
         boxShadow: iOSBoxShadow,
       },
     },
   },
+  active: {},
   valueLabel: {
     left: 'calc(-50% + 11px)',
     top: -22,
@@ -125,10 +126,11 @@ const PrettoSlider = withStyles({
     border: '2px solid currentColor',
     marginTop: -8,
     marginLeft: -12,
-    '&:focus,&:hover': {
+    '&:focus,&:hover,&$active': {
       boxShadow: 'inherit',
     },
   },
+  active: {},
   valueLabel: {
     left: 'calc(-50% + 4px)',
   },
@@ -156,7 +158,7 @@ const AirbnbSlider = withStyles({
     marginTop: -12,
     marginLeft: -13,
     boxShadow: '#ebebeb 0px 2px 2px',
-    '&:focus,&:hover': {
+    '&:focus,&:hover,&$active': {
       boxShadow: '#ccc 0px 2px 3px 1px',
     },
     '& .bar': {
@@ -168,6 +170,7 @@ const AirbnbSlider = withStyles({
       marginRight: 1,
     },
   },
+  active: {},
   valueLabel: {
     left: 'calc(-50% + 4px)',
   },

docs/src/pages/components/slider/CustomizedSlider.tsx

@@ -87,14 +87,15 @@ const IOSSlider = withStyles({
     boxShadow: iOSBoxShadow,
     marginTop: -14,
     marginLeft: -14,
-    '&:focus,&:hover': {
+    '&:focus,&:hover,&$active': {
       boxShadow: '0 3px 1px rgba(0,0,0,0.1),0 4px 8px rgba(0,0,0,0.3),0 0 0 1px rgba(0,0,0,0.02)',
       // Reset on touch devices, it doesn't add specificity
       '@media (hover: none)': {
         boxShadow: iOSBoxShadow,
       },
     },
   },
+  active: {},
   valueLabel: {
     left: 'calc(-50% + 11px)',
     top: -22,
@@ -134,10 +135,11 @@ const PrettoSlider = withStyles({
     border: '2px solid currentColor',
     marginTop: -8,
     marginLeft: -12,
-    '&:focus,&:hover': {
+    '&:focus,&:hover,&$active': {
       boxShadow: 'inherit',
     },
   },
+  active: {},
   valueLabel: {
     left: 'calc(-50% + 4px)',
   },
@@ -165,7 +167,7 @@ const AirbnbSlider = withStyles({
     marginTop: -12,
     marginLeft: -13,
     boxShadow: '#ebebeb 0px 2px 2px',
-    '&:focus,&:hover': {
+    '&:focus,&:hover,&$active': {
       boxShadow: '#ccc 0px 2px 3px 1px',
     },
     '& .bar': {
@@ -177,6 +179,7 @@ const AirbnbSlider = withStyles({
       marginRight: 1,
     },
   },
+  active: {},
   valueLabel: {
     left: 'calc(-50% + 4px)',
   },

docs/src/pages/components/slider/RangeSlider.js

@@ -15,14 +15,20 @@ function valuetext(value) {
 
 export default function RangeSlider() {
   const classes = useStyles();
+  const [value, setValue] = React.useState([20, 37]);
+
+  const handleChange = (event, newValue) => {
+    setValue(newValue);
+  };
 
   return (
     <div className={classes.root}>
       <Typography id="range-slider" gutterBottom>
         Temperature range
       </Typography>
       <Slider
-        defaultValue={[20, 37]}
+        value={value}
+        onChange={handleChange}
         valueLabelDisplay="auto"
         aria-labelledby="range-slider"
         getAriaValueText={valuetext}

docs/src/pages/components/slider/RangeSlider.tsx

@@ -16,14 +16,20 @@ function valuetext(value: number) {
 
 export default function RangeSlider() {
   const classes = useStyles();
+  const [value, setValue] = React.useState<number | number[]>([20, 37]);
+
+  const handleChange = (event: any, newValue: number | number[]) => {
+    setValue(newValue);
+  };
 
   return (
     <div className={classes.root}>
       <Typography id="range-slider" gutterBottom>
         Temperature range
       </Typography>
       <Slider
-        defaultValue={[20, 37]}
+        value={value}
+        onChange={handleChange}
         valueLabelDisplay="auto"
         aria-labelledby="range-slider"
         getAriaValueText={valuetext}

docs/src/pages/components/slider/slider.md

@@ -53,4 +53,6 @@ The component handles most of the work necessary to make it accessible.
 However, you need to make sure that:
 
 - The slider, as a whole, has a label (`aria-label` or `aria-labelledby` prop).
-- Each thumb has its current value labeled correctly (`getAriaValueText` or `aria-valuetext` prop).
+- Each thumb has a user-friendly name for its current value.
+This is not required if the value matches the semantics of the label.
+You can change the name with the `getAriaValueText` or `aria-valuetext` prop.

packages/material-ui-lab/src/Slider/Slider.js

@@ -4,7 +4,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import clsx from 'clsx';
 import { useTheme, withStyles, fade, lighten } from '@material-ui/core/styles';
-import { useForkRef, ownerWindow } from '@material-ui/core/utils';
+import { useForkRef, ownerWindow, useIsFocusVisible } from '@material-ui/core/utils';
 import { chainPropTypes } from '@material-ui/utils';
 import ValueLabel from './ValueLabel';
 
@@ -163,9 +163,9 @@ export const styles = theme => ({
       marginRight: 20,
     },
   },
-  /* Styles applied to the root element if `orientation="vertical"`. */
+  /* Pseudo-class applied to the root element if `orientation="vertical"`. */
   vertical: {},
-  /* Styles applied to the root element if `disabled={true}`. */
+  /* Pseudo-class applied to the root element if `disabled={true}`. */
   disabled: {},
   /* Styles applied to the rail element. */
   rail: {
@@ -174,7 +174,7 @@ export const styles = theme => ({
     height: 2,
     borderRadius: 1,
     backgroundColor: 'currentColor',
-    opacity: 0.24,
+    opacity: 0.38,
     '$vertical &': {
       height: '100%',
       width: 2,
@@ -207,8 +207,11 @@ export const styles = theme => ({
     transition: theme.transitions.create(['box-shadow'], {
       duration: theme.transitions.duration.shortest,
     }),
-    '&:focus,&:hover': {
+    '&$focusVisible,&:hover': {
       boxShadow: `0px 0px 0px 8px ${fade(theme.palette.primary.main, 0.16)}`,
+      '@media (hover: none)': {
+        boxShadow: 'none',
+      },
     },
     '&$active': {
       boxShadow: `0px 0px 0px 14px ${fade(theme.palette.primary.main, 0.16)}`,
@@ -219,7 +222,7 @@ export const styles = theme => ({
       height: 8,
       marginLeft: -4,
       marginTop: -3,
-      '&:focus,&:hover': {
+      '&:hover': {
         boxShadow: 'none',
       },
     },
@@ -232,8 +235,10 @@ export const styles = theme => ({
       marginBottom: -4,
     },
   },
-  /* Styles applied to the thumb element if it's active. */
+  /* Pseudo-class applied to the thumb element if it's active. */
   active: {},
+  /* Pseudo-class applied to the thumb element if keyboard focused. */
+  focusVisible: {},
   /* Styles applied to the thumb label element. */
   valueLabel: {},
   /* Styles applied to the mark element. */
@@ -320,11 +325,21 @@ const Slider = React.forwardRef(function Slider(props, ref) {
     source: valueDerived, // Keep track of the input value to leverage immutable state comparison.
   };
 
+  const { isFocusVisible, onBlurVisible, ref: focusVisibleRef } = useIsFocusVisible();
+  const [focusVisible, setFocusVisible] = React.useState(-1);
+
   const handleFocus = useEventCallback(event => {
     const index = Number(event.currentTarget.getAttribute('data-index'));
+    if (isFocusVisible(event)) {
+      setFocusVisible(index);
+    }
     setOpen(index);
   });
   const handleBlur = useEventCallback(() => {
+    if (focusVisible !== -1) {
+      setFocusVisible(-1);
+      onBlurVisible();
+    }
     setOpen(-1);
   });
   const handleMouseOver = useEventCallback(event => {
@@ -412,7 +427,8 @@ const Slider = React.forwardRef(function Slider(props, ref) {
   });
 
   const sliderRef = React.useRef();
-  const handleRef = useForkRef(sliderRef, ref);
+  const handleFocusRef = useForkRef(focusVisibleRef, sliderRef);
+  const handleRef = useForkRef(ref, handleFocusRef);
   const previousIndex = React.useRef();
   let axis = orientation;
   if (theme.direction === 'rtl' && orientation === 'horizontal') {
@@ -663,6 +679,7 @@ const Slider = React.forwardRef(function Slider(props, ref) {
             <ThumbComponent
               className={clsx(classes.thumb, {
                 [classes.active]: active === index,
+                [classes.focusVisible]: focusVisible === index,
               })}
               tabIndex={disabled ? null : 0}
               role="slider"
@@ -690,11 +707,11 @@ const Slider = React.forwardRef(function Slider(props, ref) {
 
 Slider.propTypes = {
   /**
-   * An alternative to `aria-labelledby`.
+   * The label of the slider.
    */
   'aria-label': PropTypes.string,
   /**
-   * Refers to the element containing the name of the slider.
+   * The id of the element containing a label for the slider.
    */
   'aria-labelledby': PropTypes.string,
   /**

packages/material-ui/src/utils/focusVisible.js

@@ -75,7 +75,7 @@ function handleVisibilityChange() {
   }
 }
 
-export function prepare(ownerDocument) {
+function prepare(ownerDocument) {
   ownerDocument.addEventListener('keydown', handleKeyDown, true);
   ownerDocument.addEventListener('mousedown', handlePointerDown, true);
   ownerDocument.addEventListener('pointerdown', handlePointerDown, true);
@@ -91,7 +91,7 @@ export function teardown(ownerDocument) {
   ownerDocument.removeEventListener('visibilitychange', handleVisibilityChange, true);
 }
 
-export function isFocusVisible(event) {
+function isFocusVisible(event) {
   const { target } = event;
   try {
     return target.matches(':focus-visible');
@@ -110,7 +110,7 @@ export function isFocusVisible(event) {
 /**
  * Should be called if a blur event is fired on a focus-visible element
  */
-export function handleBlurVisible() {
+function handleBlurVisible() {
   // To detect a tab/window switch, we look for a blur event followed
   // rapidly by a visibility change.
   // If we don't see a visibility change within 100ms, it's probably a

packages/material-ui/src/utils/index.js

@@ -1,8 +1,9 @@
 export { default as deprecatedPropType } from './deprecatedPropType';
 export * from './helpers';
 export * from './reactHelpers';
-export { default as requirePropFactory } from './requirePropFactory';
+export { useIsFocusVisible } from './focusVisible';
 export { default as ownerDocument } from './ownerDocument';
 export { default as ownerWindow } from './ownerWindow';
+export { default as requirePropFactory } from './requirePropFactory';
 export { default as unsupportedProp } from './unsupportedProp';
 export { default as withForwardedRef } from './withForwardedRef';