Repository navigation
[Design Polish][Layout][Improvement] Tighten mobile gutters and card padding through spacing tokens - #100960
Conversation
cardPadding and pageGutter become narrow/wide token maps in src/styles/layoutSpacing.ts. useLayoutSpacing resolves them once per component from useResponsiveLayout and returns ready styles, so call sites stop switching on the layout flag. The home content container and widget header read the tokens. Values are unchanged.
Replaces shouldUseNarrowLayout ? styles.ph5 : styles.ph8 (and the p5/p8 and mhn5/mhn8 forms) in Home widgets, Section, Wallet rows and workspace sections with the resolved styles. No value changes.
no-layout-spacing-conditional flags styles.ph*/p*/pl*/pr*/m*/mh*/mhn* picked by a shouldUseNarrowLayout or screen-width ternary. Vertical spacing is left alone. Remaining sites outside cards are grandfathered in the seatbelt baseline.
For the three known pairs (ph5/ph8, p5/p8, mhn5/mhn8) the message says which useLayoutSpacing() style to use instead of pointing at the file.
cardPaddingBottom, cardPaddingLeft and cardMarginHorizontal cover the four sites that otherwise needed an inline style object, which STYLING.md does not allow.
STYLING.md gains a tokens section, the coding-standards skill gains UI-4, and CLAUDE.md points at both so generated code reaches for the variant and useLayoutSpacing() before the linter has to.
…nput The bare styles.mh5 on list rows was the mobile page gutter and the desktop row margin at once. pageGutterMargin keeps both values as they are today and lets the column width hook read the same number.
Implements the design polish spec for mobile: cards and tables sit 12px from the screen edge and card content is inset 24px. Wide layouts are unchanged.
cardGap joins the layout spacing tokens (12 narrow, 20 wide). The Home widget stack reads it; wide columns are unchanged. Measured from the design polish Figma frames.
Section and WorkspaceEmptyStateSection read cardGapBottom from useLayoutSpacing, matching the Home widget stack. Wide stays at 20.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@codex review |
The Add / Settings button row above workspace tables sat at 20px while the table below it moved to 12px. It now reads pageGutter, matching the Categories and Workspaces mocks. Description text stays at 20px.
![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: 9688a4eeda
ℹ️ 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".
|
Make some builds whenever you are ready, excited for this one! |
TabSelectorBase reads pageGutter instead of a fixed 20px, and the Workspaces header row reads pageGutterRight, so the tab bar lines up with the 12px gutter on mobile. Wide stays at 20.
The rule now reports a narrow/wide ternary only when the pair spells the card padding values, the 5 step narrow and the 8 step wide, in any inset property, and understands wide-first flags like isLargeScreenWidth. Unrelated pairs such as ph3/ph2 pass, so the seatbelt baseline needs no entries for this rule.
|
How is this one going? |
# Conflicts: # .claude/skills/app-coding-standards/rules/ui-4-layout-spacing-tokens.md # config/eslint/eslint.seatbelt.tsv # src/components/Table/TableRow.tsx # src/components/Table/useDynamicColumnWidths.ts # src/pages/Search/SearchPageNarrow/index.tsx # src/pages/workspace/perDiem/WorkspacePerDiemPage.tsx # src/pages/workspace/rules/PolicyRulesPage.tsx # src/pages/workspace/tags/DynamicWorkspaceViewTagsPage.tsx
# Conflicts: # config/eslint/eslint.config.mjs
|
@huult main is merged again and the test job should be green now, the fix landed on main last night. ready for a final look. |
|
@arosiclair all yours |
|
I made @arosiclair's comments on the MD files. We chatted last night so we should be good to merge this cycle. Fixing conflicts now 🙏 for no conflicts haha @huult for rereview once merge hits |
|
🚧 arosiclair 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! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/arosiclair in version: 9.5.5-0 🚀
|
|
No help site update needed: this PR only changes mobile spacing and padding (gutters, card padding, card gaps), and no article in |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.5.5-2 🚀
Bundle Size Analysis (Sentry): |
|
|
||
| const tableRowPressableStyles = [ | ||
| styles.mh5, | ||
| pageGutterMargin, |
There was a problem hiding this comment.
@grgia heads up, this looks like it stranded two section headers.
TableRow moved from styles.mh5 to pageGutterMargin, and the message says it keeps both values as they are today. That holds on wide, but pageGutter is {narrow: 12, wide: 20}, so on narrow the row went 20 to 12 while these two kept the hardcoded styles.mh5:
src/components/Tables/WorkspaceExpenseDefaultsTable/WorkspaceExpenseDefaultsTableRow.tsx:99src/components/Tables/WorkspaceSpendRulesTable/WorkspaceSpendRulesTableRow.tsx:75
Both render the grey section band above a Table.Row, so on small screens the band is inset 8px further than the rows and its text no longer lines up with the row text. Workspace > Rules > Expense defaults and Card restrictions both show it. Wide is unaffected, which is probably why it slipped through.
Explanation of Change
Mobile gutters go from 20px to 12px, card padding goes from 20px to 24px, and stacked cards sit 12px apart. Desktop doesn't change.
Rather than sweep ~40 files swapping
ph5forph6, I moved the values intosrc/styles/layoutSpacing.tsand auseLayoutSpacing()hook, migrated the cards, lists and tables onto it, and then flipped the numbers. The first six commits should be pixel-identical to main; the later ones are the actual design change plus the bits design found in the builds (Rules cards, Spend filter pills, Domain button rows, the expense report table). There's a lint rule so the oldshouldUseNarrowLayout ? styles.ph5 : styles.ph8pattern can't creep back (it tells you which style to use instead), and STYLING.md now covers the tokens.Measurements are from the Figma frames on the tracking issue. Two things I inferred rather than measured: the 12px gap between stacked Section cards (only drawn on Home), and sections inside the RHP picking up the mobile gutter. Both are one number to change if design wants otherwise.
Not touched: sections outside the central pane on desktop (still 20px), tab bars and pages inside the RHP on desktop (still 20px, only the page-level tab bars on Spend, Workspaces, Inbox, Rules and Workflows moved), and table row padding (already 16px on mobile).
Fixed Issues
$ #100819
PROPOSAL:
Tests
Offline tests
None, styling only.
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
Pending. Draft opened for local and design review first.