Conversation
|
@bernhardoj 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] |
melvin-bot
Bot
requested review from
joekaufmanexpensify
and removed request for
a team
September 18, 2026 11:36
Contributor
|
Screenshots are looking good to me |
Contributor
Reviewer Checklist
Screenshots/VideosiOS: HybridAppiOS: mWeb Safari |
Contributor
|
Agree, thank you! |
bernhardoj
approved these changes
Sep 18, 2026
melvin-bot
Bot
requested review from
aldo-expensify,
cristipaval and
luacmartins
September 18, 2026 13:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Explanation of Change
Fixes the account tooltip's caret detaching from the bubble on mWeb. The caret was rendering over the bubble's rounded corner, so it looked like it was floating next to the tooltip instead of pointing out of it.
With
RIGHTanchor alignment the caret sits 8px from the bubble's right edge, putting its center 16px in, not 20px, so it was already 4px off the 40px button's center. The bubble's right edge also lined up exactly with the 12px gutter (mr3=gutterWidth= 12), and the gutter check is strict, but the two numbers it compares come from different places: the bubble's edge fromgetBoundingClientRect(fractional CSS pixels, and on mWeb the viewport width itself often isn't a whole pixel) and the window width fromDimensions, which rounds. So on mWeb the bubble read as overflowing by a sub-pixel,roundToNearestMultipleOfFourturned that into a full 4px shift, and the caret compensation that follows pushed the caret onto the corner. Its clamp allows for a 4px radius (componentBorderRadiusSmall), while the product training bubble overrides its radius to 8px.shiftHorizontal: -4centers the caret on the button, and the 4px of gutter clearance that comes with it stops the sub-pixel overflow from triggering a shift at all. Same approachThreeDotsMenuandAccountSwitcheralready use.Also evens out the gap between the search, concierge and account buttons. The account button looked closer to concierge than concierge is to search. All three share
touchableButtonImage(a 40px centered box), but the avatar's content is 28px wide against the icons' 20px, so its visual gap was 16px where the others were 20px. A 4px left margin on the account button makes them uniform. Change requested by the design team: #100705 (comment)Fixed Issues
$ #101405
PROPOSAL: N/A
Tests
insightsPagebeta.Offline tests
N/A
QA Steps
NoQA.
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
Android: mWeb Chrome
MacOS: Chrome / Safari