From b0a5b4f5bbc7da562aebf02809f9c2018b2f63f8 Mon Sep 17 00:00:00 2001 From: daledah Date: Mon, 30 Sep 2024 15:25:26 +0700 Subject: [PATCH 1/9] fix: tooltip reappears --- .../Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx index ef5327feba31..103ea832e9ac 100644 --- a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx +++ b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx @@ -59,9 +59,12 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldAutoDismiss = fa return; } // When tooltip is used inside an animated view (e.g. popover), we need to wait for the animation to finish before measuring content. - setTimeout(() => { + const timeout = setTimeout(() => { show.current?.(); }, 500); + return () => { + clearTimeout(timeout); + }; }, [shouldMeasure, shouldShow]); return ( From 95e26544e20ea400113ffd2d3e0902674604af06 Mon Sep 17 00:00:00 2001 From: daledah Date: Tue, 1 Oct 2024 16:42:40 +0700 Subject: [PATCH 2/9] fix: disable tooltip permanently after first show --- .../BaseEducationalTooltip.tsx | 30 +++++++++++++------ .../Tooltip/EducationalTooltip/index.tsx | 6 +--- src/pages/Search/SearchTypeMenu.tsx | 2 +- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx index 103ea832e9ac..2d93908dc302 100644 --- a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx +++ b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx @@ -1,4 +1,4 @@ -import React, {memo, useEffect, useRef, useState} from 'react'; +import React, {memo, useCallback, useEffect, useRef, useState} from 'react'; import type {LayoutRectangle, NativeSyntheticEvent} from 'react-native'; import {useOnyx} from 'react-native-onyx'; import GenericTooltip from '@components/Tooltip/GenericTooltip'; @@ -12,14 +12,24 @@ type LayoutChangeEventWithTarget = NativeSyntheticEvent<{layout: LayoutRectangle * A component used to wrap an element intended for displaying a tooltip. * This tooltip would show immediately without user's interaction and hide after 5 seconds. */ -function BaseEducationalTooltip({children, onHideTooltip, shouldAutoDismiss = false, ...props}: EducationalTooltipProps) { +function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, shouldAutoDismiss = false, ...props}: EducationalTooltipProps) { const hideTooltipRef = useRef<() => void>(); const [shouldMeasure, setShouldMeasure] = useState(false); const show = useRef<() => void>(); const [modal] = useOnyx(ONYXKEYS.MODAL); - const shouldShow = !modal?.willAlertModalBecomeVisible && !modal?.isVisible; + const shouldShow = !modal?.willAlertModalBecomeVisible && !modal?.isVisible && shouldRender; + + const didShow = useRef(false); + + const hideTooltip = useCallback(() => { + if (!didShow.current) { + return; + } + hideTooltipRef.current?.(); + onHideTooltip?.(); + }, []); useEffect( () => () => { @@ -34,39 +44,41 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldAutoDismiss = fa // Automatically hide tooltip after 5 seconds useEffect(() => { - if (!hideTooltipRef.current || !shouldAutoDismiss) { + if (!shouldAutoDismiss) { return; } // If the modal is open, hide the tooltip immediately and clear the timeout if (!shouldShow) { - hideTooltipRef.current(); + hideTooltip(); return; } // Automatically hide tooltip after 5 seconds if shouldAutoDismiss is true const timerID = setTimeout(() => { - hideTooltipRef.current?.(); - onHideTooltip?.(); + hideTooltip(); }, 5000); return () => { clearTimeout(timerID); }; - }, [shouldAutoDismiss, shouldShow, onHideTooltip]); + }, [shouldAutoDismiss, shouldShow, hideTooltip]); useEffect(() => { - if (!shouldMeasure || !shouldShow) { + if (!shouldMeasure || !shouldShow || didShow.current) { return; } // When tooltip is used inside an animated view (e.g. popover), we need to wait for the animation to finish before measuring content. const timeout = setTimeout(() => { show.current?.(); + didShow.current = true; }, 500); return () => { clearTimeout(timeout); }; }, [shouldMeasure, shouldShow]); + useEffect(() => hideTooltip, []); + return ( SearchActions.dismissSavedSearchRenameTooltip(), + onHideTooltip: SearchActions.dismissSavedSearchRenameTooltip, renderTooltipContent: () => { return ( From 85345dfc3b1a716d5929f69c590aee690c4983bf Mon Sep 17 00:00:00 2001 From: daledah Date: Tue, 1 Oct 2024 16:48:30 +0700 Subject: [PATCH 3/9] fix: lint --- .../BaseEducationalTooltip.tsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx index 2d93908dc302..b7b0cfaf952b 100644 --- a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx +++ b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx @@ -23,13 +23,13 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, const didShow = useRef(false); - const hideTooltip = useCallback(() => { + const hide = useCallback(() => { if (!didShow.current) { return; } hideTooltipRef.current?.(); onHideTooltip?.(); - }, []); + }, [onHideTooltip]); useEffect( () => () => { @@ -50,18 +50,18 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, // If the modal is open, hide the tooltip immediately and clear the timeout if (!shouldShow) { - hideTooltip(); + hide(); return; } // Automatically hide tooltip after 5 seconds if shouldAutoDismiss is true const timerID = setTimeout(() => { - hideTooltip(); + hide(); }, 5000); return () => { clearTimeout(timerID); }; - }, [shouldAutoDismiss, shouldShow, hideTooltip]); + }, [shouldAutoDismiss, shouldShow, hide]); useEffect(() => { if (!shouldMeasure || !shouldShow || didShow.current) { @@ -77,7 +77,11 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, }; }, [shouldMeasure, shouldShow]); - useEffect(() => hideTooltip, []); + useEffect( + () => hide, + // eslint-disable-next-line react-hooks/exhaustive-deps + [], + ); return ( Date: Tue, 1 Oct 2024 16:52:00 +0700 Subject: [PATCH 4/9] fix: compiler --- .../Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx index b7b0cfaf952b..c98b9eadc17d 100644 --- a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx +++ b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx @@ -79,7 +79,7 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, useEffect( () => hide, - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-compiler/react-compiler react-hooks/exhaustive-deps [], ); From af0e30c3ff737ab15e6a17d10c30f8d8571c8ef1 Mon Sep 17 00:00:00 2001 From: daledah Date: Tue, 1 Oct 2024 16:54:53 +0700 Subject: [PATCH 5/9] fix: typo --- .../Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx index c98b9eadc17d..dbf623ffb94a 100644 --- a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx +++ b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx @@ -79,7 +79,7 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, useEffect( () => hide, - // eslint-disable-next-line react-compiler/react-compiler react-hooks/exhaustive-deps + // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps [], ); From 48aed5cdf8d7dffb2b13a853747129626c69d29b Mon Sep 17 00:00:00 2001 From: daledah Date: Wed, 2 Oct 2024 10:55:15 +0700 Subject: [PATCH 6/9] fix: performance test --- src/components/LHNOptionsList/OptionRowLHN.tsx | 2 +- .../home/report/ReportActionCompose/ReportActionCompose.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/LHNOptionsList/OptionRowLHN.tsx b/src/components/LHNOptionsList/OptionRowLHN.tsx index 322f28aa246d..dc71c6614c67 100644 --- a/src/components/LHNOptionsList/OptionRowLHN.tsx +++ b/src/components/LHNOptionsList/OptionRowLHN.tsx @@ -187,7 +187,7 @@ function OptionRowLHN({reportID, isFocused = false, onSelectRow = () => {}, opti shiftHorizontal={variables.gbrTooltipShiftHorizontal} shiftVertical={variables.composerTooltipShiftVertical} wrapperStyle={styles.quickActionTooltipWrapper} - onHideTooltip={() => User.dismissGBRTooltip()} + onHideTooltip={User.dismissGBRTooltip} > diff --git a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx index e4b0eef6ca52..48278e6631b9 100644 --- a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx +++ b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx @@ -407,7 +407,7 @@ function ReportActionCompose({ shouldRender={!shouldHideEducationalTooltip && shouldShowEducationalTooltip} renderTooltipContent={renderWorkspaceChatTooltip} shouldUseOverlay - onHideTooltip={() => User.dismissWorkspaceTooltip()} + onHideTooltip={User.dismissWorkspaceTooltip} anchorAlignment={{ horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT, vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM, From 4e16296a258895f0ed8b01902fe25cbebf60864b Mon Sep 17 00:00:00 2001 From: daledah Date: Wed, 2 Oct 2024 15:01:08 +0700 Subject: [PATCH 7/9] fix: naming --- .../EducationalTooltip/BaseEducationalTooltip.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx index dbf623ffb94a..4fb166ef293d 100644 --- a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx +++ b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx @@ -23,7 +23,7 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, const didShow = useRef(false); - const hide = useCallback(() => { + const closeTooltip = useCallback(() => { if (!didShow.current) { return; } @@ -50,35 +50,35 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, // If the modal is open, hide the tooltip immediately and clear the timeout if (!shouldShow) { - hide(); + closeTooltip(); return; } // Automatically hide tooltip after 5 seconds if shouldAutoDismiss is true const timerID = setTimeout(() => { - hide(); + closeTooltip(); }, 5000); return () => { clearTimeout(timerID); }; - }, [shouldAutoDismiss, shouldShow, hide]); + }, [shouldAutoDismiss, shouldShow, closeTooltip]); useEffect(() => { if (!shouldMeasure || !shouldShow || didShow.current) { return; } // When tooltip is used inside an animated view (e.g. popover), we need to wait for the animation to finish before measuring content. - const timeout = setTimeout(() => { + const timerID = setTimeout(() => { show.current?.(); didShow.current = true; }, 500); return () => { - clearTimeout(timeout); + clearTimeout(timerID); }; }, [shouldMeasure, shouldShow]); useEffect( - () => hide, + () => closeTooltip, // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps [], ); From c52ab1221137d6442ef5e2c0a1664d14f8eafa7e Mon Sep 17 00:00:00 2001 From: daledah Date: Tue, 8 Oct 2024 23:03:03 +0700 Subject: [PATCH 8/9] fix: prevent rerender --- .../BaseEducationalTooltip.tsx | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx index 4fb166ef293d..92e1b8f0bb92 100644 --- a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx +++ b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx @@ -1,9 +1,10 @@ import React, {memo, useCallback, useEffect, useRef, useState} from 'react'; import type {LayoutRectangle, NativeSyntheticEvent} from 'react-native'; -import {useOnyx} from 'react-native-onyx'; import GenericTooltip from '@components/Tooltip/GenericTooltip'; import type {EducationalTooltipProps} from '@components/Tooltip/types'; +import onyxSubscribe from '@libs/onyxSubscribe'; import ONYXKEYS from '@src/ONYXKEYS'; +import {Modal} from '@src/types/onyx'; import measureTooltipCoordinate from './measureTooltipCoordinate'; type LayoutChangeEventWithTarget = NativeSyntheticEvent<{layout: LayoutRectangle; target: HTMLElement}>; @@ -17,10 +18,29 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, const [shouldMeasure, setShouldMeasure] = useState(false); const show = useRef<() => void>(); - const [modal] = useOnyx(ONYXKEYS.MODAL); + const [modal, setModal] = useState({ + willAlertModalBecomeVisible: false, + isVisible: false, + }); const shouldShow = !modal?.willAlertModalBecomeVisible && !modal?.isVisible && shouldRender; + useEffect(() => { + if (!shouldRender) return; + const unsubscribeOnyxModal = onyxSubscribe({ + key: ONYXKEYS.MODAL, + callback: (modalArg) => { + if (modalArg === undefined) { + return; + } + setModal(modalArg); + }, + }); + return () => { + unsubscribeOnyxModal(); + }; + }, [shouldRender]); + const didShow = useRef(false); const closeTooltip = useCallback(() => { From 5cd60866cf3bb8895c9eb67d69b7b2855e0ee17f Mon Sep 17 00:00:00 2001 From: daledah Date: Tue, 8 Oct 2024 23:07:16 +0700 Subject: [PATCH 9/9] fix: lint --- .../Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx index 92e1b8f0bb92..a9481f3cf3b3 100644 --- a/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx +++ b/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx @@ -4,7 +4,7 @@ import GenericTooltip from '@components/Tooltip/GenericTooltip'; import type {EducationalTooltipProps} from '@components/Tooltip/types'; import onyxSubscribe from '@libs/onyxSubscribe'; import ONYXKEYS from '@src/ONYXKEYS'; -import {Modal} from '@src/types/onyx'; +import type {Modal} from '@src/types/onyx'; import measureTooltipCoordinate from './measureTooltipCoordinate'; type LayoutChangeEventWithTarget = NativeSyntheticEvent<{layout: LayoutRectangle; target: HTMLElement}>; @@ -26,7 +26,9 @@ function BaseEducationalTooltip({children, onHideTooltip, shouldRender = false, const shouldShow = !modal?.willAlertModalBecomeVisible && !modal?.isVisible && shouldRender; useEffect(() => { - if (!shouldRender) return; + if (!shouldRender) { + return; + } const unsubscribeOnyxModal = onyxSubscribe({ key: ONYXKEYS.MODAL, callback: (modalArg) => {