[Design Polish] Use 15px bold for content card and widget headers - #100835
Conversation
Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
🚧 shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
🤖 Android test steps verification results: ✅ pass PR #100835's Tested on branch |
|
🚧 grgia has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
🤖 Web test steps verification results: ✅ pass PR #100835's
Tested on branch view run · no recording available |
|
Testing well on my end 👍 |
| cardSectionTitle: { | ||
| fontSize: variables.fontSizeLarge, |
There was a problem hiding this comment.
@MelvinBot is there a reason to do it this way rather than <Text variant="textStrong" ...
please dont change, answer in thread
There was a problem hiding this comment.
Two reasons, and they're not equally strong.
cardSectionTitle — variant wouldn't work without a much larger change.
In src/components/Text.tsx:65-66 the variant is spread before style, so anything in style wins. Every card header renders as style={[styles.textHeadline, styles.cardSectionTitle, ...]}, and textHeadline is textVariants.h1 + color: theme.heading. So <Text variant="textStrong" style={[styles.textHeadline, ...]}> would render at h1 size — the variant would be dead. Making it work means dropping textHeadline and re-supplying color: theme.heading at each call site.
And most of those call sites aren't <Text> at all — they pass the style down as a titleStyle/titleStyles prop, so there's no variant prop to set without threading a new one through each component:
The 9 call sites
Passed as a prop (no <Text> to put variant on):
src/pages/workspace/WorkspaceOverviewPage.tsx:675src/pages/workspace/fields/WorkspaceFieldsSection.tsx:223src/pages/workspace/travel/WorkspaceTravelBillingSection.tsx:504src/pages/workspace/workflows/tabs/WorkflowsSectionCard.tsx:34
Direct <Text>, but stacked behind textHeadline:
src/components/Section/index.tsx:184(covers Account > Profile, Domains > SAML, and every otherSection-based card)src/components/SpendRules/SpendRulesSection.tsx:99src/components/AgentRules/useAgentRulesSectionHeader.tsx:38src/pages/workspace/rules/MerchantRulesSection.tsx:155src/pages/workspace/reports/WorkspaceReportsPage.tsx:77
getWidgetContainerTitleStyle — you're right, variant would be cleaner here.
Only two consumers, both plain <Text style={styles.getWidgetContainerTitleStyle(theme.text)}> with nothing else stacked on: WidgetContainer.tsx:36 and InsightTitleDropdown.tsx:82. Those could become <Text variant="textStrong" color={theme.text}> and the helper could go away entirely. I kept them consistent with the cardSectionTitle change instead, but that's a preference, not a constraint.
Worth noting: nothing in src/ uses the variant prop yet, so either way this would be the first usage.
Not changing anything, per your note.
|
Over to @huult for final review |
|
@shawnborton @huult melvin timed out, can we confirm/ wrap up this PR? |
|
Let's start merging the done ones |
Co-authored-by: huult <huult@users.noreply.github.com>
e9b2e22
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
|
There's nothing to fix in the code — the only failing check is What the check found (run log): Two things stack up:
How to clear it: get a fresh approving review from someone who isn't Every other check on Next Steps: Reply with |
|
@grgia Yes, we can merge this PR. |
|
🚧 grgia has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Nice, let's get it merged! |
|
NiceFont ✅ send it |
|
All checks are green now — nothing left to fix, and no push was needed.
Current state on As noted in my earlier comment, that check was never about the diff — it counts approvals from reviewers who aren't commit authors. One thing worth knowing for the future: if another commit lands here, Next Steps: Reply with |
|
Going to merge as I think we're all good here. |
|
🚧 shawnborton 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/shawnborton in version: 9.4.78-0 🚀
|
|
🤖 No help site changes are required. I did not create a docs PR. Why: This PR only changes two style definitions in Nothing user-facing changed that the help site documents:
@shawnborton, please confirm you agree that no help site update is needed here. If you want the docs to cover something else from this change, reply with view run · no recording available |
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.4.78-3 🚀
Bundle Size Analysis (Sentry): |







Explanation of Change
Content card headers were 17px bold, which added another font size to pages that already have several. They now use the existing
textStrongtext variant — 15px bold with a 20px line height — so card headers match the design polish spec and reduce the number of type sizes on a page.Two style definitions carry every affected header, so no component files needed to change:
cardSectionTitle(used bySection, and therefore by Account > Profile, Workspaces > Overview, Domains > SAML, and every otherSection-based card) now spreadstextVariants.textStronginstead of settingfontSize: variables.fontSizeLarge/lineHeight: variables.lineHeightXLarge. It is applied aftertextHeadline, so it keeps the heading color while overriding the family, size, and line height.getWidgetContainerTitleStyle(used byWidgetContainerandInsightTitleDropdownon Home) now spreadstextVariants.textStronginstead of the rawfontSize: 17literal.Using the variant rather than raw values also removes one
rulesdir/no-raw-typographyviolation, which is why the ESLint seatbelt count forsrc/styles/index.tsdrops from 28 to 27.Per
grgia's comment, bothSectionandWidgetContainertitles use thetextStrongvariant.Fixed Issues
$ #100809
PROPOSAL:
Tests
Offline tests
QA Steps
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
Verified by MelvinBot on dev web — not a substitute for the human co-author's testing.
Home — widget headers ("Recently added", "Discover"):
Account > Profile — section card headers ("Public", "Private"):