Repository navigation
Show the status emoji on the account avatar in the native tab bar - #104972
Conversation
Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
melvin-bot
Bot
added
Melvin-Test-Android
Melvin-Test-Web
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🤖 Android test steps verification results: ✅ pass On Android, the Account tab avatar in the native bottom tab bar shows a small status emoji badge at the bottom-right once a status is set. The badge updates when the emoji changes, disappears when the status is cleared, and never appears on other tabs. The PR's Tests section is still a placeholder, so these steps come from the linked issue and the PR description.
|
|
🤖 Web test steps verification results: On web, the Account avatar in the navigation bar shows, updates and clears the status emoji badge across tab switches, so the existing web behavior still works. The PR changes only the native tab bar, so this was a regression check using the linked issue's steps. I could not check the JS console because the web test tool has no console capture.
|
|
@codex review |
|
@MelvinBot review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Code reviewThe change looks correct and well scoped. I found no blocking bugs. The Android run above showed the badge appearing, updating and clearing as expected. Two edge cases are worth a look before merge:
Other notes (not blocking)
|
![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: e49fc12699
ℹ️ 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 emojiCode = useCurrentUserPersonalDetails().status?.emojiCode; | ||
| const emojiStatus = emojiCode ? {emoji: emojiCode, backgroundColor: theme.border} : undefined; |
There was a problem hiding this comment.
When a user is acting as a delegate and also has a status, this unconditionally creates the status badge, so the native Account tab shows the emoji. AccountNavigationAvatar and its test explicitly give the delegate avatar precedence in this case because that badge communicates which account the user is acting for. Read delegateEmailSelector in both native hooks and suppress the emoji while delegated (and ideally render the delegate badge) rather than displaying the conflicting status badge.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 21ace4a. Both native hooks now get the badge from a new shared hook, useTabEmojiStatus. It reads delegateEmailSelector and passes no emoji while you're delegated, the same way AccountNavigationAvatar handles it.
I didn't draw the delegate avatar badge on the native icon. That needs a second avatar decoded into the icon, which is a bigger change. I can add it here or in a follow-up if you want it.
| const paragraph = Skia.ParagraphBuilder.Make({maxLines: 1, textAlign: TextAlign.Center}) | ||
| .pushStyle({fontSize: variables.nativeTabIconEmojiStatusFontSize * scale}) | ||
| .addText(emoji) |
There was a problem hiding this comment.
When the selected status is the supported global_create emoji (U+E100 in assets/emojis/common.ts), this paragraph uses only the default system font, which has no glyph for that private-use code point, so the badge renders blank or as a missing-glyph box. The normal Text component detects this case and switches to Custom Emoji Font; the rasterizer needs the equivalent font selection for private-use emoji while retaining system fallback for standard color emoji.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 21ace4a. For a private use area emoji such as global_create, useTabEmojiStatus loads CustomEmojiNativeFont.ttf into a TypefaceFontProvider once. Until the font loads, the badge stays hidden.
The rasterizer builds the paragraph with that provider and tries Custom Emoji Font first. Skia keeps the system font manager as the fallback, so standard emojis still use the system color emoji font. Standard emojis get no provider and draw as before.
I haven't checked this on a device yet.
|
🤖 Web test steps verification results: ✅ pass I found no regressions on web. The tab bar and left nav render and navigate normally, and the Account avatar's status emoji badge appears, updates, and clears correctly in both narrow and wide layouts. This matches the code: the PR only changes native tab bar icon drawing and adds two new style variables.
|
@MelvinBot emoji and status are overlapped in landscape mode |
…ars the status dot Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
|
I pushed a fix for the overlap in 6cb553b. In landscape, the emoji badge now has a 5 pt radius instead of 6 pt, with a 7 pt emoji instead of 8 pt, so it no longer runs into the status dot. Cause: In landscape, the iOS avatar shrinks from 26 pt to 21 pt, but the badge and the dot kept their portrait sizes. On a 21 pt avatar, their cutout rings overlap by about 1 pt. On a 26 pt iOS avatar or a 24 pt Android avatar, they have room to spare. Change: Each Geometry and checks
|
…emojis with the custom emoji font Matches AccountNavigationAvatar, which shows the delegate avatar instead of the status emoji. Private use area emojis such as global_create have no glyph in Skia's system fonts, so the badge waits for the custom emoji font and draws with it. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
|
I pushed fixes for both points from the code review and both Codex threads in 21ace4a.
Checks
|
|
@codex review |
|
@MelvinBot review |
|
The failing check is unrelated to this PR, so I didn't push a fix. Analysis: Evidence:
Re-run the Reassure job and it should pass. The other failures are the reviewer checklist and independent approval checks, which pass once reviews are done. |
Code review (21ace4a)The latest commit looks good, and I found no blocking bugs. Both points from my earlier review are fixed:
One small edge case is worth a look: On iOS in landscape, the badge can clip the status dot when the account glyph shows instead of the avatar. The badge sits on the content box, which is the 18 pt glyph until the avatar decodes. A decode can fail, for example when the app starts offline. The landscape fix in 6cb553b sized the badge for the 21 pt avatar. On the 18 pt glyph, the badge's cutout ring cuts about 2 pt into the dot. Suggested fix: in Geometry, CI and other notes
|
|
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". |
|
🤖 Web test steps verification results: ✅ pass I found no regressions on web. The account avatar shows, updates and hides the status emoji badge correctly (none → 🌴 → 🎉 → none) in the wide left nav and the narrow header avatar. Web loads none of this PR's new code: the changed hooks are
|
|
@MelvinBot please don't hide the badge on the glyph. Web keeps the emoji badge when the avatar can't load. It shows the fallback avatar (UserAvatar.tsx:72-75) and Instead, fix the overlap by placing the dot and the badge on the avatar-sized box whenever there's an emoji, even while the glyph shows. |
…account glyph shows The badge is sized to clear the dot on the avatar. On the smaller glyph, used until the avatar decodes, the badge's cutout cut into the dot in iOS landscape. The badge stays on the glyph, matching the JS bar, which keeps it over the fallback avatar. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
|
I pushed 2326d3f to fix the overlap. The emoji badge now stays on the glyph, matching web. When there's an emoji, the status dot and the badge now sit on an avatar-sized box centered on the glyph. The badge is sized to clear the dot on that box, so the iOS landscape overlap is gone. The glyph itself doesn't move. Icons with no emoji draw the same as before. See I couldn't run ESLint, typecheck, oxfmt or Jest in this run, so CI will be the first check. I checked formatting against |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #104966 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| import type {TabIconEmojiStatus} from './tabIconRasterizer'; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- require() of a bundled asset is untyped; Metro resolves it to an asset ID |
There was a problem hiding this comment.
❌ CONSISTENCY-15 (docs)
The justification joins two independent clauses with a semicolon. Comments read more naturally as two separate sentences rather than one semicolon-spliced sentence.
Split it into two sentences:
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- require() of a bundled asset is untyped. Metro resolves it to an asset ID.Reviewed at: 85e9479 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Fixed in ce9d818. I split the justification into two sentences: useTabEmojiStatus.ts:20. ESLint passes on the file.
Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
|
@aimane-chnaif 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] |
|
Can we push the whole thing just slightly more to the right and more downwards? Maybe push to the right by 2px and down by 2px? 
But actually... as soon as Insights beta is lifted, this won't really be a thing because we'll only have it in the top bar I suppose. Either way, let's try to match this: |
|
@MelvinBot address #104972 (comment) |
Matches the design feedback to move the badge right and down, as the JS badge hangs off its avatar. The canvas grows to fit it. Android keeps the badge inside, because Material scales a larger icon down to its slot. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
|
I pushed d073072, which moves the emoji badge 2pt right and 2pt down on iOS. The badge and its cutout ring now hang past the avatar's bottom-right corner, like the badge in the top bar mockup. The status dot stays where it was.
I added a unit test that checks the offset for each platform. I couldn't run ESLint, typecheck, oxfmt or Jest in this run, so CI will be the first check. This code only runs on native, so please check the badge position on an iOS device. |
Done 
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 marcaaron 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/marcaaron in version: 9.5.24-1 🚀
|
|
No help site update is needed. This bug fix makes the native iOS and Android tab bar show your status emoji on the Account avatar, which the app already does elsewhere, and no article describes this badge. |
|
@aimane-chnaif do we have to test anything here. |
|
@m-natarajan here's test case:
@MelvinBot add this to QA Steps |
|
I added your test case to the QA Steps section of the PR description:
This change shows only in the native iOS and Android apps, so QA should test it there. |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.5.24-8 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
On iOS and Android, the bottom bar is the platform's native tab bar. Its icons are PNGs drawn with Skia in
tabIconRasterizer.ts. The Account icon was drawn from only the avatar, the status dot and the label. The user's status emoji never reached it, so the emoji badge that the JS tab bar shows (AccountNavigationAvatar→AvatarWithOptionalStatus) was missing.This PR:
status.emojiCodefromuseCurrentUserPersonalDetailsin the Android and iOSuseNativeTabBarOptions, and passes it to the Account tab icon only. The badge background istheme.border, the same as the JS badge.drawTabIconat the bottom-right of the avatar, opposite the status dot. It uses the same cutout ring as the dot and stays inside the canvas so it isn't cut off. The emoji is drawn as a Skia paragraph, so it falls back to the system color emoji font. The badge is drawn over the account glyph too, while the avatar is still loading.AI Tests (run by MelvinBot): ESLint,
npm run typecheck,oxfmt, the React Compiler check (both hooks compile with Babel and OXC) andnpm run spell-changedall pass on the changed files. The new test and the related tab bar tests pass (useNativeTabBarOptionsTest,NativeBottomTabViewTest,nativeTabScreenListenersTest,useAccountTabIndicatorStatusTest: 43 tests). The full Jest suite and the Storybook smoke test were not run. The changed code runs only on native, so I could not check the drawn badge on a device or emulator in this run.Fixed Issues
$ #104966
PROPOSAL: #104966 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari