From 179812cc7df7426a4dcfd5d743e08d8552217582 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Tue, 10 Oct 2023 00:50:06 +0100 Subject: [PATCH 1/7] display tooltip over links --- .../Hoverable/hoverablePropTypes.js | 8 +- src/components/Hoverable/index.js | 15 +- src/components/Tooltip/index.js | 254 ++++++++++++++++-- 3 files changed, 246 insertions(+), 31 deletions(-) diff --git a/src/components/Hoverable/hoverablePropTypes.js b/src/components/Hoverable/hoverablePropTypes.js index d483a06d6aaf..f0c52a4d1646 100644 --- a/src/components/Hoverable/hoverablePropTypes.js +++ b/src/components/Hoverable/hoverablePropTypes.js @@ -13,6 +13,12 @@ const propTypes = { /** Function that executes when the mouse leaves the children. */ onHoverOut: PropTypes.func, + /** Direct pass-through of React's onMouseEnter event. */ + onMouseEnter: PropTypes.func, + + /** Direct pass-through of React's onMouseLeave event. */ + onMouseLeave: PropTypes.func, + /** Decides whether to handle the scroll behaviour to show hover once the scroll ends */ shouldHandleScroll: PropTypes.bool, }; @@ -24,4 +30,4 @@ const defaultProps = { shouldHandleScroll: false, }; -export {propTypes, defaultProps}; +export {propTypes, defaultProps}; \ No newline at end of file diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 38ea64952a2c..2401315800d8 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -46,6 +46,7 @@ class Hoverable extends Component { * If the user has started scrolling and the isHoveredRef is true, then we should set the hover state to false. * This is to hide the existing hover and reaction bar. */ + this.isHoveredRef = false; this.setState({isHovered: false}, this.props.onHoverOut); } this.isScrollingRef = scrolling; @@ -90,9 +91,7 @@ class Hoverable extends Component { /** * If the isScrollingRef is true, then the user is scrolling and we should not update the hover state. */ - if (this.isScrollingRef && this.props.shouldHandleScroll && !this.state.isHovered) { - return; - } + if (this.isScrollingRef && this.props.shouldHandleScroll && !this.state.isHovered) return; if (isHovered !== this.state.isHovered) { this.setState({isHovered}, isHovered ? this.props.onHoverIn : this.props.onHoverOut); @@ -155,6 +154,10 @@ class Hoverable extends Component { } }, onMouseEnter: (el) => { + if (_.isFunction(this.props.onMouseEnter)) { + this.props.onMouseEnter(el); + } + this.setIsHovered(true); if (_.isFunction(child.props.onMouseEnter)) { @@ -162,6 +165,10 @@ class Hoverable extends Component { } }, onMouseLeave: (el) => { + if (_.isFunction(this.props.onMouseLeave)) { + this.props.onMouseLeave(el); + } + this.setIsHovered(false); if (_.isFunction(child.props.onMouseLeave)) { @@ -186,4 +193,4 @@ class Hoverable extends Component { Hoverable.propTypes = propTypes; Hoverable.defaultProps = defaultProps; -export default Hoverable; +export default Hoverable; \ No newline at end of file diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 2e6789ec73f6..0083a52d6ba7 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -1,37 +1,239 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import {propTypes as tooltipPropTypes, defaultProps as tooltipDefaultProps} from './tooltipPropTypes'; -import BaseTooltip from './BaseTooltip'; +import _ from 'underscore'; +import React, {memo, useCallback, useEffect, useRef, useState} from 'react'; +import {Animated} from 'react-native'; +import {BoundsObserver} from '@react-ng/bounds-observer'; +import TooltipRenderedOnPageBody from './TooltipRenderedOnPageBody'; +import Hoverable from '../Hoverable'; +import * as tooltipPropTypes from './tooltipPropTypes'; +import TooltipSense from './TooltipSense'; +import * as DeviceCapabilities from '../../libs/DeviceCapabilities'; +import usePrevious from '../../hooks/usePrevious'; +import useLocalize from '../../hooks/useLocalize'; +import useWindowDimensions from '../../hooks/useWindowDimensions'; -const propTypes = { - ...tooltipPropTypes, +const hasHoverSupport = DeviceCapabilities.hasHoverSupport(); - /** Whether the actual Tooltip should be rendered. If false, it's just going to return the children */ - shouldRender: PropTypes.bool, -}; +/** + * Choose the correct bounding box from the given list. + * This is a helper function for chooseBoundingBox below. + * + * @param {Element} bbs The bounding boxes of DOM element being hovered over. + * @param {number} clientX The X position from the MouseEvent. + * @param {number} clientY The Y position from the MouseEvent. + * @param {number} slop An allowed slop factor when searching for the bounding + * box. If the user is moving the mouse quickly we can end up getting a + * hover event with the position outside any of our bounding boxes. We retry + * with a small slop factor in that case, so if we have a bounding box close + * enough then we go with that. + * @return {DOMRect | null} The chosen bounding box. null if we failed to find + * a matching one, which can happen if the user is moving the mouse quickly + * and the onHoverOver event actually fires outside the element bounding box. + */ +function chooseBoundingBoxWithSlop(bbs, clientX, clientY, slop) { + for (let i = 0; i < bbs.length; i++) { + const bb = bbs[i]; + if (bb.x - slop <= clientX && bb.x + bb.width + slop >= clientX && bb.y - slop <= clientY && bb.y + bb.height + slop >= clientY) { + return bb; + } + } + return null; +} + +/** + * Choose the correct bounding box for the tooltip to be positioned against. + * This handles the case where the target is wrapped across two lines, and + * so we need to find the correct part (the one that the user is hovering + * over) and show the tooltip there. + * + * @param {Element} target The DOM element being hovered over. + * @param {number} clientX The X position from the MouseEvent. + * @param {number} clientY The Y position from the MouseEvent. + * @return {DOMRect} The chosen bounding box. + */ +function chooseBoundingBox(target, clientX, clientY) { + const bbs = target.getClientRects(); + if (bbs.length === 1) { + return bbs[0]; + } + let bb = chooseBoundingBoxWithSlop(bbs, clientX, clientY, 0); + if (bb) { + return bb; + } + // Retry with a slop factor, in case the user is moving the mouse quickly. + bb = chooseBoundingBoxWithSlop(bbs, clientX, clientY, 5); + if (bb) { + return bb; + } + // Fall back to the full bounding box if we failed to find a matching one. + // This could only happen if the user is moving the mouse very quickly + // and they got it outside our slop above. + return target.getBoundingClientRect(); +} + +/** + * A component used to wrap an element intended for displaying a tooltip. The term "tooltip's target" refers to the + * wrapped element, which, upon hover, triggers the tooltip to be shown. + * @param {propTypes} props + * @returns {ReactNodeLike} + */ +function Tooltip(props) { + const {children, numberOfLines, maxWidth, text, renderTooltipContent, renderTooltipContentKey} = props; + + const {preferredLocale} = useLocalize(); + const {windowWidth} = useWindowDimensions(); + + // Is tooltip already rendered on the page's body? happens once. + const [isRendered, setIsRendered] = useState(false); + // Is the tooltip currently visible? + const [isVisible, setIsVisible] = useState(false); + // The distance between the left side of the wrapper view and the left side of the window + const [xOffset, setXOffset] = useState(0); + // The distance between the top of the wrapper view and the top of the window + const [yOffset, setYOffset] = useState(0); + // The width and height of the wrapper view + const [wrapperWidth, setWrapperWidth] = useState(0); + const [wrapperHeight, setWrapperHeight] = useState(0); + + // Whether the tooltip is first tooltip to activate the TooltipSense + const isTooltipSenseInitiator = useRef(false); + const animation = useRef(new Animated.Value(0)); + const isAnimationCanceled = useRef(false); + const prevText = usePrevious(text); + + const target = useRef(null); + const initialMousePosition = useRef({x: 0, y: 0}); -const defaultProps = { - ...tooltipDefaultProps, - shouldRender: true, -}; + const updateTargetAndMousePosition = useCallback((e) => { + target.current = e.target; + initialMousePosition.current = {x: e.clientX, y: e.clientY}; + }, []); -function Tooltip({shouldRender, children, ...props}) { - if (!shouldRender) { + /** + * Display the tooltip in an animation. + */ + const showTooltip = useCallback(() => { + if (!isRendered) { + setIsRendered(true); + } + + setIsVisible(true); + + animation.current.stopAnimation(); + + // When TooltipSense is active, immediately show the tooltip + if (TooltipSense.isActive()) { + animation.current.setValue(1); + } else { + isTooltipSenseInitiator.current = true; + Animated.timing(animation.current, { + toValue: 1, + duration: 140, + delay: 500, + useNativeDriver: false, + }).start(({finished}) => { + isAnimationCanceled.current = !finished; + }); + } + TooltipSense.activate(); + }, [isRendered]); + + // eslint-disable-next-line rulesdir/prefer-early-return + useEffect(() => { + // if the tooltip text changed before the initial animation was finished, then the tooltip won't be shown + // we need to show the tooltip again + if (isVisible && isAnimationCanceled.current && text && prevText !== text) { + isAnimationCanceled.current = false; + showTooltip(); + } + }, [isVisible, text, prevText, showTooltip]); + + /** + * Update the tooltip bounding rectangle + * + * @param {Object} bounds - updated bounds + */ + const updateBounds = (bounds) => { + if (bounds.width === 0) { + setIsRendered(false); + } + // Choose a bounding box for the tooltip to target. + // In the case when the target is a link that has wrapped onto + // multiple lines, we want to show the tooltip over the part + // of the link that the user is hovering over. + const betterBounds = chooseBoundingBox(target.current, initialMousePosition.current.x, initialMousePosition.current.y); + setWrapperWidth(betterBounds.width); + setWrapperHeight(betterBounds.height); + setXOffset(betterBounds.x); + setYOffset(betterBounds.y); + }; + + /** + * Hide the tooltip in an animation. + */ + const hideTooltip = () => { + animation.current.stopAnimation(); + + if (TooltipSense.isActive() && !isTooltipSenseInitiator.current) { + animation.current.setValue(0); + } else { + // Hide the first tooltip which initiated the TooltipSense with animation + isTooltipSenseInitiator.current = false; + Animated.timing(animation.current, { + toValue: 0, + duration: 140, + useNativeDriver: false, + }).start(); + } + + TooltipSense.deactivate(); + + setIsVisible(false); + }; + + // Skip the tooltip and return the children if the text is empty, + // we don't have a render function or the device does not support hovering + if ((_.isEmpty(text) && renderTooltipContent == null) || !hasHoverSupport) { return children; } return ( - - {children} - + <> + {isRendered && ( + + )} + + + {children} + + + ); } -Tooltip.displayName = 'Tooltip'; -Tooltip.propTypes = propTypes; -Tooltip.defaultProps = defaultProps; - -export default Tooltip; +Tooltip.propTypes = tooltipPropTypes.propTypes; +Tooltip.defaultProps = tooltipPropTypes.defaultProps; +export default memo(Tooltip); \ No newline at end of file From f724dac52d830b2875fd2d7ac54f8d968ab45582 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Tue, 10 Oct 2023 01:12:58 +0100 Subject: [PATCH 2/7] fix lint --- src/components/Hoverable/hoverablePropTypes.js | 2 +- src/components/Hoverable/index.js | 6 ++++-- src/components/Tooltip/index.js | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/Hoverable/hoverablePropTypes.js b/src/components/Hoverable/hoverablePropTypes.js index f0c52a4d1646..a3aeaa597d7a 100644 --- a/src/components/Hoverable/hoverablePropTypes.js +++ b/src/components/Hoverable/hoverablePropTypes.js @@ -30,4 +30,4 @@ const defaultProps = { shouldHandleScroll: false, }; -export {propTypes, defaultProps}; \ No newline at end of file +export {propTypes, defaultProps}; diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 2401315800d8..9d7330da3684 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -91,7 +91,9 @@ class Hoverable extends Component { /** * If the isScrollingRef is true, then the user is scrolling and we should not update the hover state. */ - if (this.isScrollingRef && this.props.shouldHandleScroll && !this.state.isHovered) return; + if (this.isScrollingRef && this.props.shouldHandleScroll && !this.state.isHovered) { + return; + } if (isHovered !== this.state.isHovered) { this.setState({isHovered}, isHovered ? this.props.onHoverIn : this.props.onHoverOut); @@ -193,4 +195,4 @@ class Hoverable extends Component { Hoverable.propTypes = propTypes; Hoverable.defaultProps = defaultProps; -export default Hoverable; \ No newline at end of file +export default Hoverable; diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 0083a52d6ba7..7137b4c6a065 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -236,4 +236,4 @@ function Tooltip(props) { Tooltip.propTypes = tooltipPropTypes.propTypes; Tooltip.defaultProps = tooltipPropTypes.defaultProps; -export default memo(Tooltip); \ No newline at end of file +export default memo(Tooltip); From be5988658c958b9989174fc4065959669cc86a93 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Wed, 11 Oct 2023 09:40:49 +0100 Subject: [PATCH 3/7] move bounding box logic to base tooltip --- src/components/Hoverable/index.js | 5 - src/components/Tooltip/BaseTooltip.js | 83 ++++++++- src/components/Tooltip/index.js | 254 +++----------------------- 3 files changed, 105 insertions(+), 237 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 9d7330da3684..281c1e7860f7 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -42,11 +42,6 @@ class Hoverable extends Component { if (!scrolling && this.isHoveredRef) { this.setState({isHovered: this.isHoveredRef}, this.props.onHoverIn); } else if (scrolling && this.isHoveredRef) { - /** - * If the user has started scrolling and the isHoveredRef is true, then we should set the hover state to false. - * This is to hide the existing hover and reaction bar. - */ - this.isHoveredRef = false; this.setState({isHovered: false}, this.props.onHoverOut); } this.isScrollingRef = scrolling; diff --git a/src/components/Tooltip/BaseTooltip.js b/src/components/Tooltip/BaseTooltip.js index f60982f52dd4..8478cd333691 100644 --- a/src/components/Tooltip/BaseTooltip.js +++ b/src/components/Tooltip/BaseTooltip.js @@ -19,6 +19,65 @@ const hasHoverSupport = DeviceCapabilities.hasHoverSupport(); * @param {propTypes} props * @returns {ReactNodeLike} */ + +/** + * Choose the correct bounding box from the given list. + * This is a helper function for chooseBoundingBox below. + * + * @param {Element} bbs The bounding boxes of DOM element being hovered over. + * @param {number} clientX The X position from the MouseEvent. + * @param {number} clientY The Y position from the MouseEvent. + * @param {number} slop An allowed slop factor when searching for the bounding + * box. If the user is moving the mouse quickly we can end up getting a + * hover event with the position outside any of our bounding boxes. We retry + * with a small slop factor in that case, so if we have a bounding box close + * enough then we go with that. + * @return {DOMRect | null} The chosen bounding box. null if we failed to find + * a matching one, which can happen if the user is moving the mouse quickly + * and the onHoverOver event actually fires outside the element bounding box. + */ +function chooseBoundingBoxWithSlop(bbs, clientX, clientY, slop) { + for (let i = 0; i < bbs.length; i++) { + const bb = bbs[i]; + if (bb.x - slop <= clientX && bb.x + bb.width + slop >= clientX && bb.y - slop <= clientY && bb.y + bb.height + slop >= clientY) { + return bb; + } + } + return null; +} + +/** + * Choose the correct bounding box for the tooltip to be positioned against. + * This handles the case where the target is wrapped across two lines, and + * so we need to find the correct part (the one that the user is hovering + * over) and show the tooltip there. + * + * @param {Element} target The DOM element being hovered over. + * @param {number} clientX The X position from the MouseEvent. + * @param {number} clientY The Y position from the MouseEvent. + * @return {DOMRect} The chosen bounding box. + */ + +function chooseBoundingBox(target, clientX, clientY) { + const bbs = target.getClientRects(); + if (bbs.length === 1) { + return bbs[0]; + } + let bb = chooseBoundingBoxWithSlop(bbs, clientX, clientY, 0); + if (bb) { + return bb; + } + // Retry with a slop factor, in case the user is moving the mouse quickly. + bb = chooseBoundingBoxWithSlop(bbs, clientX, clientY, 5); + if (bb) { + return bb; + } + // Fall back to the full bounding box if we failed to find a matching one. + // This could only happen if the user is moving the mouse very quickly + // and they got it outside our slop above. + return target.getBoundingClientRect(); +} + function Tooltip(props) { const {children, numberOfLines, maxWidth, text, renderTooltipContent, renderTooltipContentKey} = props; @@ -43,6 +102,14 @@ function Tooltip(props) { const isAnimationCanceled = useRef(false); const prevText = usePrevious(text); + const target = useRef(null); + const initialMousePosition = useRef({x: 0, y: 0}); + + const updateTargetAndMousePosition = useCallback((e) => { + target.current = e.target; + initialMousePosition.current = {x: e.clientX, y: e.clientY}; + }, []); + /** * Display the tooltip in an animation. */ @@ -91,10 +158,15 @@ function Tooltip(props) { if (bounds.width === 0) { setIsRendered(false); } - setWrapperWidth(bounds.width); - setWrapperHeight(bounds.height); - setXOffset(bounds.x); - setYOffset(bounds.y); + // Choose a bounding box for the tooltip to target. + // In the case when the target is a link that has wrapped onto + // multiple lines, we want to show the tooltip over the part + // of the link that the user is hovering over. + const betterBounds = chooseBoundingBox(target.current, initialMousePosition.current.x, initialMousePosition.current.y); + setWrapperWidth(betterBounds.width); + setWrapperHeight(betterBounds.height); + setXOffset(betterBounds.x); + setYOffset(betterBounds.y); }; /** @@ -152,6 +224,7 @@ function Tooltip(props) { onBoundsChange={updateBounds} > = clientX && bb.y - slop <= clientY && bb.y + bb.height + slop >= clientY) { - return bb; - } - } - return null; -} - -/** - * Choose the correct bounding box for the tooltip to be positioned against. - * This handles the case where the target is wrapped across two lines, and - * so we need to find the correct part (the one that the user is hovering - * over) and show the tooltip there. - * - * @param {Element} target The DOM element being hovered over. - * @param {number} clientX The X position from the MouseEvent. - * @param {number} clientY The Y position from the MouseEvent. - * @return {DOMRect} The chosen bounding box. - */ -function chooseBoundingBox(target, clientX, clientY) { - const bbs = target.getClientRects(); - if (bbs.length === 1) { - return bbs[0]; - } - let bb = chooseBoundingBoxWithSlop(bbs, clientX, clientY, 0); - if (bb) { - return bb; - } - // Retry with a slop factor, in case the user is moving the mouse quickly. - bb = chooseBoundingBoxWithSlop(bbs, clientX, clientY, 5); - if (bb) { - return bb; - } - // Fall back to the full bounding box if we failed to find a matching one. - // This could only happen if the user is moving the mouse very quickly - // and they got it outside our slop above. - return target.getBoundingClientRect(); -} - -/** - * A component used to wrap an element intended for displaying a tooltip. The term "tooltip's target" refers to the - * wrapped element, which, upon hover, triggers the tooltip to be shown. - * @param {propTypes} props - * @returns {ReactNodeLike} - */ -function Tooltip(props) { - const {children, numberOfLines, maxWidth, text, renderTooltipContent, renderTooltipContentKey} = props; - - const {preferredLocale} = useLocalize(); - const {windowWidth} = useWindowDimensions(); - - // Is tooltip already rendered on the page's body? happens once. - const [isRendered, setIsRendered] = useState(false); - // Is the tooltip currently visible? - const [isVisible, setIsVisible] = useState(false); - // The distance between the left side of the wrapper view and the left side of the window - const [xOffset, setXOffset] = useState(0); - // The distance between the top of the wrapper view and the top of the window - const [yOffset, setYOffset] = useState(0); - // The width and height of the wrapper view - const [wrapperWidth, setWrapperWidth] = useState(0); - const [wrapperHeight, setWrapperHeight] = useState(0); - - // Whether the tooltip is first tooltip to activate the TooltipSense - const isTooltipSenseInitiator = useRef(false); - const animation = useRef(new Animated.Value(0)); - const isAnimationCanceled = useRef(false); - const prevText = usePrevious(text); - - const target = useRef(null); - const initialMousePosition = useRef({x: 0, y: 0}); + /** Whether the actual Tooltip should be rendered. If false, it's just going to return the children */ + shouldRender: PropTypes.bool, +}; - const updateTargetAndMousePosition = useCallback((e) => { - target.current = e.target; - initialMousePosition.current = {x: e.clientX, y: e.clientY}; - }, []); +const defaultProps = { + ...tooltipDefaultProps, + shouldRender: true, +}; - /** - * Display the tooltip in an animation. - */ - const showTooltip = useCallback(() => { - if (!isRendered) { - setIsRendered(true); - } - - setIsVisible(true); - - animation.current.stopAnimation(); - - // When TooltipSense is active, immediately show the tooltip - if (TooltipSense.isActive()) { - animation.current.setValue(1); - } else { - isTooltipSenseInitiator.current = true; - Animated.timing(animation.current, { - toValue: 1, - duration: 140, - delay: 500, - useNativeDriver: false, - }).start(({finished}) => { - isAnimationCanceled.current = !finished; - }); - } - TooltipSense.activate(); - }, [isRendered]); - - // eslint-disable-next-line rulesdir/prefer-early-return - useEffect(() => { - // if the tooltip text changed before the initial animation was finished, then the tooltip won't be shown - // we need to show the tooltip again - if (isVisible && isAnimationCanceled.current && text && prevText !== text) { - isAnimationCanceled.current = false; - showTooltip(); - } - }, [isVisible, text, prevText, showTooltip]); - - /** - * Update the tooltip bounding rectangle - * - * @param {Object} bounds - updated bounds - */ - const updateBounds = (bounds) => { - if (bounds.width === 0) { - setIsRendered(false); - } - // Choose a bounding box for the tooltip to target. - // In the case when the target is a link that has wrapped onto - // multiple lines, we want to show the tooltip over the part - // of the link that the user is hovering over. - const betterBounds = chooseBoundingBox(target.current, initialMousePosition.current.x, initialMousePosition.current.y); - setWrapperWidth(betterBounds.width); - setWrapperHeight(betterBounds.height); - setXOffset(betterBounds.x); - setYOffset(betterBounds.y); - }; - - /** - * Hide the tooltip in an animation. - */ - const hideTooltip = () => { - animation.current.stopAnimation(); - - if (TooltipSense.isActive() && !isTooltipSenseInitiator.current) { - animation.current.setValue(0); - } else { - // Hide the first tooltip which initiated the TooltipSense with animation - isTooltipSenseInitiator.current = false; - Animated.timing(animation.current, { - toValue: 0, - duration: 140, - useNativeDriver: false, - }).start(); - } - - TooltipSense.deactivate(); - - setIsVisible(false); - }; - - // Skip the tooltip and return the children if the text is empty, - // we don't have a render function or the device does not support hovering - if ((_.isEmpty(text) && renderTooltipContent == null) || !hasHoverSupport) { +function Tooltip({shouldRender, children, ...props}) { + if (!shouldRender) { return children; } return ( - <> - {isRendered && ( - - )} - - - {children} - - - + + {children} + ); } -Tooltip.propTypes = tooltipPropTypes.propTypes; -Tooltip.defaultProps = tooltipPropTypes.defaultProps; -export default memo(Tooltip); +Tooltip.displayName = 'Tooltip'; +Tooltip.propTypes = propTypes; +Tooltip.defaultProps = defaultProps; + +export default Tooltip; From 0ad6afdbf3e2e3b60fa7136404e899913e44e60a Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Wed, 11 Oct 2023 09:45:34 +0100 Subject: [PATCH 4/7] destructure from props --- src/components/Tooltip/BaseTooltip.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Tooltip/BaseTooltip.js b/src/components/Tooltip/BaseTooltip.js index 8478cd333691..27a57dfb467e 100644 --- a/src/components/Tooltip/BaseTooltip.js +++ b/src/components/Tooltip/BaseTooltip.js @@ -79,7 +79,7 @@ function chooseBoundingBox(target, clientX, clientY) { } function Tooltip(props) { - const {children, numberOfLines, maxWidth, text, renderTooltipContent, renderTooltipContentKey} = props; + const {children, numberOfLines, maxWidth, text, renderTooltipContent, renderTooltipContentKey, shouldHandleScroll} = props; const {preferredLocale} = useLocalize(); const {windowWidth} = useWindowDimensions(); @@ -227,7 +227,7 @@ function Tooltip(props) { onMouseEnter={updateTargetAndMousePosition} onHoverIn={showTooltip} onHoverOut={hideTooltip} - shouldHandleScroll={props.shouldHandleScroll} + shouldHandleScroll={shouldHandleScroll} > {children} From eb8530a728cb34459cc03839f21e4bb26cb7b219 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Wed, 11 Oct 2023 10:43:37 +0100 Subject: [PATCH 5/7] merge and improve functions with a tolerance of 5 --- src/components/Tooltip/BaseTooltip.js | 57 ++++++++------------------- 1 file changed, 16 insertions(+), 41 deletions(-) diff --git a/src/components/Tooltip/BaseTooltip.js b/src/components/Tooltip/BaseTooltip.js index 27a57dfb467e..b8bca189c534 100644 --- a/src/components/Tooltip/BaseTooltip.js +++ b/src/components/Tooltip/BaseTooltip.js @@ -20,32 +20,6 @@ const hasHoverSupport = DeviceCapabilities.hasHoverSupport(); * @returns {ReactNodeLike} */ -/** - * Choose the correct bounding box from the given list. - * This is a helper function for chooseBoundingBox below. - * - * @param {Element} bbs The bounding boxes of DOM element being hovered over. - * @param {number} clientX The X position from the MouseEvent. - * @param {number} clientY The Y position from the MouseEvent. - * @param {number} slop An allowed slop factor when searching for the bounding - * box. If the user is moving the mouse quickly we can end up getting a - * hover event with the position outside any of our bounding boxes. We retry - * with a small slop factor in that case, so if we have a bounding box close - * enough then we go with that. - * @return {DOMRect | null} The chosen bounding box. null if we failed to find - * a matching one, which can happen if the user is moving the mouse quickly - * and the onHoverOver event actually fires outside the element bounding box. - */ -function chooseBoundingBoxWithSlop(bbs, clientX, clientY, slop) { - for (let i = 0; i < bbs.length; i++) { - const bb = bbs[i]; - if (bb.x - slop <= clientX && bb.x + bb.width + slop >= clientX && bb.y - slop <= clientY && bb.y + bb.height + slop >= clientY) { - return bb; - } - } - return null; -} - /** * Choose the correct bounding box for the tooltip to be positioned against. * This handles the case where the target is wrapped across two lines, and @@ -59,23 +33,24 @@ function chooseBoundingBoxWithSlop(bbs, clientX, clientY, slop) { */ function chooseBoundingBox(target, clientX, clientY) { + const slop = 5; const bbs = target.getClientRects(); - if (bbs.length === 1) { - return bbs[0]; - } - let bb = chooseBoundingBoxWithSlop(bbs, clientX, clientY, 0); - if (bb) { - return bb; - } - // Retry with a slop factor, in case the user is moving the mouse quickly. - bb = chooseBoundingBoxWithSlop(bbs, clientX, clientY, 5); - if (bb) { - return bb; + const clientXMin = clientX - slop; + const clientXMax = clientX + slop; + const clientYMin = clientY - slop; + const clientYMax = clientY + slop; + + for (let i = 0; i < bbs.length; i++) { + const bb = bbs[i]; + if (clientXMin <= bb.right && clientXMax >= bb.left && clientYMin <= bb.bottom && clientYMax >= bb.top) { + return bb; + } } - // Fall back to the full bounding box if we failed to find a matching one. - // This could only happen if the user is moving the mouse very quickly - // and they got it outside our slop above. - return target.getBoundingClientRect(); + + // If no matching bounding box is found, fall back to the first one. + // This could only happen if the user is moving the mouse very quickly + // and they got it outside our slop above. + return bbs[0]; } function Tooltip(props) { From 1b90bb2f4c2abc717d2f735c500357f8778756c1 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Wed, 11 Oct 2023 10:55:06 +0100 Subject: [PATCH 6/7] fix lint --- src/components/Tooltip/BaseTooltip.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Tooltip/BaseTooltip.js b/src/components/Tooltip/BaseTooltip.js index b8bca189c534..bcbb14686a2e 100644 --- a/src/components/Tooltip/BaseTooltip.js +++ b/src/components/Tooltip/BaseTooltip.js @@ -46,10 +46,10 @@ function chooseBoundingBox(target, clientX, clientY) { return bb; } } - + // If no matching bounding box is found, fall back to the first one. - // This could only happen if the user is moving the mouse very quickly - // and they got it outside our slop above. + // This could only happen if the user is moving the mouse very quickly + // and they got it outside our slop above. return bbs[0]; } From d6f8379d107409f3a25c5d02a4d23f3a83e58637 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Wed, 11 Oct 2023 23:52:58 +0100 Subject: [PATCH 7/7] revert comment and use function destructure --- src/components/Hoverable/index.js | 4 ++++ src/components/Tooltip/BaseTooltip.js | 9 ++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index 281c1e7860f7..5cba52db5a7b 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -42,6 +42,10 @@ class Hoverable extends Component { if (!scrolling && this.isHoveredRef) { this.setState({isHovered: this.isHoveredRef}, this.props.onHoverIn); } else if (scrolling && this.isHoveredRef) { + /** + * If the user has started scrolling and the isHoveredRef is true, then we should set the hover state to false. + * This is to hide the existing hover and reaction bar. + */ this.setState({isHovered: false}, this.props.onHoverOut); } this.isScrollingRef = scrolling; diff --git a/src/components/Tooltip/BaseTooltip.js b/src/components/Tooltip/BaseTooltip.js index bcbb14686a2e..f8d1cf87fc42 100644 --- a/src/components/Tooltip/BaseTooltip.js +++ b/src/components/Tooltip/BaseTooltip.js @@ -2,6 +2,7 @@ import _ from 'underscore'; import React, {memo, useCallback, useEffect, useRef, useState} from 'react'; import {Animated} from 'react-native'; import {BoundsObserver} from '@react-ng/bounds-observer'; +import Str from 'expensify-common/lib/str'; import TooltipRenderedOnPageBody from './TooltipRenderedOnPageBody'; import Hoverable from '../Hoverable'; import * as tooltipPropTypes from './tooltipPropTypes'; @@ -53,9 +54,7 @@ function chooseBoundingBox(target, clientX, clientY) { return bbs[0]; } -function Tooltip(props) { - const {children, numberOfLines, maxWidth, text, renderTooltipContent, renderTooltipContentKey, shouldHandleScroll} = props; - +function Tooltip({children, numberOfLines, maxWidth, text, renderTooltipContent, renderTooltipContentKey, shouldHandleScroll, shiftHorizontal, shiftVertical}) { const {preferredLocale} = useLocalize(); const {windowWidth} = useWindowDimensions(); @@ -183,8 +182,8 @@ function Tooltip(props) { yOffset={yOffset} targetWidth={wrapperWidth} targetHeight={wrapperHeight} - shiftHorizontal={_.result(props, 'shiftHorizontal')} - shiftVertical={_.result(props, 'shiftVertical')} + shiftHorizontal={Str.result(shiftHorizontal)} + shiftVertical={Str.result(shiftVertical)} text={text} maxWidth={maxWidth} numberOfLines={numberOfLines}