Dismiss Has filter tooltip on filter selection#84171
Conversation
|
|
||
| const updateSelectedItems = useCallback( | ||
| (listItem: ListItem) => { | ||
| hideProductTrainingTooltip(); |
There was a problem hiding this comment.
@MelvinBot This should be called only when Receipt checkbox is toggled.
There was a problem hiding this comment.
Updated — hideProductTrainingTooltip() is now only called when listItem.keyForList === CONST.SEARCH.HAS_VALUES.RECEIPT, so other filter options (Attachment, Tag, Category) no longer dismiss the tooltip.
|
|
||
| const updateSelectedItems = useCallback( | ||
| (listItem: ListItem) => { | ||
| if (listItem.keyForList === CONST.SEARCH.HAS_VALUES.RECEIPT) { |
There was a problem hiding this comment.
@MelvinBot Add more condition: shouldShowProductTrainingTooltip && to avoid redundant call.
There was a problem hiding this comment.
Added shouldShowProductTrainingTooltip && guard so hideProductTrainingTooltip() is only called when the tooltip is actually visible. Also added shouldShowProductTrainingTooltip to the dependency array.
|
@MelvinBot mark as ready for review |
melvin-bot
bot
requested review from
bernhardoj and
trjExpensify
and removed request for
a team
|
@bernhardoj 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] |
|
PR has been marked as ready for review. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safaribefore: Screen.Recording.2026-03-04.at.7.31.10.pm.movafter: Screen.Recording.2026-03-04.at.7.32.52.pm.mov |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0c0bc81b6
ℹ️ 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".
|
|
||
| const updateSelectedItems = useCallback( | ||
| (listItem: ListItem) => { | ||
| if (shouldShowProductTrainingTooltip && listItem.keyForList === CONST.SEARCH.HAS_VALUES.RECEIPT) { |
There was a problem hiding this comment.
The new dismissal guard only calls hideProductTrainingTooltip() when keyForList === receipt, so choosing other valid Has filters (for example Attachment, Tag, or Category) leaves the training tooltip active and it will continue to reappear later. Because this change is meant to dismiss the Has tooltip on filter selection, limiting the check to one option makes the behavior inconsistent for users who interact with any non-receipt filter first.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@MelvinBot Agree. Based on #84170 (comment), remove listItem.keyForList === CONST.SEARCH.HAS_VALUES.RECEIPT condition
There was a problem hiding this comment.
Removed the Receipt-specific condition. The tooltip now dismisses on any Has-filter row selection, per the clarification from trjExpensify on the linked issue.
|
Discussion is happening here. |
melvin-bot
bot
requested review from
ZhenjaHorbach
and removed request for
a team
|
@ZhenjaHorbach 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 all yours |
|
@MelvinBot typecheck is failing. |
|
Fixed the failing Root cause: PR #83878 (merged to Fix: Added the missing |
|
The typecheck failure is unrelated to this PR. Error: Analysis: This error is in Evidence: The PR diff only touches |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| import {FlashList} from '@shopify/flash-list'; | ||
| import type {FlashListRef, ListRenderItemInfo} from '@shopify/flash-list'; | ||
| import React, {useImperativeHandle, useRef} from 'react'; | ||
| import React, {useCallback, useImperativeHandle, useRef} from 'react'; |
There was a problem hiding this comment.
@MelvinBot revert this change. Being handled in #84178
There was a problem hiding this comment.
Reverted — removed all hideProductTrainingTooltip changes from updateSelectedItems. This is being handled in #84178 instead.
There was a problem hiding this comment.
@MelvinBot you reverted wrong file. Undo your last revert. You should revert BaseSelectionListWithSections.tsx change (useCallback)
There was a problem hiding this comment.
Done — undid the wrong revert on SearchFiltersHasPage.tsx and instead reverted the useCallback import in BaseSelectionListWithSections.tsx. The hideProductTrainingTooltip logic in updateSelectedItems is restored (with the shouldShowProductTrainingTooltip guard).
|
|
||
| const updateSelectedItems = useCallback( | ||
| (listItem: ListItem) => { | ||
| if (listItem.keyForList === CONST.SEARCH.HAS_VALUES.RECEIPT) { |
There was a problem hiding this comment.
| if (listItem.keyForList === CONST.SEARCH.HAS_VALUES.RECEIPT) { | |
| if (shouldShowProductTrainingTooltip) { |
There was a problem hiding this comment.
Applied — now using shouldShowProductTrainingTooltip as the guard so the tooltip only dismisses when it's actually visible, regardless of which row is selected.
|
Fixed the failing Root cause: The Fix: Added |
| import {FlashList} from '@shopify/flash-list'; | ||
| import type {FlashListRef, ListRenderItemInfo} from '@shopify/flash-list'; | ||
| import React, {useImperativeHandle, useRef} from 'react'; | ||
| import React, {useCallback, useImperativeHandle, useRef} from 'react'; |
There was a problem hiding this comment.
@MelvinBot pull main and remove useCallback. Don't commit any additional changes not requested by reviewers.
There was a problem hiding this comment.
@MelvinBot pull main and remove useCallback. Don't commit any additional changes not requested by reviewers.
There was a problem hiding this comment.
Done — merged main and removed all useCallback usage (import + 3 wrapper instances). No other changes.
|
I ran out of time before I could finish helping you. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in the #melvinbot-ai. Run details: https://github.com/Expensify/Expensify/actions/runs/22687735335 |
|
Fixed the failing |
|
Merged main and removed |
MelvinBot
force-pushed
the
claude-dismissHasFilterTooltipOnSelect
branch
from
9d2da03 to
4816b39
Compare
|
Fixed the failing |
Adds tooltip dismissal when selecting a Has filter option. Destructures hideProductTrainingTooltip from useProductTrainingContext and calls it in updateSelectedItems when the tooltip is showing. Removes useCallback wrappers (auto-memoized by React compiler). Co-authored-by: Mykhailo <mkhutornyi@users.noreply.github.com>
MelvinBot
force-pushed
the
claude-dismissHasFilterTooltipOnSelect
branch
from
1bb1ce3 to
b8666a1
Compare
|
Fixed the failing |
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
Product change that came from Jason 👍
|
@mkhutornyi everything good here? |
yes |
…ipOnSelect Dismiss Has filter tooltip on filter selection (cherry picked from commit 24fb1d4) (cherry-picked to staging by puneetlath)
|
🚀 Cherry-picked to staging by https://github.com/puneetlath in version: 9.3.31-6 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/puneetlath in version: 9.3.31-10 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.31-12 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/puneetlath in version: 9.3.32-0 🚀
|
5 participants
Explanation of Change
The educational tooltip on the "Has" filter page (added in PR #82982) does not dismiss when the user selects a filter option (e.g., Receipt). This happens because
SearchFiltersHasPagedestructuresuseProductTrainingContextbut omitshideProductTrainingTooltip, so theupdateSelectedItemscallback never dismisses the tooltip.This PR adds
hideProductTrainingTooltipto the destructured values and calls it at the start ofupdateSelectedItems, matching the established pattern used byAccountSwitcher,OptionRowLHN, andThreeDotsMenu.Fixed Issues
$ #84170
Tests
Offline tests
N/A - tooltip dismissal is a purely UI interaction that writes to Onyx optimistically. No network dependency.
QA Steps
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))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
N/A - no visual UI changes, only tooltip dismiss behavior
Android: mWeb Chrome
N/A - no visual UI changes, only tooltip dismiss behavior
iOS: Native
N/A - no visual UI changes, only tooltip dismiss behavior
iOS: mWeb Safari
N/A - no visual UI changes, only tooltip dismiss behavior
MacOS: Chrome / Safari
N/A - no visual UI changes, only tooltip dismiss behavior