IOU - Rate field disabled if destination is self DM and no workspace during manual track distance#82987
Conversation
…uring manual track distance
…uring manual track distance
…uring manual track distance
| <MenuItemWithTopDescription | ||
| key={translate('common.rate')} | ||
| shouldShowRightIcon={!!rate && !isReadOnly && iouType !== CONST.IOU.TYPE.SPLIT && !isUnreported} | ||
| shouldShowRightIcon={!!rate && !isReadOnly && iouType !== CONST.IOU.TYPE.SPLIT && (!isUnreported || isTrackExpense)} |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The boolean expression \!\!rate && \!isReadOnly && iouType \!== CONST.IOU.TYPE.SPLIT && (\!isUnreported || isTrackExpense) is duplicated identically on both the shouldShowRightIcon (line 554) and interactive (line 582) props. If this condition ever needs to change, both locations must be updated in sync, which is error-prone.
Extract it into a descriptive variable:
const isRateInteractive = \!\!rate && \!isReadOnly && iouType \!== CONST.IOU.TYPE.SPLIT && (\!isUnreported || isTrackExpense);Then use it in both props:
shouldShowRightIcon={isRateInteractive}
// ...
interactive={isRateInteractive}Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
NAB: @lorretheboy The suggestion to create a new variable in this case makes sense, since the value is the same.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppCleanShot.2026-02-20.at.12.48.46.4.mp4Android: mWeb ChromeCleanShot.2026-02-20.at.12.50.08.5.mp4iOS: HybridAppCleanShot.2026-02-20.at.12.45.51.2.mp4iOS: mWeb SafariCleanShot.2026-02-20.at.12.48.03.3.mp4MacOS: Chrome / SafariCleanShot.2026-02-20.at.12.42.29.1.mp4 |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
|
🚧 @dangrous 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/dangrous in version: 9.3.25-0 🚀
|
Revert "Merge pull request #82987 from lorretheboy/fix/82594"
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.25-13 🚀
|
|
🚀 Deployed to staging by https://github.com/dangrous in version: 9.3.26-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.26-8 🚀
|
Explanation of Change
Fixed Issues
$ #82594
PROPOSAL: #82594 (comment)
Tests
Precondition:
Offline 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))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.mov
Android: mWeb Chrome
website.android.mov
iOS: Native
Will update ASAP once I resolve iOS offline issue
iOS: mWeb Safari
website.ios.mov
MacOS: Chrome / Safari
website.mov