[HOLD AUTH 20746] Use deterministic default avatar instead of gray fallback when personal details unavailable#86713
Conversation
|
@hoangzinh 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95ed97e962
ℹ️ 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 actorDetails = parentReportAction?.actorAccountID ? personalDetails?.[parentReportAction.actorAccountID] : undefined; | ||
| const memberIcon = { | ||
| source: actorDetails?.avatar ?? FallbackAvatar, | ||
| source: actorDetails?.avatar ?? getDefaultAvatarURL({accountID: parentReportAction?.actorAccountID}), |
There was a problem hiding this comment.
Guard default-avatar lookup when accountID is missing
In getIconsForExpenseRequest, parentReportAction can be unavailable while report actions are still loading, so parentReportAction?.actorAccountID is undefined here. Passing that into getDefaultAvatarURL no longer preserves the old neutral fallback; it resolves to the deterministic default for accountID 0 (avatar 1), which shows a specific user-style avatar for an unknown actor and can cause misleading/flickering identity in the header. Please keep FallbackAvatar (or explicitly branch on a defined accountID) for this no-accountID state; the same pattern was introduced in the new chat-thread/IOU replacements.
Useful? React with 👍 / 👎.
Explanation of Change
When a user is invited to an expense report, their personal details may not have loaded yet. In this case, several functions in
ReportUtils.tswere falling back toFallbackAvatar(a generic gray silhouette SVG). This looks wrong because once personal details load, the user gets a deterministic colorful avatar based on their accountID.This PR replaces
FallbackAvatarwithgetDefaultAvatarURL({accountID})in the following functions:getIconsForParticipantsgetParticipantIcongetIconsForExpenseRequestgetIconsForChatThreadgetIconsForIOUReport(both managerIcon and ownerIcon)The two remaining usages of
FallbackAvatarare correct -- they handle cases where no accountID exists at all.This is a defensive FE improvement. It pairs with a backend fix in Auth that ensures personal details are sent to invitees.
Fixed Issues
$ #85240
PROPOSAL: #85240 (comment)
Tests
Offline tests
QA Steps
// 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