[Hold #54924] Show Product tooltip for Workspace participant on create expense#56041
[Hold #54924] Show Product tooltip for Workspace participant on create expense#56041parasharrajat wants to merge 15 commits intoExpensify:mainfrom
Conversation
|
@hoangzinh Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@hoangzinh Looks like we don't need to manage the first-time submitter logic based on the discussion on the issue. Also, When you scroll the page, the tooltip does not move along. I think there was an issue for this already on the app. So that should solve it too. But I believe that is out of scope of this PR as we will have to make change to the tooltips which will affect all tooltips in the app. #54924 What do you think? |
Yes, I agree @parasharrajat |
|
I think we should auto-dismiss the tooltip if a user submits an expense after seeing it. So next times, it won't show again. What do you think? Screen.Recording.2025-01-31.at.15.09.27.mov |
|
@parasharrajat can you also put a link into this PR checklist? Thank you
|
|
I was OOO. I will catch up soon with this one. |
|
@parasharrajat Do you have a timeframe on when PR is ready for next review? Thank you. |
|
May be today or tomorrow. |
Can you confirm this on the issue with the team? |
This needs a backend change. cc: @stitesExpensify |
👍 Just put a backend PR here. I'll update here when it's deployed |
|
@hoangzinh Can you please remind me what is next here? |
|
I think we're waiting BE API here then retest this PR |
|
@stitesExpensify Is the Backend PR merged? |
|
Yes it is on production! |
|
Bump @stitesExpensify |
1 similar comment
|
Bump @stitesExpensify |
stitesExpensify
left a comment
There was a problem hiding this comment.
Sorry, I was OOO last week. Everything looks good to me but there are conflicts now :/
|
Looks like there are new conflicting changes and we will have to retest everything from start.... |
|
@hoangzinh Can you please retest this, there were new tooltips added in #56759. Can you please check that those are not affecting ours or vice-versa? |
|
My tests passed. I noticed a a global issue with tooltip that I will report separately. |
| ]} | ||
| /> | ||
| {(hovered?: boolean) => ( | ||
| <EducationalTooltip |
There was a problem hiding this comment.
It's weird that we have 2 nested EducationalTooltip here. @parasharrajat
There was a problem hiding this comment.
Yes, because there are two tooltips we need to show. Do you have any suggestion on how can we handle this?
I can refactor this to keep one EducationalTooltip and decide to show the one based on the context values. But that will still keep two useProductTrainingContext.
There was a problem hiding this comment.
My initial idea is to pass either WORKSPACE_EXPENSE_SUBMIT or SCAN_TEST_TOOLTIP_MANAGER tooltip here
App/src/components/SelectionList/InviteMemberListItem.tsx
Lines 99 to 100 in 7bbda5d
There was a problem hiding this comment.
How will we determine which one to show here?
There was a problem hiding this comment.
Updated, check now.
|
Yeah, We discussed this earlier #56041 (comment). #54924 will solve that for us. |
|
I noticed that scrolling the participant list up after scrolling down a few times, hide the tooltip. Are we solving this issue somewhere? It is a global issue with tooltips I believe, I can reproduce the same problem with #56759. IMO, tooltips were never designed to work in a list environment where there are multiple instances of the same tooltips but only a few are active. I have a solution but I am not sure whether that will work fine everywhere. We can work on this as a separate issue and I can create a PR for the same. I would say we hold this until #54924 is done. |
|
I also agree that we should hold off on #54924 because we also apply this tooltip for existing users (who might have a long list of recipients) |
|
Not needed for now. |



Explanation of Change
Fixed Issues
$ #55558
PROPOSAL: #55558 (comment)
Tests
Offline tests
Same as tests
QA Steps
same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
30.01.2025_19.29.07_REC.mp4
Android: mWeb Chrome
30.01.2025_19.30.13_REC.mp4
iOS: Native
30.01.2025_19.18.09_REC.mp4
iOS: mWeb Safari
30.01.2025_19.19.25_REC.mp4
MacOS: Chrome / Safari
30.01.2025_19.08.49_REC.mp4
MacOS: Desktop
30.01.2025_19.31.01_REC.mp4