Repository navigation
[Design Polish][Layout][Improvement] Tighten mobile gutters and card padding through spacing tokens #100960
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+749
−195
Merged
[Design Polish][Layout][Improvement] Tighten mobile gutters and card padding through spacing tokens #100960
Changes from all commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
8e1aed8
Add layout spacing tokens and useLayoutSpacing
grgia 41520ac
Read card padding from useLayoutSpacing in card rows and sections
grgia c0c287b
Lint: ban horizontal inset conditionals on layout flags
grgia 519c9c6
Name the replacement in the layout spacing lint message
grgia e534392
Add the remaining card inset styles to useLayoutSpacing
grgia 27f1f4a
Document the type and layout spacing tokens for contributors and agents
grgia 0e5021d
Read the page gutter token in Spend rows, Table rows and the search i…
grgia 523ab42
Tighten mobile page gutters to 12px and card padding to 24px
grgia 5a3c223
Use a 12px gap between stacked Home cards on mobile
grgia 4bcc8d7
Use the 12px card gap between stacked Section cards on mobile
grgia 9688a4e
Fix lint shorthand in the spacing rule and drop two unused exported t…
grgia cb41d5e
Inset workspace page header button rows at the page gutter on mobile
grgia c3012f2
Start tab pills and the Workspaces tab header at the page gutter
grgia 99e1923
Flag only hand-spelled card insets in the layout spacing rule
grgia 65eb2ce
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia 4e48bce
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia 81a51dd
Restore the proposalPoliceComment action bundle from main
grgia 997a932
Start the table filter bar at the page gutter on mobile
grgia 4677db5
Let Section cards in Rules and bank account flows use the page gutter
grgia c2044af
Keep the 12px tab bar to page-level tab bars
grgia 549d26c
Move the remaining Spend, Domain and expense report insets onto the p…
grgia 45fafcf
Keep two right-hand-panel screens on the plain 20px inset
grgia 5a32648
Name cardPaddingLeft and cardMarginHorizontal in the lint message
grgia 8c3eff5
Update the Chats row highlight test for the page gutter token
grgia 695b8c4
Resolve the expense report list gutter from the same flag as its tabl…
grgia d00cba5
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia abb40cf
Resolve the search skeleton gutter from its effective layout flag
grgia 6483b91
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia ed0aa39
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia fca43bf
Move two insets added on main onto the card padding token
grgia dd12f23
Use the slim gutter for report header buttons and the totals row on m…
grgia 8dd856a
Drop cardMarginHorizontal now its only consumer is gone
grgia 70f774e
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia bdcc3f8
Bring the Insights tab and two skeletons onto the spacing tokens
grgia 3c13c45
Line Section menu rows up with the 24px card padding
grgia 5af0c5e
Say where the menu item padding constant comes from
grgia 59052d2
Fix the last insets a fresh review found on the merged head
grgia 7bd7c8a
Catch padding reads like styles.ph5.paddingHorizontal in the spacing …
grgia c48a095
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia b95c0fe
Read the menu item padding from one constant in both places
grgia 4faf690
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia 77edb7b
Restore the GitHub Action bundles from main
grgia b55b304
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia c86e8c1
Pin the column width test to the wide layout
grgia eba3048
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia c001fff
Move the early renewal offer card rows onto the card padding token
grgia 0e7fd83
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia 86b1e96
Move the Insights dashboard card gap and chart card bottom padding on…
grgia 94fd85c
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia 6e54099
Move the invoicing details card top padding onto the card padding token
grgia e252dc9
Cover cardPaddingTop in the layout spacing test and token lists
grgia 18d0cc0
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia a225011
Point the spacing docs at the hook instead of listing its tokens
grgia 41d7adb
Merge remote-tracking branch 'origin/main' into georgia-spacing-system
grgia File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
59 changes: 59 additions & 0 deletions
59
.claude/skills/app-coding-standards/rules/ui-4-layout-spacing-tokens.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| --- | ||
| ruleId: UI-4 | ||
| title: Type and responsive insets come from tokens | ||
| --- | ||
|
|
||
| ## [UI-4] Type and responsive insets come from tokens | ||
|
|
||
| ### Reasoning | ||
|
|
||
| Per `STYLING.md`, font sizes come from `<Text variant="...">` and paddings or margins that differ between narrow and wide layouts come from `useLayoutSpacing()`. Writing `shouldUseNarrowLayout ? styles.ph5 : styles.ph8` decides the card inset at the call site, so every card row has to agree by hand and a design change means editing dozens of files. `rulesdir/no-raw-typography` and `rulesdir/no-layout-spacing-conditional` enforce both. | ||
|
|
||
| ### Incorrect | ||
|
|
||
| ```tsx | ||
| function CardRow() { | ||
| const {shouldUseNarrowLayout} = useResponsiveLayout(); | ||
| return ( | ||
| <View style={[styles.flexRow, shouldUseNarrowLayout ? styles.ph5 : styles.ph8]}> | ||
| <Text style={{fontSize: 15}}>Title</Text> | ||
| </View> | ||
| ); | ||
| } | ||
| ``` | ||
|
|
||
| ### Correct | ||
|
|
||
| ```tsx | ||
| function CardRow() { | ||
| const {cardPaddingHorizontal} = useLayoutSpacing(); | ||
| return ( | ||
| <View style={[styles.flexRow, cardPaddingHorizontal]}> | ||
| <Text variant="textStrong">Title</Text> | ||
| </View> | ||
| ); | ||
| } | ||
| ``` | ||
|
|
||
| See `useLayoutSpacing`'s implementation for available options. Tokens live in `src/styles/layoutSpacing.ts`; add a new one there rather than writing a ternary. | ||
|
|
||
| --- | ||
|
|
||
| ### Review Metadata | ||
|
|
||
| Flag ONLY when ALL of these are true: | ||
|
|
||
| - The changed code picks a horizontal inset helper by layout flag where the pair spells the card padding values (the `5` step narrow, the `8` step wide, e.g. `styles.ph5 : styles.ph8`, `styles.mhn5 : styles.mhn8`), or sets a raw `fontSize` / `lineHeight` | ||
| - A matching token exists (`cardPadding*`, `pageGutter*`, or a `Text` variant) | ||
|
|
||
| **DO NOT flag if:** | ||
|
|
||
| - The conditional picks vertical spacing only (`pt`, `pb`, `pv`, `mt`, `mb`, `mv`, `gap`) | ||
| - The values are not a card inset or page gutter and no token exists yet (suggest adding one instead) | ||
| - The code is inside `src/styles/` | ||
|
|
||
| **Search Patterns** (hints for reviewers): | ||
| - `shouldUseNarrowLayout ? styles.p` | ||
| - `shouldUseNarrowLayout ? styles.m` | ||
| - `isSmallScreenWidth ? styles.p` | ||
| - `fontSize:` |
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
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
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
113 changes: 113 additions & 0 deletions
113
eslint-plugin-local-rules/no-layout-spacing-conditional.js
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| const name = 'no-layout-spacing-conditional'; | ||
|
|
||
| const meta = { | ||
| type: 'problem', | ||
| docs: { | ||
| description: | ||
| 'Disallow spelling the card inset by hand as a narrow/wide ternary over spacing helpers. Card padding must come from useLayoutSpacing() so every screen agrees on the same values.', | ||
| recommended: 'error', | ||
| }, | ||
| schema: [], | ||
| messages: { | ||
| layoutSpacingConditional: | ||
| '`{{test}} ? styles.{{first}} : styles.{{second}}` spells the card inset by hand. Use the matching style from `useLayoutSpacing()` (src/styles/layoutSpacing.ts) so the value stays in one place.', | ||
| layoutSpacingConditionalKnown: 'Replace `{{test}} ? styles.{{first}} : styles.{{second}}` with `{{replacement}}` from `useLayoutSpacing()` (src/styles/layoutSpacing.ts).', | ||
| }, | ||
| }; | ||
|
|
||
| const NARROW_FLAGS = new Set(['shouldUseNarrowLayout', 'isSmallScreenWidth', 'isExtraSmallScreenWidth']); | ||
| const WIDE_FLAGS = new Set(['isLargeScreenWidth', 'isExtraLargeScreenWidth']); | ||
| const KNOWN_REPLACEMENTS = { | ||
| 'ph5:ph8': 'cardPaddingHorizontal', | ||
| 'p5:p8': 'cardPadding', | ||
| 'mhn5:mhn8': 'cardEdgeToEdge', | ||
| 'pl5:pl8': 'cardPaddingLeft', | ||
| }; | ||
| const SPACING_CLASS_PATTERN = /^([pm](?:h|l|r|hn|ln|rn)?)(\d+)$/; | ||
| const CARD_PADDING_STEPS = {narrow: '5', wide: '8'}; | ||
| const TS_WRAPPER_TYPES = new Set(['TSAsExpression', 'TSSatisfiesExpression', 'TSNonNullExpression', 'TSTypeAssertion']); | ||
|
|
||
| function unwrap(node) { | ||
| if (TS_WRAPPER_TYPES.has(node.type)) { | ||
| return unwrap(node.expression); | ||
| } | ||
| return node; | ||
| } | ||
|
|
||
| function getSpacingClassName(node) { | ||
| let unwrapped = unwrap(node); | ||
| if (unwrapped.type === 'MemberExpression' && !unwrapped.computed && unwrapped.property.type === 'Identifier' && /^(padding|margin)/.test(unwrapped.property.name)) { | ||
| unwrapped = unwrap(unwrapped.object); | ||
| } | ||
| if (unwrapped.type !== 'MemberExpression' || unwrapped.computed || unwrapped.property.type !== 'Identifier') { | ||
| return undefined; | ||
| } | ||
| return SPACING_CLASS_PATTERN.test(unwrapped.property.name) ? unwrapped.property.name : undefined; | ||
| } | ||
|
|
||
| function findSpacingClassName(node) { | ||
| const unwrapped = unwrap(node); | ||
| if (unwrapped.type === 'ArrayExpression') { | ||
| for (const element of unwrapped.elements) { | ||
| const className = element ? findSpacingClassName(element) : undefined; | ||
| if (className) { | ||
| return className; | ||
| } | ||
| } | ||
| return undefined; | ||
| } | ||
| return getSpacingClassName(unwrapped); | ||
| } | ||
|
|
||
| function getLayoutFlag(test) { | ||
| const unwrapped = unwrap(test); | ||
| if (unwrapped.type === 'UnaryExpression' && unwrapped.operator === '!') { | ||
| const inner = getLayoutFlag(unwrapped.argument); | ||
| return inner ? {name: `!${inner.name}`, isNarrowWhenTrue: !inner.isNarrowWhenTrue} : undefined; | ||
| } | ||
| let flagName; | ||
| if (unwrapped.type === 'Identifier') { | ||
| flagName = unwrapped.name; | ||
| } else if (unwrapped.type === 'MemberExpression' && !unwrapped.computed && unwrapped.property.type === 'Identifier') { | ||
| flagName = unwrapped.property.name; | ||
| } | ||
| if (NARROW_FLAGS.has(flagName)) { | ||
| return {name: flagName, isNarrowWhenTrue: true}; | ||
| } | ||
| if (WIDE_FLAGS.has(flagName)) { | ||
| return {name: flagName, isNarrowWhenTrue: false}; | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| function create(context) { | ||
| return { | ||
| ConditionalExpression(node) { | ||
| const flag = getLayoutFlag(node.test); | ||
| if (!flag) { | ||
| return; | ||
| } | ||
| const first = findSpacingClassName(node.consequent); | ||
| const second = findSpacingClassName(node.alternate); | ||
| if (!first || !second) { | ||
| return; | ||
| } | ||
| const narrow = flag.isNarrowWhenTrue ? first : second; | ||
| const wide = flag.isNarrowWhenTrue ? second : first; | ||
| const [, narrowPrefix, narrowStep] = SPACING_CLASS_PATTERN.exec(narrow); | ||
| const [, widePrefix, wideStep] = SPACING_CLASS_PATTERN.exec(wide); | ||
| if (narrowPrefix !== widePrefix || narrowStep !== CARD_PADDING_STEPS.narrow || wideStep !== CARD_PADDING_STEPS.wide) { | ||
| return; | ||
| } | ||
| const replacement = KNOWN_REPLACEMENTS[`${narrow}:${wide}`]; | ||
| const data = {test: flag.name, first, second}; | ||
| if (replacement) { | ||
| context.report({node, messageId: 'layoutSpacingConditionalKnown', data: {...data, replacement}}); | ||
| return; | ||
| } | ||
| context.report({node, messageId: 'layoutSpacingConditional', data}); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| export {name, meta, create}; | ||
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.