Persist require fields tooltip dismissal and keep a direction always selected - #97726
Conversation
…selected Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
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.
|
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
Screen.Recording.2026-08-04.at.8.32.27.PM.mov |
|
🤖 Code review — both fixes are structurally sound. The tooltip-persistence change (NVP-backed dismissal + reserving a slot outside 1.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe495af4f7
ℹ️ 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".
|
@Krishna2323 working on the BE PR for the tooltips. LEts replace with |
|
BE PR is merged, will need to be deployed |
|
@JmillsExpensify @JS00001 @Expensify/design, what do you think about this comment? Any thoughts on how we should handle it? Maybe instead of "Don't require", we could show "Default" and simply close the RHP when nothing has changed? |
Is it not already selected in that video you shared? I don't think I totally understand what's going on there. |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
I think we need some clarification from @JmillsExpensify here |
|
I'm not a huge fan of the "default" option, as I think we've seen that it's confusing in practice and users don't actually understand what it means. I think I prefer to simply have the |
|
As for receipts, |
|
Right, so is default doing nothing then? If so, then we still need that weird tap to unselect behavior, right? We could call it something different to default though |
|
@Expensify/design, since we're only going to have the Require option for Description/Attendees, should we consider using toggles instead? |
|
A toggle can't be untoggled so that's the problem. There's technically 3 options. Unset, always require, and never require. Unless I've misunderstood something. |
@dubielzyk-expensify I'm asking for the Description and Attendees rows, since we will only have require option, I think it's better to use a toggle? |
|
Ahh, yeah I so 👍 Unless we go for a different pattern across it all and wanna unify. |
|
The left feels more right but it's a bit of a bummer that it looks so unbalanced. I think it's still also a bit confusing, cause if |
|
Hmm, yeah I see what you're saying @dubielzyk-expensify. Now that I see the UI, I guess I don't understand why we can support a toggle but not a segmented control? They are the same thing right? |
|
Discussed in Slack. I don't think we should hold up the release on this one. We can update/change post-launch. |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Updated description and attendee rows to use toggle: Monosnap.screencast.2026-08-11.18-39-23.mp4@situchan ready for review |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@JS00001 this one is ready for final review. |
|
🚧 JS00001 has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.4.53-0 🚀
|
|
🤖 Help site review: no changes required. I reviewed the changes in this PR against the help site articles under
The relevant help article, Workspace-Rules.md (the Require fields section under Category Rules), already documents this feature accurately at the appropriate level. No draft PR was created since there's nothing to update. If you feel the "Require" / "Don't require" options should be documented more explicitly on the help site, let me know and I'll draft that update. |
|
Deploy Blocker #98640 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.53-10 🚀
Bundle Size Analysis (Sentry): |


Explanation of Change
This PR fixes two regressions in Field requirements.
1. Coupling tooltip persisted incorrectly
The coupling tooltip tracked its dismissal in component state, so saving a rule and creating another remounted the row and caused the tooltip to reappear.
It now dismisses through the product training NVP, just like other educational tooltips, recording:
'x'for the close button'click'for a tooltip pressThe two tooltip names are excluded from
ProductTrainingTooltipNameso they reserve an NVP slot without joining the priority queue, since these tooltips render per row rather than throughProductTrainingContext.Rendering also waits for the NVP to load, preventing an already-dismissed tooltip from briefly flashing.
2. "Don't require" state for Description and Attendees
Description and Attendees are stored as booleans, so
"Don't require"is indistinguishable from having no override. As a result, reopening a saved rule showed neither option selected.These fields now default to Don't require instead of
undefined, and pressing the already-selected option is a no-op instead of clearing it.Receipt fields keep their existing blank state, where no selection means there is no category override and the policy-level receipt requirement still applies.
Fixed Issues
$ #96795
$ #96814
PROPOSAL:
Tests
#96795 Rules- Receipt require tooltip doesn't appears time after dismissal
#96814 Rules - Unable to save Don't require for Description and Attendees
Offline tests
QA Steps
Same as tests
Verify that no errors appear in the JS console
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
web_chrome.mp4