Repository navigation
Migrate User Avatar usages - #98031
roryabraham merged 14 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05ef70312c
ℹ️ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6657970111
ℹ️ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b535919c94
ℹ️ 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".
|
@mananjadhav 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] |
|
PR doesn’t need product input as a refactor PR. Unassigning and unsubscribing myself. |
|
Reviewing the PR. |
mananjadhav
left a comment
There was a problem hiding this comment.
Left a few comments.
|
@mananjadhav thx for a good review 😄 Addressed all. |
|
|
||
| const subscriptionLink = subscriptionPlan ? `${environmentURL}/${ROUTES.SETTINGS_SUBSCRIPTION.getRoute(Navigation.getActiveRoute())}` : CONST.PLAN_TYPES_AND_PRICING_HELP_URL; | ||
|
|
||
| const iconSize = isIllustration ? 48 : StyleUtils.getAvatarSize(CONST.AVATAR_SIZE.DEFAULT); |
There was a problem hiding this comment.
suggestion: Can we replace the magic number with some const please?
mananjadhav
left a comment
There was a problem hiding this comment.
One final comment. Starting to test.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb Chromemweb-android-avatar.moviOS: HybridAppios-avatar.moviOS: mWeb Safarimweb-safari-avatar.movMacOS: Chrome / Safariweb-user-avatar.mov |
|
Tested web, still checking other platforms once. |
|
@mananjadhav kind bump |
|
Apologies I had to take up a deploy issue. Back to testing this. |
|
Checklist here #98031 (comment) |
| } | ||
| } | ||
|
|
||
| const integrationIcons = [ |
There was a problem hiding this comment.
NAB for now: I don't think that we should use static arrays and .map for rendering like this. A few reasons:
- It will be more tempting for future authors to add a configuration prop to the thing rendered in the
.mapcallback to handle a different variant of the thing in the array. - React Compiler won't memoize it as granularly. I think the whole
.mapcallback will be memoized as one function, rather than granularly memoizing all the JSX nodes. - It doesn't have that much of a readability boost over JSX.
- The guidelines say not to do it this way
Not going to block on this, but @jmusial we should address this in a fast-follower.
There was a problem hiding this comment.
@MelvinBot create an issue for this. Label it Daily, assign to jmusial and roryabraham.
|
🚧 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.61-0 🚀
|
Help site review — no changes requiredI reviewed the changes in this PR against the help site articles under This PR is a purely internal component refactor: it migrates raw No draft PR was created since there is nothing to document.
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.4.61-4 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
This PR migrates raw
Avatarcall sites across 33 files to theUserAvatar(orIconorAccountAvatarif the case calls for it).This removes duplicated avatar-resolution logic and per-site Onyx lookups and ensures consistent fallback and tooltip behaviour everywhere a single account's avatar is rendered.
Some
Avataruses are left un migrated - this depend on dynamiciconsand can be eitherUserorAvatarFixed Issues
$ #95600
PROPOSAL:
Tests
Offline tests
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-18.at.14.30.42.mov
Android: mWeb Chrome
Screen.Recording.2026-08-18.at.14.30.42.mov
iOS: Native
Screen.Recording.2026-08-18.at.14.28.38.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-08-18.at.14.13.17.mov