fix: add a tooltip to train users#82982
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@mkhutornyi 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] |
|
@puneetlath 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] |
|
Open for review. It seems we still have a bug with the tooltip placement in the native settings. I will investigate and provide an update soon. |
src/pages/Search/SearchAdvancedFiltersPage/SearchFiltersHasPage.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c261304ee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/pages/Search/SearchAdvancedFiltersPage/SearchFiltersHasPage.tsx
Outdated
Show resolved
Hide resolved
|
Triggered an adhoc build so I can review tomorrow. |
|
🚧 @JmillsExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
JmillsExpensify
left a comment
There was a problem hiding this comment.
Tests well! Product approved.
|
Looks like we have conflicts. And this is waiting for @mkhutornyi to take the first review? |
mkhutornyi
left a comment
There was a problem hiding this comment.
Please address bot feedback.
And add more unit test to make #82982 (comment) happy.
Reviewer Checklist
Screenshots/Videos |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 513dd6395e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function SearchFiltersHasPage() { | ||
| const styles = useThemeStyles(); | ||
| const {translate} = useLocalize(); | ||
| const {renderProductTrainingTooltip, shouldShowProductTrainingTooltip} = useProductTrainingContext(CONST.PRODUCT_TRAINING_TOOLTIP_NAMES.HAS_FILTER_NEGATION); |
There was a problem hiding this comment.
Gate has-negation tooltip to expense searches
useProductTrainingContext(CONST.PRODUCT_TRAINING_TOOLTIP_NAMES.HAS_FILTER_NEGATION) is always enabled on the Has filter page, so users opening this page for non-expense types (e.g., chat, where getHasOptions() only exposes link/attachment) still get the receipt-specific -has:receipt coaching. If they dismiss it in that context, the same dismissal key is set and the tooltip will not appear later when they actually search expenses, which defeats the intended training flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch! Let's fix this
src/languages/it.ts
Outdated
| outstandingFilter: '<tooltip>Filtra per le spese\nche <strong>necessitano di approvazione</strong></tooltip>', | ||
| scanTestDriveTooltip: '<tooltip>Invia questa ricevuta per\n<strong>completare il test drive!</strong></tooltip>', | ||
| gpsTooltip: '<tooltip>Tracciamento GPS in corso! Quando hai finito, interrompi il tracciamento qui sotto.</tooltip>', | ||
| hasFilterNegation: '<tooltip>Search for expenses without receipts using <strong>-has:receipt</strong>.</tooltip>', |
There was a problem hiding this comment.
Localize Italian has-negation tooltip copy
The new Italian translation value is left in English ("Search for expenses without receipts using ..."), so Italian users see untranslated product-training copy while other locales were localized in this change. This is a user-facing regression in locale quality introduced by this commit.
Useful? React with 👍 / 👎.
|
@Expensify/design please confirm tooltip position is correct. Screenshots are added in checklist. |
|
Position is looking good to me in those screenshots 👍 |
|
Is this ready for me to review? |
Not yet. Waiting for 2 above bugs to be addressed. |
|
Are you zoomed in or out when you are taking the screenshot? I see a slight white line separating the arrow and the body of the tooltip. My eagle eye also tells me maybe this needs to go to the right by 1px and down by 4px. |
|
@shawnborton are you looking at these screenshots? I think they're different and more up to date than what's in the OP. |
|
Well shucks I missed those! I think those look good. I'm not sure if we have a standard rule for always having the pointer of a tooltip 8px away from the target, but I think what is there is totally fine. All good from me then, carry on... |
LOL I wasn't sure about this either. If all our others are 8px away from the target I'm down to scootch this one a bit lower. |
|
@mkhutornyi i fixed |
mkhutornyi
left a comment
There was a problem hiding this comment.
@daledah Please pull main to fix failing test
|
🚧 @puneetlath has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ 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/puneetlath in version: 9.3.31-0 🚀
|
|
Deploy Blocker ##84168 was identified to be related to this PR. |
|
Deploy Blocker #84170 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.31-12 🚀
|






Explanation of Change
Fixed Issues
$ #81737
PROPOSAL: #81737 (comment)
Tests
Offline tests
same as tests
QA Steps
same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)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