[HOLD for payment 2024-08-19] [Guided Setup Stage 3] Add the tooltip for Workspace Chat

[deetergp]

Part of the Extend Onboarding to Invited Users project

Main issue: https://github.com/Expensify/Expensify/issues/392615
Doc section: https://docs.google.com/document/d/1EX1tKSkhffpqP4TXD7iKIPKWnU5U8ELv32j-QTtXH2g/edit#heading=h.bfsayh3fuyk8
Project: #wave-collect

Feature Description

When a user is invited to a Workspace as a non-admin, when they respond to the invitation email, they should be signed directly into the app and taken to their workspace chat. When viewing the chat for the first time, they should see a tool tip hovering above the Create button that says "Get started! Submit your first expense" that gets dismissed when they click on the button.

Frontend Changes (from doc)

In order to know when to show the tooltip, look for an onyx key called workspaceTooltip with a property of shouldShow set to true. This property will only be sent on their first sign-in. Once they click on the Create button or mouse into the composer, we should dismiss the tooltip and set workspaceTooltip.shouldShow to false. It will not be sent again on subsequent sign-ins. See the backend documentation for more details.

Backend Changes (from doc)

When a user has been invited to a workspace as a non-admin, we are going to want to show them a tooltip on their first signing encouraging them to submit their first expense. In order for the frontend to know this is their first sign-in we will check that their isInviteOnboardingComplete is false and inviteType is workspace and then also include an additional key in the onyx updates from this method:

$onyxData[] = [
    'onyxMethod' => Onyx::METHOD_MERGE,
    'key' => OnyxKeys::NVP_WORKSPACE_TOOLTIP, // workspaceTooltip
    'value' => [
        'shouldShow' => true,
    ],
],

When the user clicks the Create button or click into the composer field, we will dismiss the tooltip and set workspaceTooltip.shouldShow to false. The NVP will no longer be sent on subsequent sign-ins.

Manual Test Steps

Note: These won't be manually testable till the following backend GHs are done and deployed
https://github.com/Expensify/Expensify/issues/411150
https://github.com/Expensify/Expensify/issues/411149
https://github.com/Expensify/Expensify/issues/411148
https://github.com/Expensify/Expensify/issues/411146

Follow the steps for Invited to workspace as a non-admin from the design doc.

Automated Tests

Not sure if we can test for this.

Issue Owner

Current Issue Owner: @stephanieelliott

[melvin-bot]

Triggered auto assignment to @stephanieelliott (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.

[trjExpensify]

Trying to confirm if this will be held on work in #44277. CC: @tienifr

[tienifr]

#44277 and this share the same anchorAlignment mechanism but I faced several issues in #44277 that do not happen in this one. I think we can definitely bring the anchorAlignment implementation from #44277 here and get the PR right away to speed up the process, no need to hold. I can take this too.

[trjExpensify]

But @tienifr does that mean the anchorAlignment approach isn't the right one if it doesn't work for #44277?

[tienifr]

It worked perfectly, there's just another problem that didn't work in that PR.

[trjExpensify]

Okay, can you post your proposal here for posterity? Also, @dukenv0307 can you be the C+ on this? Thanks!

[tienifr]

Proposal

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

Add tooltip for Workspace chat

What is the root cause of that problem?

This is a new feature

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

We can reuse the EducationalTooltip from #40066 with the same approach there.

Additionally, in order for the tooltip to align to the right of the target and the pointer to the left of the tooltip, we need to implement anchorAlignment property which mimics Popover's behavior:

App/src/components/PopoverWithMeasuredContent.tsx

Lines 43 to 46 in 456c8da

	anchorAlignment = {
	horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
	vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
	},

By default anchorAlignment is Bottom center.

1. Add anchorAlignment to Tooltip component tree
2. Modify this function to accept anchorAlignment:

App/src/styles/utils/generators/TooltipStyleUtils/index.ts

Line 126 in 58e2575

const createTooltipStyleUtils: StyleUtilGenerator<GetTooltipStylesStyleUtil> = ({theme, styles}) => ({

3. Apply the EducationalTooltip for ReportActionCompose with shouldRender as the condition of first time viewing the chat (I'm not sure if the BE already returned this prop or we need to create a new Onyx prop) with the given content.

Specifically for step 2, the current logic to compute tooltip position in createTooltipStyleUtils already supports the Bottom center alignment. We need to handle other cases:

Vertical alignment

Modify shouldShowBelow to include the TOP vertical alignment (tooltip shown below target):

App/src/styles/utils/generators/TooltipStyleUtils/index.ts

Lines 171 to 174 in 58e2575

	shouldShowBelow =
	shouldForceRenderingBelow ||
	yOffset - tooltipHeight - POINTER_HEIGHT < GUTTER_WIDTH + titleBarHeight ||
	!!(tooltip && isOverlappingAtTop(tooltip, xOffset, yOffset, tooltipTargetWidth, tooltipTargetHeight));

|| anchorAlignment.vertical === CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP

Horizontal alignment

Reuse the current logic to calculate initial tooltip left position which includes the distance from the left edge of screen to the target xOffset and avoids displaying too near to the edge of the screen horizontalShift and manualShiftHorizontal:

rootWrapperLeft = xOffset + horizontalShift + manualShiftHorizontal

LEFT

We just need to shift the pointer a bit to the right so it won't overflow the tooltip border:

pointerWrapperLeft = POINTER_WIDTH / 2

RIGHT

1. Move the pointer to the right by the tooltip width and shift left a bit so it won't overflow the tooltip border like this:

pointerWrapperLeft = horizontalShiftPointer + (tooltipWidth - POINTER_WIDTH * 1.5)

2. Move the tooltip to the right by the target width and shift left by itself so it won't get out of the target space like this:

rootWrapperLeft += tooltipTargetWidth - tooltipWidth

CENTER

Just reuse the current logic:

pointerWrapperLeft = horizontalShiftPointer + (tooltipWidth / 2 - POINTER_WIDTH / 2)
rootWrapperLeft += tooltipTargetWidth / 2 - tooltipWidth / 2

Code changes are here.

POC

[dukenv0307]

@trjExpensify I would love to work on this issue as C+. Thanks

[deetergp]

Proposal looks good to me, how about you @dukenv0307?

[dukenv0307]

LGTM as well @deetergp

[deetergp]

Nice! Let's 🚀 @tienifr

[tienifr]

When viewing the chat for the first time

@deetergp How could we know the user is viewing the chat for the first time? Does the BE return this or do we need to introduce a new Onyx value?

[deetergp]

Good question, @tienifr. How do we do it for the other tool tips?

[trjExpensify]

I believe for the quick action button we have a isFirstQuickAction boolean:

App/src/types/onyx/QuickAction.ts

Line 19 in fef5dfc

isFirstQuickAction?: boolean;

[trjExpensify]

@deetergp did you give @tienifr what he needs here to move this on?

[deetergp]

@trjExpensify @tienifr I have updated the Tooltip section of the doc with details on what to expect when deciding whether or not to show the tooltip. Please let me know if you've got any remaining questions. Thanks!

[tienifr]

@deetergp Could you please grant me access to the doc tienifr032022@gmail.com?

[trjExpensify]

@tienifr isn't C+, so hasn't got an NDA in place to access design docs. Let's get that into the GH issue OP.

[deetergp]

@trjExpensify @tienifr I've added the edits from the doc to the GH description under Frontend Changes (from doc) and Backend Changes (from doc).

[tienifr]

I marked the PR ready above 👆👆

[deetergp]

Oh nice! I must have missed the memo on that one 😅

[melvin-bot]

@deetergp, @stephanieelliott, @tienifr, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[deetergp]

Settle down Melvin, we just merged the PR.

[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.18-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Feature: Tooltip for workspace chat #45390

If no regressions arise, payment will be issued on 2024-08-19. 🎊

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

• @tienifr requires payment through NewDot Manual Requests
• @dukenv0307 requires payment (Needs manual offer from BZ)

[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:

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

[stephanieelliott]

Hey @dukenv0307 can you please propose a regression test when you get a chance?

[dukenv0307]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: N/A
• 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: N/A
• A discussion in #expensify-bugs 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: N/A
• Determine if we should create a regression test for this bug. Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression test:

1. Log in as User A, an account that owns a workspace
2. Invite B, a user that does not yet have an Expensify account
3. Check B's email for the invitation
4. Open the new.expensify.com link from the email in a different browser
5. On A, open the workspace chat
6. Verify a tooltip shows over the composer's Create button saying Get started! Submit your first expense or ¡Empecemos! Presenta tu primer gasto
7. Press Create button
8. Verify the tooltip disappears (or it will automatically disappear in 5 seconds)

Do we 👍 or 👎

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @tienifr owed $250 via NewDot
• ROLE: @dukenv0307 paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@stephanieelliott)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[stephanieelliott]

Summarizing payment on this issue:

• Contributor: @dukenv0307 $500 via Upwork -- offer extended, please accept: https://www.upwork.com/nx/wm/offer/103599573
• Contributor+: @tienifr $500 via NewDot -- please request

Upwork job is here: https://www.upwork.com/jobs/~0162602935c704dbe5

[tienifr]

@stephanieelliott The PR took a lot of times and efforts to implement (39 files changed). Should it be 500$? cc @dukenv0307 @deetergp

[stephanieelliott]

Chatted with @deetergp in DM, we agree that it's fair to raise this issue ot $500 given the work involved. Updated the payment summary here, also updated your Upwork offer @tienifr -- please accept when you get a chance!

[dukenv0307]

Thanks @stephanieelliott, I have accepted the offer on Upwork

[tienifr]

Updated the payment summary #45046 (comment), also updated your Upwork offer @tienifr

@stephanieelliott I actually receive payment via NewDot, I've requested there 👍

[garrettmknight]

@tienifr paid $500 via NewDot