[HOLD for payment 2025-01-15] [$500] Make it possible to click the element the product training tooltip is pointing to.

[ishpaul777]

Part of the Migrate Existing Users to NewDot project

Main issue: https://github.com/Expensify/Expensify/issues/437980
Doc section: https://docs.google.com/document/d/1m8e1ASwG70t651qSO6OfsSvW18RFrcWkO897iUllDLs/edit?tab=t.0#heading=h.yh89nmhpipyt
Project:

Feature Description

Make it possible to click the element the product training tooltip is pointing to.

Ideal Expected Behaviour:

1. you should be able to interact with anything on the screen. The presence of a tooltip shouldn't change what you can do
2. you should be able to click the element the tooltip is pointing to
3. the tooltip should remain until you click that element. So if you click something else that's fine, we allow you. But the tooltip isn't dismissed as a result

Manual tests scenario:

1. Tooltip pointing to Concierge chat in LHN shows.
2. Click on conceirge chat, tooltip should disappear.
3. if user clicks on any other chat than Concierge, tooltip should disappear but when user goes back to LHN Tooltip should appear again.

Note: None of the tooltips should be autodismissed after few seconds

We have 4 tooltips at this time see list here and other 4 will be added in #54064 this should be the expected behavior for all tooltips

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021867697197444537249
• Upwork Job ID: 1867697197444537249
• Last Price Increase: 2024-12-27
• Automatic offers:
• ishpaul777 | Reviewer | 105520989
• rayane-djouah | Contributor | 105520990

Issue Owner

Current Issue Owner: @twisterdotcom

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021867697197444537249

[melvin-bot]

Current assignee @ishpaul777 is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to @twisterdotcom (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[melvin-bot]

Upwork job price has been updated to $500

[puneetlath]

Updating the price since this is part of an important migrate initiative.

[storm21-tecn]

The core issue here is that operations that should happen outside of the render cycle are being executed at the wrong time and in the wrong way. Functions like useSharedValue and useState, which manage state, should be properly synchronized using hooks like useEffect. This ensures that side effects are handled outside of the render cycle, preventing unnecessary re-renders and ensuring smoother performance.

Using useEffect: Values should not be updated directly during the render cycle. Instead, hooks like useEffect should be used to update values after the component is initially rendered or after specific dependencies change. This ensures that updates are done in a controlled manner and helps avoid unnecessary renders.

    height.set(-reanimated.height.get());
    heightWhenOpened.set(-reanimated.height.get());
    progress.set(reanimated.progress.get());
    isClosed.set(reanimated.progress.get() === 0);
}, []);

This approach ensures that values are updated only after the component has mounted, preventing unnecessary re-renders.

Synchronized Value Management: Shared values with useSharedValue should only be updated outside the render cycle—specifically in hooks like useEffect or useAnimatedStyle. This ensures that React processes updates correctly and optimizes performance.

Conditional Checks: The shouldShow function should only be called under the appropriate conditions. Screen size, user interactions, or other conditions should be checked only when necessary to avoid redundant checks.

[melvin-bot]

📣 @storm21-tecn! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[melvin-bot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[rayane-d]

Edited by proposal-police: This proposal was edited at the latest timestamp recorded during the edit.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Make it possible to click the element the product training tooltip is pointing to.

What is the root cause of that problem?

New feature

What changes do you think we should make in order to solve the problem?

• Removed all auto-dismiss behavior (no more hiding tooltips after a few seconds).
• Removed various onHideTooltip props and related logic throughout the code.
• Removed shouldUseOverlay from several tooltip usages so clicking the other elements is possible.
• Ensured clicking the tooltip’s target element (e.g., FAB, chats, filter button) hides the tooltip as expected.
• Added calls to hideProductTrainingTooltip() when clicking the element the tooltip is pointing to.
• Standardized hiding tooltips by calling hideProductTrainingTooltip()
• Prevented tooltips from rendering if a modal is open (ensures tooltips don’t overlap modals).
• Removed the TooltipManager file and all references to it.
• Updated the useProductTrainingContext hook to handle tooltip registration and unregistration without TooltipManager.
• Switched from useIsFocused to useFocusEffect in some components for better focus handling.
• Refactored places where tooltips are hidden on button presses or when navigating away.

Test branch:

main...rayane-djouah:z:Make-it-possible-to-click-the-element-the-product-training-tooltip-is-pointing-to-pr

Previous solution

1. Captures the user’s click coordinates on the transparent overlay.
2. Compares these coordinates against the position and dimensions of the target element that the tooltip references.
3. If the click falls within the target element’s bounding box, the tooltip is hidden, and a callback (onChildrenElementPress) is invoked to trigget the underlying element onPress function, and a callback onHideTooltip is triggered to ensure that the tooltip is not displayed again
4. If the click is outside the target element, the tooltip is simply hidden (onDismissTooltip callback) without triggering the underlying element’s action. The onHideTooltip is not called in this case to make sure that the tooltip will appear again.

    const handleOverlayClick = (event?: MouseEvent | KeyboardEvent) => {

        // Captures the user’s click coordinates on the transparent overlay.
        const clickX = event?.clientX;
        const clickY = event?.clientY;

        // Check if the click falls within the target element bounds
        const isWithinTarget = !!clickX && !!clickY && clickX >= xOffset && clickX <= xOffset + targetWidth && clickY >= yOffset && clickY <= yOffset + targetHeight;

        // Hide the tooltip
        onDismissTooltip();

        // If the click is within target bounds, trigger the children element's press callback
        requestAnimationFrame(() => {
            if (!isWithinTarget) {
                return;
            }
            onHideTooltip();
            onChildrenElementPress?.();
        });
    };

On native:

    const handleOverlayClick = (event?: GestureEvent) => {

        // Captures the user’s click coordinates on the transparent overlay.
        const pageX = event?.nativeEvent.pageX;
        const pageY = event?.nativeEvent.pageY;

        // Compares these coordinates against the position and dimensions of the target element that the tooltip references.
        const isWithinTarget = !!pageX && !!pageY && pageX >= xOffset && pageX <= xOffset + targetWidth && pageY >= yOffset && pageY <= yOffset + targetHeight;

        onDismissTooltip();();

        requestAnimationFrame(() => {
            if (!isWithinTarget) {
                return;
            }
            // the tooltip should not be displayed again
            onHideTooltip();
            onChildrenElementPress?.();
        });
    };

7. a "hole" overlay area with the same dimensions of the target element’s bounding box is introduced in TransparentOverlay component to display a hand cursor within the target element

    const holeStyle = useMemo(
        () => ({
            position: styles.pFixed,
            top: holeY,
            left: holeX,
            width: holeWidth,
            height: holeHeight,
            zIndex: 1500,
        }),
        [holeX, holeY, holeWidth, holeHeight, styles.pFixed],
    );
    return (
        <View
            onPointerDown={handlePointerDown}
            style={styles.fullScreen}
        >
            <PressableWithoutFeedback
                onPress={onPress}
                style={[styles.flex1, styles.cursorDefault, overlay]}
                accessibilityLabel={translate('common.close')}
                role={CONST.ROLE.BUTTON}
            />
            >
               {/* "Hole" area with a hand cursor */}
                <PressableWithoutFeedback
                    onPress={onPress}
                    style={holeStyle}
                    role={CONST.ROLE.BUTTON}
                />
            </PressableWithoutFeedback>
        </View>
    );

8. we pass the dimensions of the hole as follows:

{shouldUseOverlay && (
    <TransparentOverlay
        onPress={handleOverlayClick}
        holeX={xOffset}
        holeY={yOffset}
        holeWidth={targetWidth}
        holeHeight={targetHeight}
    />
)}

9. we need to remove auto dismissing tooltip logic here

Test branch:

main...rayane-djouah:z:Make-it-possible-to-click-the-product-training-tooltip-element

Result:

Screen.Recording.2024-12-14.at.4.56.17.PM.mov

Screen.Recording.2024-12-14.at.4.53.36.PM.mov

What alternative solutions did you explore? (Optional)

On web, we can dispatch the mouse press event to the underlaying element, but we can't do this on native, so onChildrenElementPress callback is a better approach

// If the click is within the target element, re-dispatch the event to the element
const targetElement = document.elementFromPoint(clickX, clickY) as HTMLElement | null;
if (targetElement) {
    // Create a new MouseEvent and dispatch it to simulate a click
    const simulatedClick = new MouseEvent('click', {bubbles: true, cancelable: true, clientX: clickX, clientY: clickY});
    targetElement.dispatchEvent(simulatedClick);
}

[rayane-d]

Updated the proposal with more details

[ishpaul777]

NOT OVERDUE DISCUSSING HERE https://expensify.slack.com/archives/C02NK2DQWUX/p1733780305744839

[melvin-bot]

@puneetlath, @twisterdotcom, @ishpaul777 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[iwiznia]

This indeed caused a regreession: #54841

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.81-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Feat: Make it possible to click the element the product training tooltip is pointing to #54710

If no regressions arise, payment will be issued on 2025-01-15. 🎊

For reference, here are some details about the assignees on this issue:

• @rayane-djouah requires payment through NewDot Manual Requests (Contributor)
• @ishpaul777 requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@rayane-djouah / @ishpaul777] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[twisterdotcom]

Okay, sorry for missing this folks. The regression messages certainly caused a bit of confusion. @puneetlath / @iwiznia are we worried about the regressions here enough to reduce payment? Or, now that everything is resolved, can we go ahead with paying out the existing offers?

[iwiznia]

I thought all regressions reduced payments, if that's not the case, then I am not sure what the logic is for reducing payment.

[rayane-d]

Note: My eligibility for NewDot payments began on December 28th, and I was assigned to this GitHub issue on December 31st

[rayane-d]

• [@rayane-djouah / @ishpaul777] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.

Regression Test Proposal

- Sign in with a new account and complete the onboarding flow.

### For all product training tooltips (Concierge chat, FAB, QAB, Workspace chat, Filters button, Inbox Icon, Saved search three dots, "+" button tooltip in invited workspace chat):
1. You should be able to interact with any element on the screen. The presence of a tooltip should not restrict interactions.
2. You should be able to click the element the tooltip is pointing to.
3. The tooltip should remain visible until you click the element it is pointing to. Interacting with other elements should not dismiss the tooltip except for modals.
4. Verify that when a modal is opened, the tooltip is hidden, and when the modal is closed, the tooltip reappears.
5. Verify that after clicking the element the tooltip is pointing to, the tooltip is dismissed and does not show again.

Do we agree 👍 or 👎

[twisterdotcom]

This seems fine to me: #54068 (comment)

[twisterdotcom]

Payment Summary:

• @rayane-djouah paid $500 via NewDot Manual Request
• @ishpaul777 paid $500 here (Reviewer)

[garrettmknight]

$500 approved for @rayane-djouah