Feature: Tooltip for workspace chat#45390
Conversation
|
Update: I had several issues:
|
|
@tienifr did you fix those issues or ask for help if you need it? We want to keep this PR moving. CC: @dukenv0307 as you'll have some context on this. |
|
@tienifr Can you update the PR and let me know what's your blocker right now? Thanks |
|
@tienifr you have conflicts. Can you fix those and confirm this is ready for a review today? |
|
@dukenv0307 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] |
|
@tienifr Is it ready for review? |
|
Bump, @tienifr please confirm this is ready for re-review? |
|
Other than the bug, the videos above looked good to me! |
| <View style={pointerStyle} /> | ||
| </View> | ||
| </Animated.View> | ||
| <Portal hostName={!shouldUseOverlay ? 'modal' : undefined}> |
There was a problem hiding this comment.
Teleport the portal to the modal layer, otherwise, the tooltip in any modal/popover would be obscured by that modal. For example the tooltip for QAB.
| return; | ||
| } | ||
| setShouldUseOverlay(false); | ||
| hideTooltip(); |
There was a problem hiding this comment.
Interacting with shouldRender prop causes the target component to re-render thus leads to flickering. I move the logic to hide tooltip from the target component (i.e. composer) to inside the tooltip itself to avoid such flickering.
|
@dukenv0307 I fixed the Android bug above and also discovered several other bugs. Added explanations for them above ^. Ready for review. |
…oltip-for-workspace-chat
|
@dukenv0307 Thanks for the suggestions. Updated. |
|
LGTM |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/deetergp in version: 9.0.18-0 🚀
|
|
This PR is failing for Desktop app because of issue #47169 |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.18-10 🚀
|
| }} | ||
| wrapperStyle={styles.reportActionComposeTooltipWrapper} | ||
| shiftHorizontal={variables.composerTooltipShiftHorizontal} | ||
| shiftVertical={variables.composerTooltipShiftVertical} |
There was a problem hiding this comment.
On mobile safari, the viewport is scrolled when the keyboard is shown. We should also add the topOffset here to avoid this bug.
More details: #53976 (comment)
9 participants
Details
Display a tooltip the first time user views the workspace chat.
Fixed Issues
$ #45046
PROPOSAL: #45046 (comment)
Tests
new.expensify.comlink from the email in a different browserGet started! Submit your first expenseor¡Empecemos! Presenta tu primer gastoOffline tests
NA
QA Steps
See Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop