You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[HOLD for payment 2025-02-04] [$250] Web - Tooltip - Educational tooltip appears on not here page #55058
Version Number: 9.0.83-0 Reproducible in staging?: Yes Reproducible in production?: Yes If this was caught during regression testing, add the test name, ID and link from TestRail: Exp Email or phone of affected tester (no customers):joaojjasjaojsi@gmail.com Issue reported by: Applause Internal Team Device used: Mac 15.0 / Chrome App Component: Other
Action Performed:
Go to staging.new.expensify.com
Log in with a new account.
Complete the onboarding.
Open FAB to dismiss the tooltip.
Create a new workspace.
Go back to LHN.
Do not open workspace chat because we need to keep the tooltip untouched.
When we open the validate login page, the nav stack looks like: [BottomTab, ValidateLogin]
useIsCurrentRouteHome gets the state using useNavigationState which gets the state of the current navigator, which is the BottomTabNavigator, so it always return true even though the validate login page covers the whole screen.
What changes do you think we should make in order to solve the problem?
There is no hook to listen for root navigator, so I'm thinking to have the same approach as this.
Edited by proposal-police: This proposal was edited at 2025-01-10 14:21:47 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Tooltip - Educational tooltip appears on not here page
What is the root cause of that problem?
The page showing ‘Not Found’ in this ticket includes the ValidateLogin page and OptionRowLHN inside BottomTabNavigator. When the full-screen ‘Not Found’ view of ValidateLogin is displayed, the condition to trigger the educational tooltip is still executed and returns true, causing the tooltip to appear.
With this issue, any page with the route name ValidateLogin will cause this problem to occur, such as ‘Not Found’ for invalid magic links, ExpiredValidateCodeModal, or JustSignedInModal...
What changes do you think we should make in order to solve the problem?
To resolve this issue, we should prevent the educational tooltip from being displayed when the topmost route name is ValidateLogin. This can be implemented as follows:
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
This is a UI bug, so unit tests are not needed
What alternative solutions did you explore? (Optional)
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
Edited by proposal-police: This proposal was edited at 2025-01-12 05:17:08 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Web - Tooltip - Educational tooltip appears on not here page
What is the root cause of that problem?
We are displaying the tooltip without verifying if the target element is within the viewable area of the page. This results in the tooltip being shown even when the user cannot see the target content.
What changes do you think we should make in order to solve the problem?
We should only display the tooltip if the target element is visible within the viewport (i.e., the user can see the content). To implement this, we can leverage the IntersectionObserver API here to detect the visibility of the target element. Here’s the implementation or we can use similar this library:
<GenericTooltipshouldForceAnimateshouldRender={shouldRender}intersectionObserverEntry={intersectionObserverEntry}// eslint-disable-next-line react/jsx-props-no-spreading{...props}>{({ showTooltip, hideTooltip, updateTargetBounds })=>{// eslint-disable-next-line react-compiler/react-compilerhideTooltipRef.current=hideTooltip;returnReact.cloneElement(childrenasReact.ReactElement,{ref: elementRef,onLayout: (e: LayoutChangeEventWithTarget)=>{if(!shouldMeasure){setShouldMeasure(true);}// e.target is specific to native, use e.nativeEvent.target on web insteadconsttarget=e.target||e.nativeEvent.target;show.current=()=>measureTooltipCoordinate(target,updateTargetBounds,showTooltip);},});}}</GenericTooltip>
And in the BaseGenericTooltip, we need to check the IntersectionObserverEntry to ensure that the content is visible, as shown in the example
constisElementVisible=useMemo(()=>{consttargetElement=intersectionObserverEntry?.targetasHTMLElement;letisElementVisible=falseif(intersectionObserverEntry&&intersectionObserverEntry.isIntersecting){constrect=intersectionObserverEntry.intersectionRectconstelementAtPoint=document.elementFromPoint(rect.x+targetWidth/2,rect.y);if(elementAtPoint&&'contains'intargetElement&&'contains'intargetElement){isElementVisible=targetElement.contains(elementAtPoint)||elementAtPoint.contains(targetElement);//TODO: Add additional conditions to handle cases where the element should be hidden.}}returnisElementVisible},[anchorAlignment,intersectionObserverEntry,targetWidth])
And pass isElementVisible to getTooltipStyles to evaluate the condition.
I believe this implementation will resolve the root issue where the tooltip is displayed, but the content is not actually visible. For the detailed code, we can discuss it later in PR.
POC
Before
After
Screen.Recording.2025-01-12.at.11.15.49.AM.mov
Screen.Recording.2025-01-12.at.11.15.08.AM.mov
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
No, UI bug
What alternative solutions did you explore? (Optional)
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.89-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
@Ollyws@twisterdotcom@Ollyws The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]
[Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.
Link to discussion:
[Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
Regression Test Proposal Template
[BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.
Link to issue:
Regression Test Proposal
Precondition:
Test:
1. Login as a new user and complete the onboarding
2. Create a new workspace
3. Go back to LHN
4. Verify there is a tooltip on the workspace chat (if there is other tooltip showing, dismiss it first)
5. Go to /v/1/1 or a not found page (/asd)
6. Verify there are no tooltip shows
7. Go back
8. Verify the tooltip shows
Activity
melvin-bot commentedon Jan 10, 2025
Triggered auto assignment to @RachCHopkins (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.bernhardoj commentedon Jan 10, 2025
Proposal
Please re-state the problem that we are trying to solve in this issue.
The educational tooltip shows on the validate login page.
What is the root cause of that problem?
The tooltip shows if the current active route is home.
App/src/components/LHNOptionsList/OptionRowLHN.tsx
Line 57 in 219e66d
App/src/components/LHNOptionsList/OptionRowLHN.tsx
Line 62 in 219e66d
When we open the validate login page, the nav stack looks like: [BottomTab, ValidateLogin]
useIsCurrentRouteHomegets the state using useNavigationState which gets the state of the current navigator, which is the BottomTabNavigator, so it always return true even though the validate login page covers the whole screen.What changes do you think we should make in order to solve the problem?
There is no hook to listen for root navigator, so I'm thinking to have the same approach as this.
App/src/libs/Navigation/AppNavigator/AuthScreens.tsx
Lines 209 to 220 in 219e66d
We will create a new Onyx key called
FULLSCREEN_VISIBILITYand set it to true when the validate login page is focused and to false when remove.Then attach the listener here (and other screen if needed)
App/src/libs/Navigation/AppNavigator/AuthScreens.tsx
Lines 428 to 436 in 219e66d
Next, we listen for the onyx in OptionRowLHN.
Then, update the visibility logic here to prevent the tooltip from showing when
isFullscreenVisibleis true.App/src/components/LHNOptionsList/OptionRowLHN.tsx
Lines 59 to 66 in 219e66d
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
N/A
huult commentedon Jan 10, 2025
Edited by proposal-police: This proposal was edited at 2025-01-10 14:21:47 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Tooltip - Educational tooltip appears on not here page
What is the root cause of that problem?
The page showing ‘Not Found’ in this ticket includes the
ValidateLoginpage andOptionRowLHNinsideBottomTabNavigator. When the full-screen ‘Not Found’ view ofValidateLoginis displayed, the condition to trigger the educational tooltip is still executed and returnstrue, causing the tooltip to appear.App/src/components/LHNOptionsList/OptionRowLHN.tsx
Line 62 in 270a302
With this issue, any page with the route name
ValidateLoginwill cause this problem to occur, such as ‘Not Found’ for invalid magic links, ExpiredValidateCodeModal, or JustSignedInModal...What changes do you think we should make in order to solve the problem?
To resolve this issue, we should prevent the educational tooltip from being displayed when the topmost route name is ValidateLogin. This can be implemented as follows:
App/src/components/LHNOptionsList/OptionRowLHN.tsx
Line 59 in 270a302
Update to:
const rootState = navigationRef.getRootState() as State<RootStackParamList>; // add this line const topmostCentralPaneRoute = getTopmostRouteName(rootState); // add this line const {tooltipToRender, shouldShowTooltip} = useMemo(() => { const tooltip = shouldShowGetStartedTooltip ? CONST.PRODUCT_TRAINING_TOOLTIP_NAMES.CONCEIRGE_LHN_GBR : CONST.PRODUCT_TRAINING_TOOLTIP_NAMES.LHN_WORKSPACE_CHAT_TOOLTIP; const shouldShowTooltips = shouldShowWokspaceChatTooltip || shouldShowGetStartedTooltip; const shouldTooltipBeVisible = shouldUseNarrowLayout ? isScreenFocused && isActiveRouteHome : isActiveRouteHome; // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing return {tooltipToRender: tooltip, shouldShowTooltip: shouldShowTooltips && shouldTooltipBeVisible && topmostCentralPaneRoute !== SCREENS.VALIDATE_LOGIN}; // update this line }, [shouldShowGetStartedTooltip, shouldShowWokspaceChatTooltip, shouldUseNarrowLayout, isScreenFocused, isActiveRouteHome, topmostCentralPaneRoute]);What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
This is a UI bug, so unit tests are not needed
What alternative solutions did you explore? (Optional)
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
linhvovan29546 commentedon Jan 12, 2025
Edited by proposal-police: This proposal was edited at 2025-01-12 05:17:08 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Web - Tooltip - Educational tooltip appears on not here page
What is the root cause of that problem?
We are displaying the tooltip without verifying if the target element is within the viewable area of the page. This results in the tooltip being shown even when the user cannot see the target content.
What changes do you think we should make in order to solve the problem?
We should only display the tooltip if the target element is visible within the viewport (i.e., the user can see the content). To implement this, we can leverage the IntersectionObserver API here to detect the visibility of the target element. Here’s the implementation or we can use similar this library:
App/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx
Lines 58 to 61 in fb487a5
And add ref to here
And in the
BaseGenericTooltip, we need to check theIntersectionObserverEntryto ensure that the content is visible, as shown in the exampleApp/src/components/Tooltip/BaseGenericTooltip/index.tsx
Lines 69 to 71 in fb487a5
And pass
isElementVisibletogetTooltipStylesto evaluate the condition.App/src/styles/utils/generators/TooltipStyleUtils/index.ts
Line 124 in fb487a5
I believe this implementation will resolve the root issue where the tooltip is displayed, but the content is not actually visible. For the detailed code, we can discuss it later in PR.
POC
Screen.Recording.2025-01-12.at.11.15.49.AM.mov
Screen.Recording.2025-01-12.at.11.15.08.AM.mov
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
No, UI bug
What alternative solutions did you explore? (Optional)
We can use my proposal here
melvin-bot commentedon Jan 12, 2025
Triggered auto assignment to @twisterdotcom (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.27 remaining items
melvin-bot commentedon Jan 28, 2025
Reviewinglabel has been removed, please complete the "BugZero Checklist".melvin-bot commentedon Jan 28, 2025
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.89-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2025-02-04. 🎊
For reference, here are some details about the assignees on this issue:
melvin-bot commentedon Jan 28, 2025
@Ollyws @twisterdotcom @Ollyws The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]
melvin-bot commentedon Feb 4, 2025
Payment Summary
Upwork Job
BugZero Checklist (@twisterdotcom)
twisterdotcom commentedon Feb 4, 2025
Bump on the checklist here please @Ollyws.
twisterdotcom commentedon Feb 4, 2025
Payment Summary:
bernhardoj commentedon Feb 5, 2025
Requested in ND.
Ollyws commentedon Feb 5, 2025
BugZero Checklist:
Bug classification
Source of bug:
Where bug was reported:
Who reported the bug:
[Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.
Link to comment: 270a302#r152162983
[Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.
Link to discussion:
[Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
[BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.
Link to issue:
Regression Test Proposal
Precondition:
Test:
Do we agree 👍 or 👎
Ollyws commentedon Feb 5, 2025
Requested in ND.
JmillsExpensify commentedon Feb 10, 2025
$250 approved for @bernhardoj
garrettmknight commentedon Feb 11, 2025
$250 approved for @Ollyws