use state ref instead

mui · Aug 8, 2020 · d3eb864 · d3eb864
1 parent 6a4ca14
commit d3eb864
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 58 deletions.
diff --git a/packages/material-ui-lab/src/Rating/Rating.js b/packages/material-ui-lab/src/Rating/Rating.js
@@ -179,7 +179,12 @@ const Rating = React.forwardRef(function Rating(props, ref) {
     value = focus;
   }
 
-  const { isFocusVisible, onBlurVisible, ref: focusVisibleRef } = useIsFocusVisible();
+  const {
+    isFocusVisibleRef,
+    onBlur: handleBlurVisible,
+    onFocus: handleFocusVisible,
+    ref: focusVisibleRef,
+  } = useIsFocusVisible();
   const [focusVisible, setFocusVisible] = React.useState(false);
 
   const rootRef = React.useRef();
@@ -267,7 +272,8 @@ const Rating = React.forwardRef(function Rating(props, ref) {
   };
 
   const handleFocus = (event) => {
-    if (isFocusVisible(event)) {
+    handleFocusVisible(event);
+    if (isFocusVisibleRef.current === true) {
       setFocusVisible(true);
     }
 
@@ -287,9 +293,9 @@ const Rating = React.forwardRef(function Rating(props, ref) {
       return;
     }
 
-    if (focusVisible !== false) {
+    handleBlurVisible(event);
+    if (isFocusVisibleRef.current === false) {
       setFocusVisible(false);
-      onBlurVisible();
     }
 
     const newFocus = -1;

diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.js b/packages/material-ui/src/ButtonBase/ButtonBase.js
@@ -90,11 +90,19 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) {
 
   const rippleRef = React.useRef(null);
 
+  const {
+    isFocusVisibleRef,
+    onFocus: handleFocusVisible,
+    onBlur: handleBlurVisible,
+    ref: focusVisibleRef,
+  } = useIsFocusVisible();
   const [focusVisible, setFocusVisible] = React.useState(false);
   if (disabled && focusVisible) {
     setFocusVisible(false);
   }
-  const { isFocusVisible, onBlurVisible, ref: focusVisibleRef } = useIsFocusVisible();
+  React.useEffect(() => {
+    isFocusVisibleRef.current = focusVisible;
+  }, [focusVisible, isFocusVisibleRef]);
 
   React.useImperativeHandle(
     action,
@@ -143,18 +151,11 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) {
   const handleTouchEnd = useRippleHandler('stop', onTouchEnd);
   const handleTouchMove = useRippleHandler('stop', onTouchMove);
 
-  const hadFocusVisibleEventRef = React.useRef(false);
   const handleBlur = useRippleHandler(
     'stop',
     (event) => {
-      // checking against `focusVisible` does not suffice if we focus and blur syncronously.
-      // React wouldn't have time to trigger a re-render so `focusVisible` would be stale.
-      // Ideally we would adjust `isFocusVisible(event)` to look at `relatedTarget` for blur events.
-      // This doesn't work in IE 11 due to https://github.com/facebook/react/issues/3751
-      // TODO: check again if React releases their internal changes to focus event handling (https://github.com/facebook/react/pull/19186).
-      if (hadFocusVisibleEventRef.current) {
-        hadFocusVisibleEventRef.current = false;
-        onBlurVisible(event);
+      handleBlurVisible(event);
+      if (isFocusVisibleRef.current === false) {
         setFocusVisible(false);
       }
       if (onBlur) {
@@ -170,8 +171,8 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) {
       buttonRef.current = event.currentTarget;
     }
 
-    if (isFocusVisible(event)) {
-      hadFocusVisibleEventRef.current = true;
+    handleFocusVisible(event);
+    if (isFocusVisibleRef.current === true) {
       setFocusVisible(true);
 
       if (onFocusVisible) {

diff --git a/packages/material-ui/src/Link/Link.js b/packages/material-ui/src/Link/Link.js
@@ -68,20 +68,26 @@ const Link = React.forwardRef(function Link(props, ref) {
     ...other
   } = props;
 
-  const { isFocusVisible, onBlurVisible, ref: focusVisibleRef } = useIsFocusVisible();
+  const {
+    isFocusVisibleRef,
+    onBlur: handleBlurVisible,
+    onFocus: handleFocusVisible,
+    ref: focusVisibleRef,
+  } = useIsFocusVisible();
   const [focusVisible, setFocusVisible] = React.useState(false);
   const handlerRef = useForkRef(ref, focusVisibleRef);
   const handleBlur = (event) => {
-    if (focusVisible) {
-      onBlurVisible();
+    handleBlurVisible(event);
+    if (isFocusVisibleRef.current === false) {
       setFocusVisible(false);
     }
     if (onBlur) {
       onBlur(event);
     }
   };
   const handleFocus = (event) => {
-    if (isFocusVisible(event)) {
+    handleFocusVisible(event);
+    if (isFocusVisibleRef.current === true) {
       setFocusVisible(true);
     }
     if (onFocus) {

diff --git a/packages/material-ui/src/Slider/Slider.js b/packages/material-ui/src/Slider/Slider.js
@@ -394,7 +394,12 @@ const Slider = React.forwardRef(function Slider(props, ref) {
         }))
       : marksProp || [];
 
-  const { isFocusVisible, onBlurVisible, ref: focusVisibleRef } = useIsFocusVisible();
+  const {
+    isFocusVisibleRef,
+    onBlur: handleBlurVisible,
+    onFocus: handleFocusVisible,
+    ref: focusVisibleRef,
+  } = useIsFocusVisible();
   const [focusVisible, setFocusVisible] = React.useState(-1);
 
   const sliderRef = React.useRef();
@@ -403,15 +408,16 @@ const Slider = React.forwardRef(function Slider(props, ref) {
 
   const handleFocus = useEventCallback((event) => {
     const index = Number(event.currentTarget.getAttribute('data-index'));
-    if (isFocusVisible(event)) {
+    handleFocusVisible(event);
+    if (isFocusVisibleRef.current === true) {
       setFocusVisible(index);
     }
     setOpen(index);
   });
-  const handleBlur = useEventCallback(() => {
-    if (focusVisible !== -1) {
+  const handleBlur = useEventCallback((event) => {
+    handleBlurVisible(event);
+    if (isFocusVisibleRef.current === false) {
       setFocusVisible(-1);
-      onBlurVisible();
     }
     setOpen(-1);
   });

diff --git a/packages/material-ui/src/Tooltip/Tooltip.js b/packages/material-ui/src/Tooltip/Tooltip.js
@@ -291,12 +291,19 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
     }
   };
 
-  const { isFocusVisible, onBlurVisible, ref: focusVisibleRef } = useIsFocusVisible();
-  const [childIsFocusVisible, setChildIsFocusVisible] = React.useState(false);
-  const handleBlur = () => {
-    if (childIsFocusVisible) {
+  const {
+    isFocusVisibleRef,
+    onBlur: handleBlurVisible,
+    onFocus: handleFocusVisible,
+    ref: focusVisibleRef,
+  } = useIsFocusVisible();
+  // We don't necessarily care about the focusVisible state (which is safe to access via ref anyway).
+  // We just need to re-render the Tooltip if the focus-visible state changes.
+  const [, setChildIsFocusVisible] = React.useState(false);
+  const handleBlur = (event) => {
+    handleBlurVisible(event);
+    if (isFocusVisibleRef.current === false) {
       setChildIsFocusVisible(false);
-      onBlurVisible();
     }
   };
 
@@ -308,7 +315,8 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
       setChildNode(event.currentTarget);
     }
 
-    if (isFocusVisible(event)) {
+    handleFocusVisible(event);
+    if (isFocusVisibleRef.current === true) {
       setChildIsFocusVisible(true);
       handleEnter()(event);
     }
@@ -343,7 +351,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
       if (childrenProps.onBlur && forward) {
         childrenProps.onBlur(event);
       }
-      handleBlur();
+      handleBlur(event);
     }
 
     if (

diff --git a/packages/material-ui/src/utils/useIsFocusVisible.d.ts b/packages/material-ui/src/utils/useIsFocusVisible.d.ts
@@ -1,5 +1,8 @@
+import * as React from 'react';
+
 export default function useIsFocusVisible(): {
-  isFocusVisible: (event: React.ChangeEvent) => boolean;
-  onBlurVisible: () => void;
+  isFocusVisibleRef: React.MutableRefObject<boolean>;
+  onBlur: (event: React.FocusEvent<any>) => void;
+  onFocus: (event: React.FocusEvent<any>) => void;
   ref: React.Ref<unknown>;
 };
diff --git a/packages/material-ui/src/utils/useIsFocusVisible.js b/packages/material-ui/src/utils/useIsFocusVisible.js
@@ -115,32 +115,53 @@ function isFocusVisible(event) {
   return hadKeyboardEvent || focusTriggersKeyboardModality(target);
 }
 
-/**
- * Should be called if a blur event is fired on a focus-visible element
- */
-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
-  // regular focus change.
-  hadFocusVisibleRecently = true;
-  window.clearTimeout(hadFocusVisibleRecentlyTimeout);
-  hadFocusVisibleRecentlyTimeout = window.setTimeout(() => {
-    hadFocusVisibleRecently = false;
-  }, 100);
-}
-
 export default function useIsFocusVisible() {
   const ref = React.useCallback((node) => {
     if (node != null) {
       prepare(node.ownerDocument);
     }
   }, []);
 
-  if (process.env.NODE_ENV !== 'production') {
-    // eslint-disable-next-line react-hooks/rules-of-hooks
-    React.useDebugValue(isFocusVisible);
+  const isFocusVisibleRef = React.useRef(false);
+
+  /**
+   * Should be called if a blur event is fired
+   */
+  function handleBlurVisible() {
+    // checking against potential state variable does not suffice if we focus and blur syncronously.
+    // React wouldn't have time to trigger a re-render so `focusVisible` would be stale.
+    // Ideally we would adjust `isFocusVisible(event)` to look at `relatedTarget` for blur events.
+    // This doesn't work in IE 11 due to https://github.com/facebook/react/issues/3751
+    // TODO: check again if React releases their internal changes to focus event handling (https://github.com/facebook/react/pull/19186).
+    if (isFocusVisibleRef.current) {
+      // 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
+      // regular focus change.
+      hadFocusVisibleRecently = true;
+      window.clearTimeout(hadFocusVisibleRecentlyTimeout);
+      hadFocusVisibleRecentlyTimeout = window.setTimeout(() => {
+        hadFocusVisibleRecently = false;
+      }, 100);
+
+      isFocusVisibleRef.current = false;
+
+      return true;
+    }
+
+    return false;
+  }
+
+  /**
+   * Should be called if a blur event is fired
+   */
+  function handleFocusVisible(event) {
+    if (isFocusVisible(event)) {
+      isFocusVisibleRef.current = true;
+      return true;
+    }
+    return false;
   }
 
-  return { isFocusVisible, onBlurVisible: handleBlurVisible, ref };
+  return { isFocusVisibleRef, onFocus: handleFocusVisible, onBlur: handleBlurVisible, ref };
 }
diff --git a/...aterial-ui/src/utils/focusVisible.test.js → ...al-ui/src/utils/useIsFocusVisible.test.js b/...aterial-ui/src/utils/focusVisible.test.js → ...al-ui/src/utils/useIsFocusVisible.test.js
@@ -17,21 +17,27 @@ function simulatePointerDevice() {
 }
 
 const SimpleButton = React.forwardRef(function SimpleButton(props, ref) {
-  const { isFocusVisible, onBlurVisible, ref: focusVisibleRef } = useIsFocusVisible();
+  const {
+    isFocusVisibleRef,
+    onBlur: handleBlurVisible,
+    onFocus: handleFocusVisible,
+    ref: focusVisibleRef,
+  } = useIsFocusVisible();
 
   const handleRef = useForkRef(focusVisibleRef, ref);
 
   const [focusVisible, setFocusVisible] = React.useState(false);
 
-  const handleBlur = () => {
-    if (focusVisible) {
+  const handleBlur = (event) => {
+    handleBlurVisible(event);
+    if (isFocusVisibleRef.current === false) {
       setFocusVisible(false);
-      onBlurVisible();
     }
   };
 
   const handleFocus = (event) => {
-    if (isFocusVisible(event)) {
+    handleFocusVisible(event);
+    if (isFocusVisibleRef.current === true) {
       setFocusVisible(true);
     }
   };