feat: Add verified badge in domains list row after verifying the domain#83811
feat: Add verified badge in domains list row after verifying the domain#83811marcochavezf merged 7 commits intoExpensify:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where a domain row in the Workspaces list didn't display a "Verified" badge after the domain was successfully verified. Previously, only a "Not verified" badge was shown for admin-owned unverified domains; verified domains showed no badge at all.
Changes:
DomainsListRow: Adds anisDomainVerifiedprop and appliesstyles.badgeSuccessto the badge's border when the domain is verified. Also addsstyles.flexShrink1to the domain title text to prevent overflow.DomainMenuItem: Refactors badge text logic to show a "Verified" badge (with success styling) for verified admin domains, in addition to the existing "Not verified" badge for unverified admin domains.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/components/Domain/DomainsListRow.tsx |
Adds isDomainVerified prop; applies badgeSuccess style to badge border when verified; fixes text overflow with flexShrink1 |
src/components/Domain/DomainMenuItem.tsx |
Refactors badge text computation to show "Verified" badge for verified admin domains alongside existing "Not verified" badge |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| title={item.title} | ||
| badgeText={isAdmin && !isValidated ? translate('domain.notVerified') : undefined} | ||
| badgeText={badgeText} | ||
| isDomainVerified={isValidated} |
There was a problem hiding this comment.
The isDomainVerified prop is passed unconditionally to DomainsListRow regardless of whether isAdmin is true or false. When !isAdmin, badgeText is undefined so the badge block never renders and isDomainVerified has no effect. To keep the props consistent and avoid passing values that have no effect, consider passing isDomainVerified only when badgeText is defined — i.e., isDomainVerified={isAdmin ? isValidated : undefined} — so that the prop is only provided when it is actually meaningful.
| isDomainVerified={isValidated} | |
| isDomainVerified={isAdmin ? isValidated : undefined} |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| /** The text to display inside a badge next to the title */ | ||
| badgeText?: string; | ||
|
|
||
| /** Whether the domain is verified */ |
There was a problem hiding this comment.
❌ CLEAN-REACT-PATTERNS-3 (docs)
The isDomainVerified prop leaks domain-specific business logic into DomainsListRow, which should be a generic, reusable row component. This prop exists solely to conditionally apply a success style to the Badge, but the Badge component already has a built-in success prop that handles this. The DomainsListRow component shouldn't need to know about domain verification status — it should expose an abstract capability like isBadgeSuccess (or simply badgeSuccess), or pass Badge's success prop directly:
// In DomainsListRow props
type DomainsListRowProps = {
// ...
/** Whether the badge should use success styling */
isBadgeSuccess?: boolean;
// ...
};
// In DomainsListRow usage of Badge
<Badge
text={badgeText}
success={isBadgeSuccess}
textStyles={styles.textStrong}
badgeStyles={[styles.alignSelfCenter, styles.badgeBordered]}
/>Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
@abdulrahuman5196 I'm travelling right now, will look into this ASAP.
There was a problem hiding this comment.
Yes, this is a valid concern. I have updated the logic.
| const {translate} = useLocalize(); | ||
| const {isAdmin, isValidated, action} = item; | ||
|
|
||
| const badgeText = (() => { |
There was a problem hiding this comment.
❌ CLEAN-REACT-PATTERNS-4 (docs)
The IIFE (() => { ... })() used to compute badgeText is unnecessary complexity. This is a simple derived value that can be expressed as a ternary without wrapping it in an immediately-invoked function. Replace with a plain conditional:
const badgeText = isAdmin
? (isValidated ? translate('common.verified') : translate('domain.notVerified'))
: undefined;Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Could you check on this?
There was a problem hiding this comment.
The concern is valid, but the ternary approach triggers no-nested-ternary lint rule. I switched to a simple conditional assignment.
|
@Expensify/design please take a look. |
|
Resolving merge conflicts. |
|
Badge success updated in main to 
|
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.
|
|
@abdulrahuman5196 Friendly bump for review. |
abdulrahuman5196
left a comment
abdulrahuman5196
left a comment
There was a problem hiding this comment.
@samranahm Can you check on the comments?
| /** The text to display inside a badge next to the title */ | ||
| badgeText?: string; | ||
|
|
||
| /** Whether the domain is verified */ |
| const {translate} = useLocalize(); | ||
| const {isAdmin, isValidated, action} = item; | ||
|
|
||
| const badgeText = (() => { |
There was a problem hiding this comment.
Could you check on this?
|
@abdulrahuman5196 Please take a look. |
|
@abdulrahuman5196 Friendly bump. |
|
@abdulrahuman5196 Friendly bump. |
|
Checking now |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp
Android: mWeb Chrome
iOS: HybridApp
iOS: mWeb Safari
MacOS: Chrome / Safari
|
abdulrahuman5196
left a comment
abdulrahuman5196
left a comment
There was a problem hiding this comment.
Changes looks good and works well. Reviewers checklist is also complete.
All yours. @marcochavezf
🎀 👀 🎀
C+ Reviewed
|
@marcochavezf All yours, please take a look. |
|
🚧 @marcochavezf 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. |
|
@marcochavezf @heyjennahay @samranahm QA team does not have easy access to new unverified domains we can verify. Any chance you can run QA on this PR internally? |
|
🚀 Deployed to staging by https://github.com/marcochavezf in version: 9.3.52-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. The relevant article —
This PR makes the UI match what the documentation already describes (adding a visible "Verified" badge alongside the existing "Not Verified" badge). No other domain-related articles reference badge styling or verification status display. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.52-9 🚀
|
Explanation of Change
Fixed Issues
$ #79767
PROPOSAL: #79767 (comment)
Tests
Offline tests
QA Steps
Same as test
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari