Repository navigation
Migrate Workspace Avatar usages - #97465
roryabraham merged 13 commits into
Conversation
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.
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@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] |
|
@parasharrajat @roryabraham One of you needs to 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] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ffb29df87
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-08-06.at.21.54.08.movAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-08-06.at.21.46.54.mov |
| shouldShowRightIcon={!isReadOnly && !!canUpdateSenderWorkspace} | ||
| title={senderWorkspace?.name} | ||
| icon={senderWorkspace?.avatarURL ? senderWorkspace.avatarURL : getDefaultWorkspaceAvatar(senderWorkspace?.name)} | ||
| icon={senderWorkspace?.avatarURL} |
There was a problem hiding this comment.
Can you elaborate on this change? Why do we need this change?
There was a problem hiding this comment.
Now WorkspaceAvatars calculate fallback internally (WorkspaceAvatar.tsx:35-38), so passing defaults on callsites is redundant
| ); | ||
| } | ||
|
|
||
| WorkspaceCell.displayName = 'WorkspaceCell'; |
There was a problem hiding this comment.
Should we bring back this line?
There was a problem hiding this comment.
not really, it's not doing anything
| size = CONST.AVATAR_SIZE.DEFAULT, | ||
| type = CONST.ICON_TYPE_AVATAR, | ||
| avatar, | ||
| size = CONST.AVATAR_SIZE.XXXX_LARGE, |
There was a problem hiding this comment.
Are we correct to change the default size from CONST.AVATAR_SIZE.DEFAULT to CONST.AVATAR_SIZE.XXXX_LARGE?
There was a problem hiding this comment.
On main, old DEFAULT default was dead code - every call site passed size={CONST.AVATAR_SIZE.XXXX_LARGE}
I found this requirement in the original issue. @jmusial Are you planning to do it in another PR? |
roryabraham
left a comment
roryabraham
left a comment
There was a problem hiding this comment.
Overall this all seems like a step in the right direction. I'm a bit shaky on whether a ReactNode as a prop is really a best practice. I think really we want to compose via JSX trees rather than props. But it also seems like to do that would require further decomposition of other components, which we can evaluate separately.
Possibly, working through the issue it turned out there are sites that genuinely require I'd suggest dropping this requirement from sub issue and revisiting after #95599 and #95600 are done |
|
🚧 roryabraham 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/roryabraham in version: 9.4.52-0 🚀
|
|
🤖 No help site changes required. I reviewed the changes in this PR against the help site content under This is a purely internal code refactor with no user-facing impact:
The PR itself states "No user-facing behavior change intended," and the diff confirms it — there are no Since the help site documents user-facing product behavior (which is unchanged here), no articles need updating and no draft docs PR was created. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.52-11 🚀
Bundle Size Analysis (Sentry): |
| {shouldUseNarrowTableLayout && ( | ||
| <View style={[styles.flex1, styles.flexRow, styles.gap3, styles.alignItemsCenter]}> | ||
| <Avatar | ||
| <WorkspaceAvatar |
There was a problem hiding this comment.
Checklist
This change caused the crash reported in #100204. avatarID can be undefined when switching between the Spend and Workspace pages while inviting a member to a workspace.
item.policyID can be undefined at render time because a new policy_<id> record doesn't arrive all at once, it streams in as several partial Onyx merges, and fields like role can arrive before id does. A render that catches that gap reaches WorkspaceAvatar.tsx:58 with no id to work with.
6 participants
Explanation of Change
Migrates call sites with a statically known avatar kind from the legacy
Avatarwrapper toWorkspaceAvatardirectly.Changes
AvatarButtonWithIcon&AvatarWithImagePickerto accept a ready avatar node instead of rendering the avatar internally from source/type/DefaultAvatar props.Adds a
getAccountIDFromAvatarIDhelper that safely narrowsavatarIDto a numeric account ID, rejecting policy-ID-shaped strings whole instead of parsing a bogus leading number.Drops redundant
getDefaultWorkspaceAvatarfallbacks at 9 call sites -WorkspaceAvatarnow derives the default fromnameinternally (MenuItem's workspace branch no longer requiresiconfor this to kick in).No user-facing behavior change intended; sites with runtime-dynamic icon types keep the wrapper.
Fixed Issues
$ #95599
PROPOSAL:
Tests
Scenario 1
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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-08-03.at.17.12.24.mov
Android: mWeb Chrome
Screen.Recording.2026-08-03.at.17.17.10.mov
iOS: Native
Screen.Recording.2026-08-03.at.17.30.40.mov
iOS: mWeb Safari
Uploading Screen Recording 2026-08-03 at 17.31.44.mov…
MacOS: Chrome / Safari
Screen.Recording.2026-08-03.at.17.07.06.mov