Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🚧 @shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Do you want me to start reviewing this yet? I see a lot of inconsistencies so far. Like we aren't using rounded border or bold text here: And this badge isn't updated here: Or here: Or here: |
|
@shawnborton yeah its still not ready for review. Thanks for the review.. i am covering all cases |
|
update: i tackled most of the issues ... doing retesting |
|
Sounds good, let us know when you want us to do another adhoc build. |
|
feel free to create another build hoc ... i have covered the mentioned cases here are the results so far: Web Screenshots
|
|
Hmm this doesn't look correct: It should map to the new "Success" badge style here: |
|
Same thing here: That badge style is not updated either... |
|
@shawnborton For the this is how it looks like in staging vs current branch but i want to double check as i dont feel confident about how it looks what do u think |
|
For that one, let's update it to use "Success with Icon": |
|
(also that should say 16px tall, not 20px tall - sorry about that!) |
|
🚧 @shawnborton 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! 🧪🧪
|
|
Some minor comments to address in the meantime... this is my fault, but let's use the default button color as the default badge color, right now I think we have them using rowHover which was my fault: But then when the badge is indeed inside of an "active" LHN row, let's make the button use rowHover, like so: |
|
Something happened with the alignment of the workspaces table: |
|
The text color of the Success badge is incorrect: From Figma, it should match the Paid badges we use: |
|
Thanks for the feedback ... covering that now |
| <Badge | ||
| text={reportStatus} | ||
| isCondensed | ||
| badgeStyles={[ |
There was a problem hiding this comment.
Did you end up changing this?
| error?: boolean; | ||
|
|
||
| /** Whether badge uses strong (filled) style instead of outlined */ | ||
| isStrong?: boolean; |
There was a problem hiding this comment.
strong to me indicates bold. Why not isFilled if it's determining whether it is filled or not?
src/components/MenuItem.tsx
Outdated
| badgeSuccess?: boolean; | ||
|
|
||
| /** Whether the badge should use strong (filled) variant */ | ||
| badgeStrong?: boolean; |
There was a problem hiding this comment.
If it's a bool, I think it should be isBadgeStrong. Also, same comment as above, isBadgeFilled makes more sense to me.
There was a problem hiding this comment.
I named it strong to align with the design terminology
the design call these variants "Success Strong" and "Danger Strong". As for the is prefix, I kept it consistent with the existing boolean props on Badge and MenuItem (success, error, pressable) which don't use is either.
should we still change it ?
There was a problem hiding this comment.
Ok I'm fine leaving the strong ones then. I still think we should use correct nomenclature for booleans though. I think our past-selves did it wrong by not using proper naming for those props.
src/components/MenuItem.tsx
Outdated
| badgeStrong?: boolean; | ||
|
|
||
| /** Whether the badge should use condensed (smaller) sizing */ | ||
| badgeCondensed?: boolean; |
There was a problem hiding this comment.
| badgeCondensed?: boolean; | |
| isBadgeCondensed?: boolean; |
src/hooks/useSearchTypeMenu.tsx
Outdated
|
|
||
| sectionItems.push({ | ||
| badgeText: getItemBadgeText(item.key, reportCounts), | ||
| badgeSuccess: true, |
There was a problem hiding this comment.
Should use is if boolean.
src/pages/Search/SearchTypeMenu.tsx
Outdated
| interactive | ||
| title={translate(item.translationPath)} | ||
| badgeStyle={styles.todoBadge} | ||
| badgeSuccess |
There was a problem hiding this comment.
Should use is if boolean.
| badgeSuccess?: boolean; | ||
| badgeStrong?: boolean; | ||
| badgeCondensed?: boolean; |
There was a problem hiding this comment.
Should use is if boolean. Also, same comment as above about strong.
The same Badge configuration (isCondensed, ml0, borderNone, custom background/text color) was duplicated across ParentNavigationSubtitle, MoneyRequestReportPreviewContent, StatusCell, and WithdrawalIDListItemHeader. Extract into a reusable StatusBadge wrapper.
…sed-strong-variants
|
@abzokhattab typecheck failing. |
|
yeah its unrelated to the changes ... we can wait until it gets fixed on main |
…sed-strong-variants
…sed-strong-variants
|
there we go @puneetlath |
|
Looks like there are conflicts now. |
|
done |
|
🚧 @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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.3.32-0 🚀
|
|
Deploy Blocker #84360 was identified to be related to this PR. |
- Add isHighlighted prop to MemberRightIcon to apply badgeDefaultActive style when the parent row is selected, preventing the badge from blending into the activeComponentBG highlight - Map DONE action to reportStatusBadge.closed in BadgeActionCell so the Done action badge matches the Done status badge color (pink)
- Add ListItemFocusContext so InviteMemberListItem can propagate isFocused to rightElement children. MemberRightIcon consumes this context to apply badgeDefaultActive when the row is focused, preventing the badge from blending into the activeComponentBG highlight. - Map DONE action to reportStatusBadge.closed in BadgeActionCell so the Done action badge matches the Done status badge color (pink).
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.32-3 🚀
|
10 participants
Explanation of Change
isStrongandisCondensedprops toBadgecomponentFixed Issues
$ #81993
PROPOSAL:
Tests
Offline tests
Same as tests
QA Steps
Same as tests
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
Screen.Recording.2026-02-22.at.22.40.42.mov
Android: mWeb Chrome
Screen.Recording.2026-02-22.at.22.43.33.mov
iOS: Native
Screen.Recording.2026-02-22.at.22.31.53.mov
iOS: mWeb Safari
Screen.Recording.2026-02-22.at.22.45.33.mov
MacOS: Chrome / Safari
Screen.Recording.2026-02-22.at.22.10.37.mov