From d3eb864debb13a942f1ce179a2faba6489f16b90 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sat, 8 Aug 2020 13:19:48 +0200 Subject: [PATCH] use state ref instead --- packages/material-ui-lab/src/Rating/Rating.js | 14 +++-- .../material-ui/src/ButtonBase/ButtonBase.js | 25 ++++---- packages/material-ui/src/Link/Link.js | 14 +++-- packages/material-ui/src/Slider/Slider.js | 16 +++-- packages/material-ui/src/Tooltip/Tooltip.js | 22 ++++--- .../src/utils/useIsFocusVisible.d.ts | 7 ++- .../src/utils/useIsFocusVisible.js | 59 +++++++++++++------ ...ible.test.js => useIsFocusVisible.test.js} | 16 +++-- 8 files changed, 115 insertions(+), 58 deletions(-) rename packages/material-ui/src/utils/{focusVisible.test.js => useIsFocusVisible.test.js} (89%) diff --git a/packages/material-ui-lab/src/Rating/Rating.js b/packages/material-ui-lab/src/Rating/Rating.js index e583dd00300b4e..7aaab103441edc 100644 --- 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 index 80913037ad2566..73f521645d0953 100644 --- 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 index 9dc14fc342768e..deed18c4e578a2 100644 --- a/packages/material-ui/src/Link/Link.js +++ b/packages/material-ui/src/Link/Link.js @@ -68,12 +68,17 @@ 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) { @@ -81,7 +86,8 @@ const Link = React.forwardRef(function Link(props, ref) { } }; 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 index 1abb49565d0975..65de00f31187d5 100644 --- 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 index d9e7b94b046093..0dfb45f2e6d1b4 100644 --- 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 index e1587469851064..bc64b8b201deb7 100644 --- 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; + onBlur: (event: React.FocusEvent) => void; + onFocus: (event: React.FocusEvent) => void; ref: React.Ref; }; diff --git a/packages/material-ui/src/utils/useIsFocusVisible.js b/packages/material-ui/src/utils/useIsFocusVisible.js index 7723b74da68a28..cd1e213922a9c1 100644 --- a/packages/material-ui/src/utils/useIsFocusVisible.js +++ b/packages/material-ui/src/utils/useIsFocusVisible.js @@ -115,21 +115,6 @@ 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) { @@ -137,10 +122,46 @@ export default function useIsFocusVisible() { } }, []); - 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/packages/material-ui/src/utils/focusVisible.test.js b/packages/material-ui/src/utils/useIsFocusVisible.test.js similarity index 89% rename from packages/material-ui/src/utils/focusVisible.test.js rename to packages/material-ui/src/utils/useIsFocusVisible.test.js index 289b048a65bb58..fdf6df9a0d9094 100644 --- a/packages/material-ui/src/utils/focusVisible.test.js +++ b/packages/material-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); } };