Vertically center the condensed badge count in the LHN tab pill - #100443
Vertically center the condensed badge count in the LHN tab pill#100443MelvinBot wants to merge 1 commit into
Conversation
Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
🤖 Web test steps verification results: ✅ pass Verified on dev NewDot web: the LHN Unread/To-dos tab pills and the Search type-menu badges render their digits vertically centered for single-digit and two-digit counts (up to 12), at both wide and narrow viewports, with no failed requests or error UI. Web is the no-regression platform here — the bug in #100397 only reproduces on iOS Native and Android Native, so this run confirms the style change doesn't break the already-correct web rendering. view run · no recording available |
|
🤖 Android test steps verification results: ✅ pass Verified on the standalone Android NewDot build of |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp100443-android-hybrid-001.mp4Android: mWeb Chrome100443-mweb-chrome-001.mp4iOS: HybridApp100443-ios-hybrid-001.mp4iOS: mWeb Safari100443-mweb-safari-001.mp4MacOS: Chrome / Safari100443-web-chrome-001.mp4 |
|
@MelvinBot Tidy the tests/QA sections with the following test steps. The test steps are applicable for iOS/Android mobile versions. Precondition: Ensure there are few unread and todos messages. Also ensure few draft reports Test Steps:
|
|
@MelvinBot Please mark the PR as ready for review. |
|
🤖 Android test steps verification results: ✅ pass Updated the PR's |
|
@MelvinBot Also please tick the boxes in the PR Author Checklist section as I have already verified these in PR Reviewer Checklist |
|
🤖 Done — this PR is now marked Ready for review. Two things still need a human before a reviewer can sign off:
CI is otherwise green: all lint, typecheck, spellcheck, unit/Bun/Storybook tests, perf tests, and both native builds pass. The only failing check is |
|
🤖 Done — all 38 boxes in the Two things worth flagging: Two boxes are still unticked, both outside that section — the Correction to my last comment: I said the The only failing check remains |
rojiphil
left a comment
There was a problem hiding this comment.
@JmillsExpensify @roryabraham Changes LGTM.
Over to you for review. Thanks.
|
Videos look good to me 👍 |






















Explanation of Change
The unread count in the LHN tab pill renders too high on iOS and Android.
Three styles in
src/styles/index.tscombine to cause it:badgeTextspreadslineHeightBadge, which setslineHeight: 16.condensedBadgeTextoverridesfontSizedown to9(fontSizeExtraSmall) but leaves that inheritedlineHeight: 16in place.tabSelectorBadgesetheight: 16, whichdefaultBadge'sborderWidth: 1shrinks to a 14px content box.So a 9px glyph sits in a 16px line box inside a 14px container. Web is fine because CSS splits the 7px of leading symmetrically. Native does not — iOS approximates the correction with
NSBaselineOffset, and Android'sCustomLineHeightSpansplits the leadingceilabove /floorbelow on top ofincludeFontPadding: true.alignItems: 'center'has no slack left to fix it, andjustifyContent: 'center'acts on the horizontal axis here becausedefaultBadgesetsflexDirection: 'row'.This PR makes two changes:
condensedBadgeTextnow setslineHeight: undefined, so the digit centers on its own font metrics instead of inside an oversized line box. This is the same fix already used bybuttonText,avatarInnerTextSmall, andheaderEnvBadgeTextin this file. Scoping it tocondensedBadgeTextrather thanbadgeTextkeeps the blast radius to condensed badges only.tabSelectorBadgenow usesminHeight: 16instead ofheight: 16, so the pill can't clip its own content when the OS font scale is above 1.condensedBadgealready usesminHeight: 16, so this makes the two consistent.Other places that use this same badge config and should show the same improvement: the To-dos LHN tab (
src/pages/inbox/sidebar/InboxTabSelector.tsx) and the Search narrow type menu (src/pages/Search/SearchTypeMenuNarrow.tsx).Fixed Issues
$ #100397
PROPOSAL: #100397 (comment)
Tests
These steps apply to iOS: Native and Android: Native.
Precondition: the account has a few unread messages, a few to-dos, and a few draft reports.
Unreadtab message counter is vertically centered in the pill.Todostab message counter is vertically centered in the pill.Draftsreport counter is vertically centered in the pill.AI Tests
Run locally by MelvinBot on this branch (
Expensify/Apprequired-local bundle from the test-selection-matrix, plus the path-gated style check):npm run lint-changednpm run typechecknpm run spell-changednpm run gh-actions-unused-styles(scripts/findUnusedStyles.ts)npm run storybook -- --smoke-test --cinpm test—tests/ui(247 suites)npm test—tests/actions+tests/navigation(131 suites)npm test—tests/unit(~972 suites, run in 4 alphabetical chunks to stay inside the runner time limit)Prettier was covered through
npm run lint-changed— this repo has no standaloneprettiernpm script, and formatting is enforced by ESLint.The test steps above were also run end to end by MelvinBot on Android: Native (all 6 steps passed) and on web as a no-regression check. iOS: Native still needs a human check, as does behaviour at an enlarged OS font scale on both native platforms.
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari