From 96cf606f3a73f6719e53fb6f44310125e45771ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 12 Sep 2023 10:45:08 +0200 Subject: [PATCH 01/17] rewrite Hoverable class component into functional --- src/components/Hoverable/index.js | 222 ++++++++++++------------------ 1 file changed, 88 insertions(+), 134 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 0b560703a069..0ee228299361 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -1,147 +1,101 @@ import _ from 'underscore'; -import React, {Component} from 'react'; +import React, {useEffect, useCallback, useState, useRef, useMemo} from 'react'; import {propTypes, defaultProps} from './hoverablePropTypes'; import * as DeviceCapabilities from '../../libs/DeviceCapabilities'; +function mapChildren(children, callbackParam) { + if (_.isArray(children) && children.length === 1) return children[0]; + + if (_.isFunction(children)) return children(callbackParam); + + return children; +} + /** * It is necessary to create a Hoverable component instead of relying solely on Pressable support for hover state, * because nesting Pressables causes issues where the hovered state of the child cannot be easily propagated to the * parent. https://github.com/necolas/react-native-web/issues/1875 */ -class Hoverable extends Component { - constructor(props) { - super(props); - - this.handleVisibilityChange = this.handleVisibilityChange.bind(this); - this.checkHover = this.checkHover.bind(this); - - this.state = { - isHovered: false, - }; - - this.wrapperView = null; - } - - componentDidMount() { - document.addEventListener('visibilitychange', this.handleVisibilityChange); - document.addEventListener('mouseover', this.checkHover); - } - - componentDidUpdate(prevProps) { - if (prevProps.disabled === this.props.disabled) { - return; - } - - if (this.props.disabled && this.state.isHovered) { - this.setState({isHovered: false}); - } - } - - componentWillUnmount() { - document.removeEventListener('visibilitychange', this.handleVisibilityChange); - document.removeEventListener('mouseover', this.checkHover); - } - - /** - * Sets the hover state of this component to true and execute the onHoverIn callback. - * - * @param {Boolean} isHovered - Whether or not this component is hovered. - */ - setIsHovered(isHovered) { - if (this.props.disabled) { - return; - } - - if (isHovered !== this.state.isHovered) { - this.setState({isHovered}, isHovered ? this.props.onHoverIn : this.props.onHoverOut); - } - } - - /** - * Checks the hover state of a component and updates it based on the event target. - * This is necessary to handle cases where the hover state might get stuck due to an unreliable mouseleave trigger, - * such as when an element is removed before the mouseleave event is triggered. - * @param {Event} e - The hover event object. - */ - checkHover(e) { - if (!this.wrapperView || !this.state.isHovered) { - return; - } - - if (this.wrapperView.contains(e.target)) { - return; - } - - this.setIsHovered(false); - } - - handleVisibilityChange() { - if (document.visibilityState !== 'hidden') { - return; - } - - this.setIsHovered(false); - } - - render() { - let child = this.props.children; - if (_.isArray(this.props.children) && this.props.children.length === 1) { - child = this.props.children[0]; - } - - if (_.isFunction(child)) { - child = child(this.state.isHovered); - } - - if (!DeviceCapabilities.hasHoverSupport()) { - return child; - } - - return React.cloneElement(React.Children.only(child), { - ref: (el) => { - this.wrapperView = el; - - // Call the original ref, if any - const {ref} = child; - if (_.isFunction(ref)) { - ref(el); - return; - } - - if (_.isObject(ref)) { - ref.current = el; - } - }, - onMouseEnter: (el) => { - this.setIsHovered(true); - - if (_.isFunction(child.props.onMouseEnter)) { - child.props.onMouseEnter(el); - } - }, - onMouseLeave: (el) => { - this.setIsHovered(false); - - if (_.isFunction(child.props.onMouseLeave)) { - child.props.onMouseLeave(el); - } - }, - onBlur: (el) => { - // Check if the blur event occurred due to clicking outside the element - // and the wrapperView contains the element that caused the blur and reset isHovered - if (!this.wrapperView.contains(el.target) && !this.wrapperView.contains(el.relatedTarget)) { - this.setIsHovered(false); - } - - if (_.isFunction(child.props.onBlur)) { - child.props.onBlur(el); - } - }, - }); - } + +function Hoverable({disabled, onHoverIn, onHoverOut, children}) { + const [isHovered, setIsHovered] = useState(false); + const wrapperView = useRef(null); + + useEffect(() => { + // For now, listener is quite simple so we don't need to extract its declaration from effect + const onVisibilityChange = () => document.visibilityState === 'hidden' && setIsHovered(false); + + document.addEventListener('visibilitychange', onVisibilityChange); + + return () => document.removeEventListener('visibilitychange', onVisibilityChange); + }, []); + + useEffect(() => { + if (disabled) return setIsHovered(false); + }, [disabled]); + + useEffect(() => { + if (isHovered) return onHoverIn(); + onHoverOut(); + }, [isHovered, onHoverIn, onHoverOut]); + + const child = useMemo(() => React.Children.only(mapChildren(children, isHovered)), [children, isHovered]); + + const refCallback = useCallback( + (el) => { + if (!el) return; + + wrapperView.current = el; + + const {ref} = child; + + if (_.isFunction(ref)) return ref(el); + + if (_.isObject(ref)) return (ref.current = el); + }, + [child], + ); + + const onMouseEnter = useCallback( + (el) => { + setIsHovered(true); + + if (_.isFunction(child.props.onMouseEnter)) child.props.onMouseEnter(el); + }, + [child], + ); + + const onMouseLeave = useCallback( + (el) => { + setIsHovered(false); + + if (_.isFunction(child.props.onMouseLeave)) child.props.onMouseLeave(el); + }, + [child], + ); + + const onBlur = useCallback( + (el) => { + // Check if the blur event occurred due to clicking outside the element + // and the wrapperView contains the element that caused the blur and reset isHovered + if (!wrapperView.current.contains(el.target) && !wrapperView.current.contains(el.relatedTarget)) { + setIsHovered(false); + } + + if (_.isFunction(child.props.onBlur)) child.props.onBlur(el); + }, + [child], + ); + + if (!DeviceCapabilities.hasHoverSupport()) return child; + + return React.cloneElement(child, { + ref: refCallback, + onMouseEnter, + onMouseLeave, + onBlur, + }); } Hoverable.propTypes = propTypes; Hoverable.defaultProps = defaultProps; - -export default Hoverable; From cc13306066def12c4d7698db4bdd81ca37d4d797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 12 Sep 2023 11:46:08 +0200 Subject: [PATCH 02/17] add Hoverable missing export statement --- src/components/Hoverable/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 0ee228299361..8b5379b1ad7a 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -99,3 +99,5 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { Hoverable.propTypes = propTypes; Hoverable.defaultProps = defaultProps; + +export default Hoverable; From 7d23059f1703f92bed79a05e99d8a77d3be14027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 12 Sep 2023 12:07:11 +0200 Subject: [PATCH 03/17] do not call hover callbacks if disabled --- src/components/Hoverable/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 8b5379b1ad7a..62c3dfbb8ddb 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -35,9 +35,10 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { }, [disabled]); useEffect(() => { + if (disabled) return; if (isHovered) return onHoverIn(); onHoverOut(); - }, [isHovered, onHoverIn, onHoverOut]); + }, [disabled, isHovered, onHoverIn, onHoverOut]); const child = useMemo(() => React.Children.only(mapChildren(children, isHovered)), [children, isHovered]); From b8e2551db0cda6c9db4ba2deb793c0b114c67db1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 12 Sep 2023 12:08:32 +0200 Subject: [PATCH 04/17] narrow useCallback deps --- src/components/Hoverable/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 62c3dfbb8ddb..5882b05d9ad5 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -63,7 +63,7 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { if (_.isFunction(child.props.onMouseEnter)) child.props.onMouseEnter(el); }, - [child], + [child.props], ); const onMouseLeave = useCallback( @@ -72,7 +72,7 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { if (_.isFunction(child.props.onMouseLeave)) child.props.onMouseLeave(el); }, - [child], + [child.props], ); const onBlur = useCallback( @@ -85,7 +85,7 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { if (_.isFunction(child.props.onBlur)) child.props.onBlur(el); }, - [child], + [child.props], ); if (!DeviceCapabilities.hasHoverSupport()) return child; From 0a2a3079e1303d5030e12026646d782d7f235920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 12 Sep 2023 13:06:10 +0200 Subject: [PATCH 05/17] add displayName to Hoverable --- src/components/Hoverable/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 5882b05d9ad5..e6f3390a940d 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -100,5 +100,6 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { Hoverable.propTypes = propTypes; Hoverable.defaultProps = defaultProps; +Hoverable.displayName = 'Hoverable'; export default Hoverable; From 79586473bdab939f0828983a78ce7f4c1fec5aff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 12 Sep 2023 13:13:40 +0200 Subject: [PATCH 06/17] remove unnecessary comment --- src/components/Hoverable/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index e6f3390a940d..7504b9b78a76 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -22,7 +22,6 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { const wrapperView = useRef(null); useEffect(() => { - // For now, listener is quite simple so we don't need to extract its declaration from effect const onVisibilityChange = () => document.visibilityState === 'hidden' && setIsHovered(false); document.addEventListener('visibilitychange', onVisibilityChange); From 393aa90262d3b49883b36658ce50aa988da22cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 12 Sep 2023 14:45:08 +0200 Subject: [PATCH 07/17] add checks for hover callbacks --- src/components/Hoverable/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 7504b9b78a76..3faf801c9816 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -35,8 +35,8 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { useEffect(() => { if (disabled) return; - if (isHovered) return onHoverIn(); - onHoverOut(); + if (onHoverIn && isHovered) return onHoverIn(); + if (onHoverOut && !isHovered) return onHoverOut(); }, [disabled, isHovered, onHoverIn, onHoverOut]); const child = useMemo(() => React.Children.only(mapChildren(children, isHovered)), [children, isHovered]); From d2258a1657e2218fc1995915fc998ab11d2bbfa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 12 Sep 2023 15:23:21 +0200 Subject: [PATCH 08/17] fix forward ref issues --- src/components/Hoverable/index.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 3faf801c9816..5e2021b0e542 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -17,7 +17,7 @@ function mapChildren(children, callbackParam) { * parent. https://github.com/necolas/react-native-web/issues/1875 */ -function Hoverable({disabled, onHoverIn, onHoverOut, children}) { +function InnerHoverable({disabled, onHoverIn, onHoverOut, children}, ref) { const [isHovered, setIsHovered] = useState(false); const wrapperView = useRef(null); @@ -43,17 +43,14 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { const refCallback = useCallback( (el) => { - if (!el) return; - wrapperView.current = el; - const {ref} = child; - + if (!ref) return; if (_.isFunction(ref)) return ref(el); - - if (_.isObject(ref)) return (ref.current = el); + // eslint-disable-next-line no-param-reassign + if (_.has(ref, 'current')) return (ref.current = wrapperView.current); }, - [child], + [ref], ); const onMouseEnter = useCallback( @@ -97,6 +94,8 @@ function Hoverable({disabled, onHoverIn, onHoverOut, children}) { }); } +const Hoverable = React.forwardRef(InnerHoverable); + Hoverable.propTypes = propTypes; Hoverable.defaultProps = defaultProps; Hoverable.displayName = 'Hoverable'; From 269b58a7aa55d1c0155f48c6d4c69ae71a334f2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 12 Sep 2023 19:49:04 +0200 Subject: [PATCH 09/17] use useImperativeHandle to manage outerRef --- src/components/Hoverable/index.js | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 5e2021b0e542..9b17c4c23765 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -1,5 +1,5 @@ import _ from 'underscore'; -import React, {useEffect, useCallback, useState, useRef, useMemo} from 'react'; +import React, {useEffect, useCallback, useState, useRef, useMemo, useImperativeHandle} from 'react'; import {propTypes, defaultProps} from './hoverablePropTypes'; import * as DeviceCapabilities from '../../libs/DeviceCapabilities'; @@ -17,9 +17,9 @@ function mapChildren(children, callbackParam) { * parent. https://github.com/necolas/react-native-web/issues/1875 */ -function InnerHoverable({disabled, onHoverIn, onHoverOut, children}, ref) { +function InnerHoverable({disabled, onHoverIn, onHoverOut, children}, outerRef) { const [isHovered, setIsHovered] = useState(false); - const wrapperView = useRef(null); + const ref = useRef(null); useEffect(() => { const onVisibilityChange = () => document.visibilityState === 'hidden' && setIsHovered(false); @@ -39,19 +39,9 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children}, ref) { if (onHoverOut && !isHovered) return onHoverOut(); }, [disabled, isHovered, onHoverIn, onHoverOut]); - const child = useMemo(() => React.Children.only(mapChildren(children, isHovered)), [children, isHovered]); - - const refCallback = useCallback( - (el) => { - wrapperView.current = el; + useImperativeHandle(outerRef, () => ref.current, []); - if (!ref) return; - if (_.isFunction(ref)) return ref(el); - // eslint-disable-next-line no-param-reassign - if (_.has(ref, 'current')) return (ref.current = wrapperView.current); - }, - [ref], - ); + const child = useMemo(() => React.Children.only(mapChildren(children, isHovered)), [children, isHovered]); const onMouseEnter = useCallback( (el) => { @@ -75,7 +65,7 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children}, ref) { (el) => { // Check if the blur event occurred due to clicking outside the element // and the wrapperView contains the element that caused the blur and reset isHovered - if (!wrapperView.current.contains(el.target) && !wrapperView.current.contains(el.relatedTarget)) { + if (!ref.current.contains(el.target) && !ref.current.contains(el.relatedTarget)) { setIsHovered(false); } @@ -87,7 +77,7 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children}, ref) { if (!DeviceCapabilities.hasHoverSupport()) return child; return React.cloneElement(child, { - ref: refCallback, + ref, onMouseEnter, onMouseLeave, onBlur, From 789d927236339f6fbade1726b31330d0adea207d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Wed, 13 Sep 2023 13:34:58 +0200 Subject: [PATCH 10/17] change callback name to be more decsriptive --- src/components/Hoverable/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 9b17c4c23765..ef25886a5520 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -22,11 +22,11 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children}, outerRef) { const ref = useRef(null); useEffect(() => { - const onVisibilityChange = () => document.visibilityState === 'hidden' && setIsHovered(false); + const unsetHoveredWhenDocumentIsHidden = () => document.visibilityState === 'hidden' && setIsHovered(false); - document.addEventListener('visibilitychange', onVisibilityChange); + document.addEventListener('visibilitychange', unsetHoveredWhenDocumentIsHidden); - return () => document.removeEventListener('visibilitychange', onVisibilityChange); + return () => document.removeEventListener('visibilitychange', unsetHoveredWhenDocumentIsHidden); }, []); useEffect(() => { From b3ffe6886d8a1fd74067a52049d91deb506781de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 15 Sep 2023 19:52:44 +0200 Subject: [PATCH 11/17] update isHovered properly on disabled --- src/components/Hoverable/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index ef25886a5520..4c2ed4d527e3 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -30,8 +30,8 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children}, outerRef) { }, []); useEffect(() => { - if (disabled) return setIsHovered(false); - }, [disabled]); + if (disabled && isHovered) return setIsHovered(false); + }, [disabled, isHovered]); useEffect(() => { if (disabled) return; From b28f30b01605896cbe0e14ab0a83432dbb15d4bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 15 Sep 2023 20:04:02 +0200 Subject: [PATCH 12/17] add missing checkHover handler --- src/components/Hoverable/index.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 4c2ed4d527e3..8cba78d61091 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -29,6 +29,31 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children}, outerRef) { return () => document.removeEventListener('visibilitychange', unsetHoveredWhenDocumentIsHidden); }, []); + /** + * Checks the hover state of a component and updates it based on the event target. + * This is necessary to handle cases where the hover state might get stuck due to an unreliable mouseleave trigger, + * such as when an element is removed before the mouseleave event is triggered. + * @param {Event} e - The hover event object. + */ + const unsetHoveredIfOutside = useCallback( + (e) => { + if (!ref.current || !isHovered) return; + + if (ref.current.contains(e.target)) return; + + setIsHovered(false); + }, + [isHovered], + ); + + useEffect(() => { + if (!DeviceCapabilities.hasHoverSupport()) return; + + document.addEventListener('mouseover', unsetHoveredIfOutside); + + return () => document.removeEventListener('mouseover', unsetHoveredIfOutside); + }, [unsetHoveredIfOutside]); + useEffect(() => { if (disabled && isHovered) return setIsHovered(false); }, [disabled, isHovered]); From 3d427f877309e37e474e5a25de8d6e720db21d44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Sep 2023 16:32:04 +0200 Subject: [PATCH 13/17] obey curly eslint rule --- src/components/Hoverable/index.js | 65 +++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 6925d5d89465..13db4ee9a2d8 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -6,9 +6,13 @@ import * as DeviceCapabilities from '../../libs/DeviceCapabilities'; import CONST from '../../CONST'; function mapChildren(children, callbackParam) { - if (_.isArray(children) && children.length === 1) return children[0]; + if (_.isArray(children) && children.length === 1) { + return children[0]; + } - if (_.isFunction(children)) return children(callbackParam); + if (_.isFunction(children)) { + return children(callbackParam); + } return children; } @@ -28,11 +32,15 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle const updateIsHoveredOnScrolling = useCallback( (hovered) => { - if (disabled) return; + if (disabled) { + return; + } isHoveredRef.current = hovered; - if (!shouldHandleScroll && !isScrolling) return setIsHovered(hovered); + if (!shouldHandleScroll && !isScrolling) { + return setIsHovered(hovered); + } }, [disabled, shouldHandleScroll, isScrolling], ); @@ -46,7 +54,9 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle }, []); useEffect(() => { - if (!shouldHandleScroll) return; + if (!shouldHandleScroll) { + return; + } const scrollingListener = DeviceEventEmitter.addListener(CONST.EVENTS.SCROLLING, (scrolling) => { setIsScrolling(scrolling); @@ -63,9 +73,13 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle */ const unsetHoveredIfOutside = useCallback( (e) => { - if (!ref.current || !isHovered) return; + if (!ref.current || !isHovered) { + return; + } - if (ref.current.contains(e.target)) return; + if (ref.current.contains(e.target)) { + return; + } setIsHovered(false); }, @@ -73,7 +87,9 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle ); useEffect(() => { - if (!DeviceCapabilities.hasHoverSupport()) return; + if (!DeviceCapabilities.hasHoverSupport()) { + return; + } document.addEventListener('mouseover', unsetHoveredIfOutside); @@ -81,13 +97,22 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle }, [unsetHoveredIfOutside]); useEffect(() => { - if (disabled && isHovered) return setIsHovered(false); + if (!disabled || !isHovered) { + return; + } + setIsHovered(false); }, [disabled, isHovered]); useEffect(() => { - if (disabled) return; - if (onHoverIn && isHovered) return onHoverIn(); - if (onHoverOut && !isHovered) return onHoverOut(); + if (disabled) { + return; + } + if (onHoverIn && isHovered) { + return onHoverIn(); + } + if (onHoverOut && !isHovered) { + return onHoverOut(); + } }, [disabled, isHovered, onHoverIn, onHoverOut]); useImperativeHandle(outerRef, () => ref.current, []); @@ -98,7 +123,9 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle (el) => { updateIsHoveredOnScrolling(true); - if (_.isFunction(child.props.onMouseEnter)) child.props.onMouseEnter(el); + if (_.isFunction(child.props.onMouseEnter)) { + child.props.onMouseEnter(el); + } }, [child.props, updateIsHoveredOnScrolling], ); @@ -107,7 +134,9 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle (el) => { updateIsHoveredOnScrolling(false); - if (_.isFunction(child.props.onMouseLeave)) child.props.onMouseLeave(el); + if (_.isFunction(child.props.onMouseLeave)) { + child.props.onMouseLeave(el); + } }, [child.props, updateIsHoveredOnScrolling], ); @@ -120,12 +149,16 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle setIsHovered(false); } - if (_.isFunction(child.props.onBlur)) child.props.onBlur(el); + if (_.isFunction(child.props.onBlur)) { + child.props.onBlur(el); + } }, [child.props], ); - if (!DeviceCapabilities.hasHoverSupport()) return child; + if (!DeviceCapabilities.hasHoverSupport()) { + return child; + } return React.cloneElement(child, { ref, From 88a1bc0da0a6c7e6302531a215069c57df3bfdea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Sep 2023 17:26:02 +0200 Subject: [PATCH 14/17] update hovered on scroll end and fix conditional --- src/components/Hoverable/index.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 13db4ee9a2d8..1e3673189f28 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -38,9 +38,10 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle isHoveredRef.current = hovered; - if (!shouldHandleScroll && !isScrolling) { - return setIsHovered(hovered); + if (shouldHandleScroll && isScrolling) { + return; } + setIsHovered(hovered); }, [disabled, shouldHandleScroll, isScrolling], ); @@ -60,6 +61,9 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle const scrollingListener = DeviceEventEmitter.addListener(CONST.EVENTS.SCROLLING, (scrolling) => { setIsScrolling(scrolling); + if (!scrolling) { + setIsHovered(isHoveredRef.current); + } }); return scrollingListener.remove; From 0dbb737458dd3bae5a820c99698a5e3b9637d986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 26 Sep 2023 12:52:55 +0200 Subject: [PATCH 15/17] Fix scrollingListener cleanup --- src/components/Hoverable/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 1e3673189f28..aa28122e3e2e 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -66,7 +66,7 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle } }); - return scrollingListener.remove; + return () => scrollingListener.remove(); }, [shouldHandleScroll]); /** From d1c0a27e7a5b0dc2653a6859fe312af2eea54445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 26 Sep 2023 12:54:17 +0200 Subject: [PATCH 16/17] Replace isScrolling state with ref --- src/components/Hoverable/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index aa28122e3e2e..eff165870048 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -25,8 +25,8 @@ function mapChildren(children, callbackParam) { function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandleScroll}, outerRef) { const [isHovered, setIsHovered] = useState(false); - const [isScrolling, setIsScrolling] = useState(false); + const isScrolling = useRef(false); const isHoveredRef = useRef(false); const ref = useRef(null); @@ -38,12 +38,12 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle isHoveredRef.current = hovered; - if (shouldHandleScroll && isScrolling) { + if (shouldHandleScroll && isScrolling.current) { return; } setIsHovered(hovered); }, - [disabled, shouldHandleScroll, isScrolling], + [disabled, shouldHandleScroll], ); useEffect(() => { @@ -60,7 +60,7 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle } const scrollingListener = DeviceEventEmitter.addListener(CONST.EVENTS.SCROLLING, (scrolling) => { - setIsScrolling(scrolling); + isScrolling.current = scrolling; if (!scrolling) { setIsHovered(isHoveredRef.current); } From a9601db495e6d2904cd047ee58c6a140e30d87a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 29 Sep 2023 11:19:11 +0200 Subject: [PATCH 17/17] add useImperativeHandle explanation comment --- src/components/Hoverable/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index eff165870048..f39c44b278ae 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -119,6 +119,7 @@ function InnerHoverable({disabled, onHoverIn, onHoverOut, children, shouldHandle } }, [disabled, isHovered, onHoverIn, onHoverOut]); + // Expose inner ref to parent through outerRef. This enable us to use ref both in parent and child. useImperativeHandle(outerRef, () => ref.current, []); const child = useMemo(() => React.Children.only(mapChildren(children, isHovered)), [children, isHovered]);