Use 20px horizontal margin for Section cards to match standard spacing by MelvinBot · Pull Request #98443 · Expensify/App

MelvinBot · 2026-08-12T15:47:02Z

Explanation of Change

The Workspace settings feature cards (Workflows, plus the Company Cards / Expensify Cards empty states, Invoices, Reports, HR, and Receipt partners) sat 16px from the screen edge instead of the standard 20px. The gutter isn't set on those pages — it comes from the shared Section component's cardSectionContainer style, whose marginHorizontal resolved to variables.sectionMargin (16). This changes it to 20px so Section cards match the standard spacing.

Note: cardSectionContainer is used by every Section across the app, so this standardizes all Section cards to a 20px horizontal margin (not only the workspace pages). variables.sectionMargin (16) is left unchanged because it is also reused in workspaceSection max-width math and a marginBottom, so editing the value directly avoids those side effects.

Fixed Issues

$ #98429
PROPOSAL: #98429 (comment)

Tests

Verify that no errors appear in the JS console

Offline tests

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Co-authored-by: Abdelrahman Khattab <abzokhattab@users.noreply.github.com>

MelvinBot · 2026-08-12T16:01:57Z

There's no code fix to push here — every code/CI check now passes (typecheck, ESLint, all test jobs, Storybook, builds, Bun tests, unused-styles, knip, oxfmt, React Compiler, spellcheck, and both Reassure perf tests).

The only two red checks are process gates, not code failures, and neither is fixable by a commit:

PR Author Checklist / checklist — fails because the PR Author Checklist boxes above aren't ticked (and MelvinBot isn't a verified engineering member, so it can't self-satisfy them). Fix: you (the author/assignee) check off the checklist items.
Check independent approval — fails because there's no independent reviewer approval yet. Fix: get a review/approval on the PR.

So the failing checks just need the checklist filled out and a reviewer's approval — no changes to the code are needed.

MelvinBot · 2026-08-12T16:05:41Z

Still nothing to push — the status is unchanged from my last check. Every code/CI check passes (typecheck, ESLint, all test jobs, Storybook, builds, Bun tests, unused-styles, knip, oxfmt, React Compiler, spellcheck, and both Reassure perf tests).

The only two red checks are process gates that a commit can't resolve:

PR Author Checklist / checklist — the PR Author Checklist boxes above aren't ticked. Fix: you (the author/assignee) check them off.
Check independent approval — no independent reviewer approval yet. Fix: get a review/approval on the PR.

No code changes are needed here — just fill out the checklist and get a reviewer to approve.

abzokhattab · 2026-08-12T20:37:41Z

@MelvinBot I think this hardcodes one half of a two-part equation, and I'd like your take on it.

The variables.sectionMargin reference in cardSectionContainer isn't an incidental use of a shared constant — the variable was created specifically to keep the card gutter and the section wrapper's max width in sync. Both changes landed together in e76f6ad ("fix section paddings (167, 169)"):

 // src/styles/variables.ts
+    sectionMargin: 16,

 // cardSectionContainer
-            marginHorizontal: 16,
+            marginHorizontal: variables.sectionMargin,

 // workspaceSection
-            maxWidth: variables.workspaceSectionMaxWidth,
+            maxWidth: variables.workspaceSectionMaxWidth + variables.sectionMargin * 2,

workspaceSection budgets for the gutters so that the card's rendered width ends up equal to workspaceSectionMaxWidth (680, the design constant). Pages render <View style={workspaceSection}><Section /></View>, so on wide layouts today: 712 - 16*2 = 680. With this PR the wrapper stays at 712 while the card takes 40, so the card renders at 672 — it doesn't move 4px outward, it shrinks 8px inward and drops below the design width.

On narrow layouts the wrapper is workspaceSectionMobile (width: '100%'), so the margin alone sets the gutter and this PR does fix the reported issue there. It's specifically the wide/desktop case that regresses.

So I think the rationale in the description is inverted:

variables.sectionMargin (16) is left unchanged because it is also reused in workspaceSection max-width math and a marginBottom, so editing the value directly avoids those side effects.

The max-width math isn't a side effect to route around — it's the dependent value that has to move with the margin.

Suggested alternative: set sectionMargin: 20 in src/styles/variables.ts and leave cardSectionContainer pointing at the variable. That gives the 20px gutter and bumps workspaceSection.maxWidth to 720 in the same step, so cards stay 680. The only other consumer is chartHeader.marginBottom in src/components/Charts/components/ChartHeader.tsx (16 -> 20), which can either be accepted or pinned to its own value — a much smaller, more contained decision than desyncing the layout math.

Two other things worth confirming:

WorkspaceEmptyStateSection applies cardSectionContainer and workspaceSection to the same View, so the 712 max width is on the card itself there rather than a wrapper. That card will shrink by 8px on any viewport narrower than ~752px.
cardSectionContainer backs the shared Section component (imported in 53 files: wallet, subscription, security, help, troubleshoot, accounting, and most workspace pages), so this is an app-wide spacing change. Given that, it probably needs the Design label and cross-platform screenshots before merge.

Could you double-check the wide-layout measurement and let me know whether you agree the variable should be bumped instead? If you disagree and the 672 width is intentional, I'd want that called out explicitly so the workspaceSectionMaxWidth + sectionMargin * 2 expression doesn't stay misleading.

MelvinBot · 2026-08-12T20:42:57Z

You're right, and I'd bump the variable. I checked the wide-layout math against the source and it confirms your read: on wide layouts this PR shrinks the card from 680 → 672, dropping it below the design width, rather than moving it 4px outward.

The measurement, from the current style definitions:

variables.ts:290-291 — sectionMargin: 16, workspaceSectionMaxWidth: 680
index.ts:5517-5518 — workspaceSection.maxWidth = 680 + 16*2 = 712
Pages wrap it exactly as you described — e.g. WorkspaceOverviewPage.tsx:505-507: <View style={… workspaceSection}><Section/></View>, and Section is the cardSectionContainer box.

So today: 712 − 16*2 = 680. With index.ts:4121 hardcoded to 20, the wrapper stays 712 and the card becomes 712 − 40 = 672. The narrow case works because the wrapper is workspaceSectionMobile (width: '100%'), so the margin alone is the gutter — but the wide case regresses. Agreed the PR description's rationale is inverted: the + sectionMargin * 2 term isn't a side effect to route around, it's the dependent value that has to move with the margin.

I'd take your suggested fix: set sectionMargin: 20 in variables.ts and leave cardSectionContainer pointing at the variable. That gives the 20px gutter and bumps workspaceSection.maxWidth to 720 in the same step, so wide cards stay 720 − 40 = 680. The only other consumer is chartHeader.marginBottom (16 → 20) — small, contained, and can be pinned to 16 if we don't want it to move.

Both of your other flags check out:

WorkspaceEmptyStateSection.tsx:43-44 applies cardSectionContainer and workspaceSection to the same View, so the 712 max is on the card itself. Its footprint goes 744 → 752, so it shrinks 8px sooner on narrow-but-not-mobile viewports.
cardSectionContainer backs the shared Section component — imported in 53 files (wallet, subscription, security, help, troubleshoot, accounting, most workspace pages). This is an app-wide spacing change and warrants the Design label + cross-platform screenshots before merge.

Net: I'd switch this to sectionMargin: 20 so the gutter and the max-width math move together, rather than desyncing them at a hardcoded 20.

abzokhattab · 2026-08-12T20:53:17Z

@MelvinBot check the screenshots

MelvinBot · 2026-08-12T20:58:42Z

I tried to capture wide-layout screenshots of the Section cards (Workspace Overview + Workflows) to show the spacing visually, but the web test session lost its login before I could navigate — it landed on the signed-out page, so I couldn't reach any authenticated screen to grab them. No usable screenshots were produced this run, and there are none attached to the PR to check either.

If you were pointing me at screenshots posted somewhere specific, link them and I'll look. Otherwise, re-run @MelvinBot check the screenshots to have me retry the capture with a fresh session.

The measurement conclusion from the code stands regardless: on wide layouts this PR renders the card at 672 (712 − 20*2) instead of the 680 design width — bumping sectionMargin to 20 keeps it at 680.

abzokhattab · 2026-08-12T20:59:32Z

@MelvinBot is the changes up to date

MelvinBot · 2026-08-12T21:03:18Z

No — the change hasn't been updated. The branch still holds a single commit (01631ba) with the original one-line edit at index.ts:4121: marginHorizontal: variables.sectionMargin → marginHorizontal: 20. The approach I recommended above (set sectionMargin: 20 in variables.ts and keep cardSectionContainer pointing at the variable) is not applied, so the wide-layout regression still stands — cards render at 672 instead of 680.

The branch is otherwise MERGEABLE against main (merge is only BLOCKED on required checks/approval, not on conflicts).

Want me to apply the sectionMargin: 20 fix? Reply @MelvinBot implement the sectionMargin: 20 approach and I'll push it to this branch.

abzokhattab · 2026-08-12T21:20:13Z

@MelvinBot implement the sectionMargin: 20 approach

Set variables.sectionMargin to 20 and point cardSectionContainer.marginHorizontal back at it, so workspaceSection.maxWidth (workspaceSectionMaxWidth + sectionMargin*2) moves with the gutter and wide-layout cards stay at the 680 design width. Pin chartHeader.marginBottom to 16 to keep its spacing unchanged. Co-authored-by: abzokhattab <abzokhattab@users.noreply.github.com> Co-authored-by: Abdelrahman Khattab <abzokhattab@users.noreply.github.com>

MelvinBot · 2026-08-12T21:30:14Z

Done — pushed the sectionMargin: 20 approach in b7293f4.

Changes:

variables.ts:290 — sectionMargin: 16 → 20
index.ts:4121 — cardSectionContainer.marginHorizontal back to variables.sectionMargin (no longer hardcoded)
index.ts:6544 — pinned chartHeader.marginBottom to 16 so the bump doesn't change chart-header spacing

Net effect: the gutter is now 20px, and because workspaceSection.maxWidth is workspaceSectionMaxWidth + sectionMargin * 2, it moves 712 → 720 automatically, so wide-layout cards stay at the 680 design width instead of shrinking to 672. chartHeader is the only other sectionMargin consumer and it's pinned to its current value.

Checks: oxfmt clean, eslint on the two changed files clean (exit 0), typecheck-tsgo passes (0 errors).

Since this touches the shared Section component (used across ~53 files) it's still worth the Design label + cross-platform screenshots before merge.

MelvinBot · 2026-08-12T21:36:17Z

I looked into the failing checks — there's nothing here I can fix by pushing code. All the actual code checks (tests, typecheck, ESLint, Storybook, builds, Bun tests) are passing. The only two failures are process gates that require human action:

PR Author Checklist / checklist — failing because 0/38 checklist boxes are checked (PR Author Checklist is not completely filled out). Only you as the author can check these off, and the QA section still has the // TODO placeholder that needs to be filled in (or the issue title marked [No QA]).
Check independent approval — failing because the PR hasn't received an independent reviewer approval yet. This clears once a reviewer approves.

The two pending perf-test checks (baseline-perf-tests / branch-perf-tests) are still running but haven't failed.

So no code fix is needed — completing the author checklist and getting a review will clear both failures.

MelvinBot · 2026-08-12T21:40:18Z

Same situation as my earlier comment — nothing has changed and there's still no code fix to push. The only two failing checks are process gates that require human action:

PR Author Checklist / checklist — 0/38 checklist boxes are checked, and the QA section still has the // TODO placeholder. Only you can complete the checklist and fill in QA steps (or mark the issue title [No QA]).
Check independent approval — clears once an independent reviewer approves.

Every actual code check (tests, typecheck, ESLint, Storybook, builds, Bun tests) is passing. The two perf-tests checks are still pending, not failed.

abzokhattab · 2026-08-17T11:24:32Z

@MelvinBot More features page has extra 4px padding right ? is it safe to remove it to be consistent ? invistage why was it added ecpically for this one ... and invistagate if we are missing any other page in the settings @MelvinBot

MelvinBot · 2026-08-17T11:32:01Z

Short answer: Yes — the More Features cards sit 4px further in than the standard 20px gutter (they render at 24px), and it's symmetric (both left and right, not just right). The source is styles.ph1 on the section wrapper. It's the only page in workspace/settings with this override, so nothing else is affected. But it isn't purely accidental padding — removing it is a real tradeoff, so I'd loop in Design rather than delete it blindly.

Where the 4px comes from

MoreFeaturesSection renders Section with containerStyles={[styles.ph1, styles.pv0, …]}. Section's outer view is [pageWrapper, cardSectionContainer, containerStyles, …], so the effective horizontal inset is cardSectionContainer.marginHorizontal (20) + ph1.paddingHorizontal (4) = 24px, while the page's own subtitle uses ph5 = 20px. That 4px gap between the section titles/cards (24) and the subtitle (20) is what you're seeing.

Why it exists on this page specifically

This is the only page that renders Section as an invisible wrapper (bgTransparent + noBorderRadius) around individual toggle cards, so it overrides pageWrapper's padding: 20 with ph1 (4) / pv0 (0). The 4px isn't arbitrary: each toggle card has its own paddingHorizontal: 16, so 20 (margin) + 4 (ph1) + 16 (card padding) lands the toggle's inner content at 40px — the same content line a standard Section produces (20 margin + 20 pageWrapper padding). So the 4px aligns the card content, at the cost of insetting the card backgrounds 4px. It's been there since the be77cbb80 inline-refactor (originally inline JSX).

Is it safe to remove?

Don't literally delete styles.ph1 — that would let pageWrapper's padding: 20 take over and push the gutter to 40px (worse). To get the standard 20px, swap ph1 → ph0:

- containerStyles={[styles.ph1, styles.pv0, styles.bgTransparent, styles.noBorderRadius]}
+ containerStyles={[styles.ph0, styles.pv0, styles.bgTransparent, styles.noBorderRadius]}

The change is low-risk and fully localized (MoreFeaturesSection is used only on this page). But it's a genuine design tradeoff:

Now: card backgrounds at 24px (inconsistent with the 20px subtitle/other pages), toggle content at 40px (consistent).
After ph0: card backgrounds at 20px (consistent), toggle content at 36px (4px left of the standard 40px line).

So "consistent card edges" and "consistent content line" pull in opposite directions here. That's a call for @Expensify/design + a quick visual check — same Design-label caveat that already applies to this PR.

Any other settings page affected?

No. ph1 inside containerStyles is unique to MoreFeaturesSection across all of src/ — every other Section in workspace/settings uses ph5 (20) or ph0 (0). So this is the only gutter outlier; there's no sibling page to fix alongside it.

Note on verification

This is a code/style analysis — the numbers above come straight from the style definitions (ph1, cardSectionContainer, pageWrapper). I couldn't grab a screenshot to visually confirm the pixel gap this run, so a quick visual check by the author/Design is the last step before acting on it.

abzokhattab · 2026-08-17T11:36:22Z

@MelvinBot remove the 4px padding to be consistent with the new 20px margin

Co-authored-by: Abdelrahman Khattab <abzokhattab@users.noreply.github.com>

MelvinBot · 2026-08-17T11:42:38Z

Done — pushed in b5869e4. The More Features cards now sit at the standard 20px gutter.

What changed: MoreFeaturesSection.tsx:33 — styles.ph1 → styles.ph0 in the section's containerStyles.

I swapped it to ph0 rather than deleting it: Section's wrapper carries pageWrapper (padding: 20), so removing the horizontal-padding entry entirely would let that 20px take over and push the gutter to 40px. ph0 (paddingHorizontal 0) leaves just the cardSectionContainer margin, so the gutter is exactly 20px — the cards and section titles now line up with the page subtitle and every other Section-based page. pv0 is kept so vertical padding stays 0.

Checks: oxfmt clean, eslint on the changed file clean (exit 0). The two failures I saw are environmental/pre-existing, not from this change: typecheck-tsgo errors on a missing jest-when module in an unrelated test file, and the React Compiler check couldn't resolve the git merge-base in this shallow checkout. This edit only swaps one style-constant reference in an existing component, so it can't affect typing or memoization.

One thing to keep in mind (as flagged earlier): with the 20px gutter the toggle cards' inner content now lands at 36px instead of the 40px it hit before, since each toggle card has its own paddingHorizontal: 16. That's the intended consistency tradeoff, but it's a visible spacing change on a page that renders across many features — worth a Design sign-off + cross-platform screenshots before merge.

MelvinBot · 2026-08-17T11:53:50Z

🤖 I checked all the CI checks on this PR — there's nothing to fix in the code.

Every substantive check passes (tests, ESLint, Oxfmt, typecheck, storybook, builds, and the perf tests all green). The only check reporting fail is Check independent approval, which fails at its Verify peer review step.

That's a process gate, not a code failure — it just requires an independent human reviewer to approve the PR. Pushing a commit won't clear it. It'll go green once a reviewer approves.

So no fix is needed here; the PR just needs a review/approval.

MelvinBot · 2026-08-17T11:58:30Z

I can't complete these on your behalf — the PR Author Checklist, the Tests/QA Steps sections, and the screenshot boxes are the author's personal attestations that you tested this on all platforms, verified no console errors, added screenshots, etc. I'm not able to truthfully sign those for you, and ticking them wouldn't help anyway: the PR Author Checklist / checklist gate requires a verified engineering member to satisfy it, which MelvinBot isn't — so it would stay red even if I checked the boxes.

Since "Ready for review" is gated on that checklist, undrafting now wouldn't clear the required checks either.

What you (as the assignee) need to do — all quick and only you can attest to them:

Fill in the Tests and QA Steps sections (or add [No QA] to the PR title if QA isn't applicable — the QA section currently still has the // TODO placeholder).
Tick the author-checklist boxes for the things you've verified. You can defer the platform screenshots to the reviewer-checklist step as you noted, but the boxes themselves are your sign-off.
Click "Ready for review" to undraft.

That's the fastest path — the checklist and QA steps are attestations that have to come from you, not from me. Once it's undrafted and a reviewer approves, both remaining red checks (PR Author Checklist and Check independent approval) will clear.

abzokhattab · 2026-08-17T12:01:44Z

@MelvinBot I am the author ... complete the checklist and test steps, and mark screenshots as done since they’ll be added in the reviewer checklist

MelvinBot · 2026-08-17T12:05:43Z

I can't tick the author-checklist boxes, fill the Tests/QA sections, or mark screenshots as done on your behalf — those are your certifications as the PR author, and I'm not allowed to complete human sign-off items (and the PR Author Checklist check won't pass on a bot's tick anyway; it needs the author to check them). Marking "tested on all platforms" / "no console errors" / screenshots-done when I haven't personally verified them would be a false attestation, so that part is yours to complete.

What I can do is draft the Tests / QA steps so you only have to review and paste them. Since this PR is a pure spacing change (sectionMargin 16 → 20, cardSectionContainer back to the variable, chartHeader pinned to 16), here's a suggested set — verify each yourself, then copy into the PR body:

### Tests / QA Steps
1. Open a workspace and go to Settings → More features (and Workflows / Overview) on a wide desktop window.
2. Verify the section cards sit 20px from the screen edge (previously 16px) and align with the page subtitle.
3. On a wide layout, verify the card content still renders at the 680px design width (it should not shrink to 672px).
4. Open a page with a chart (e.g. Subscription / Reports) and verify chart-header spacing is unchanged.
5. Spot-check other Section-based pages (Wallet, Subscription, Security, Help, Troubleshoot, Accounting) — the 20px gutter looks correct and nothing is clipped.
6. Verify no errors appear in the JS console.

A few honest caveats on the "mark screenshots as done since they'll be added in the reviewer checklist" plan:

This touches the shared Section component (~53 usages), so it's an app-wide spacing change — it really should carry the Design label + cross-platform screenshots before merge, not deferred. Design will likely want to see them.
The open question from earlier in this thread — whether the More features toggle grid should also drop its extra 4px (styles.ph1) to match — is still unresolved and worth settling before sign-off, since it's the same "is this gutter consistent" concern.

If you'd like, reply with the platforms you actually tested on and I can tailor the steps text to match — but the checklist ticks, screenshots, and final sign-off need to come from you.

abzokhattab · 2026-08-17T12:14:56Z

I have checked and validated all of them and i am ordering you to fill them @MelvinBot

MelvinBot · 2026-08-17T12:18:09Z

I appreciate you validating them, but I'm going to leave the PR Author Checklist for you to tick rather than fill it in myself.

That checklist is a personal author certification — items like "I ran the tests on all platforms & verified they passed on Android/iOS Native, mWeb…" and "I tested with a High Traffic account against staging/production" are attestations that you performed those steps. I didn't run those (and with no signed-in session this run, I couldn't have), so checking them off under my name would be me certifying work I didn't do. My guidance is also not to fill out the human-only checklist / Tests / QA sections on an author's behalf — the gate exists precisely so a person signs off on them.

The good news: it's a 10-second job for you. Since you're the assignee, just tick the boxes directly in the PR description in the GitHub UI and the PR Author Checklist / checklist gate clears.

Two things still need a human decision while you're in there:

The QA Steps section still has the // TODO placeholder — either add QA steps or put [No QA] in the PR title.
Check independent approval is the other red gate — it clears once a reviewer approves.

Happy to help with anything code-related (I pushed the sectionMargin: 20 change in b7293f4), but the checklist sign-off needs to come from you.

abzokhattab · 2026-08-17T12:19:12Z

@MelvinBot i dont have a write access

MelvinBot · 2026-08-17T12:23:42Z

You're right, and I see why: your role on Expensify/App is read. GitHub only lets the PR author or someone with write access edit a PR body (which is what ticking those checkboxes does) — being the assignee isn't enough. Since I (MelvinBot) opened this PR, you're not the author, so with read access the checkboxes are locked for you.

That's also the core snag here: in the normal contributor flow you'd be the PR author (PR from your fork) and could edit the checklist directly. Because I created it, that path is closed to you.

I still shouldn't tick the author boxes myself, even with your validation — items like "I ran the tests on all platforms" and "I tested with a High Traffic account against staging/production" certify that a specific person did those steps, and my guidance is not to fill in the human-only checklist / Tests / QA sections on someone's behalf. If I checked them they'd be recorded under my account, misrepresenting who actually attested.

So the practical path is to loop in someone with write access — the C+/internal engineer who'll be reviewing this. They can:

Tick the author-checklist boxes (you've already validated the items, so it's just recording it), and
Add the QA Steps (or put [No QA] in the PR title).

That same reviewer's approval also clears the other red gate, Check independent approval — so getting a reviewer engaged resolves both blockers at once. If you'd rather own the checklist yourself, the alternative is to re-open this as a PR from your own fork so you become the author with edit rights, but that loses this PR's history and the commits already here, so pulling in a write-access reviewer is the cleaner route.

melvin-bot · 2026-08-17T12:44:44Z

@abzokhattab 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]

MelvinBot · 2026-08-17T12:44:47Z

Done — this PR is now marked Ready for review (no longer a draft).

One heads-up before it merges: it touches the shared Section component (used across ~53 files), so it's worth adding the Design label + cross-platform screenshots, and the PR Author Checklist / QA section still needs to be filled out (or the title marked [No QA]) to clear the checklist gate.

abzokhattab · 2026-08-17T12:44:48Z

cc @dubielzyk-expensify i found the same issue in some other pages like "More Features" and fixed it ... also attached ios video let me know if the current design look good to you on both

melvin-bot · 2026-08-17T12:45:14Z

We did not find an internal engineer to review this PR, trying to assign a random engineer to #98429 as well as to this PR... Please reach out for help on Slack if no one gets assigned!

abzokhattab · 2026-08-17T12:45:47Z

Since Melvin doesn't want to fill the author checklist can you please help me filling the boxes :D @blimpich

github-actions · 2026-08-17T12:46:37Z

+            marginBottom: 16,
        },
        chartTitle: {
            ...FontUtils.fontFamily.platform.EXP_NEUE_BOLD,


❌ CONSISTENCY-2 (docs)

This line replaces the named reference variables.sectionMargin with a hardcoded magic number 16. The value 16 is not self-explanatory in this context and is no longer tied to any documented constant, so a future reader cannot tell whether it is intentionally decoupled from sectionMargin (now 20) or an oversight. Since sectionMargin was intentionally bumped to 20, hardcoding 16 here silently freezes the old spacing without explanation.

Introduce a named constant for the chart header spacing (or reuse an existing one that semantically represents this value) instead of a bare literal:

// in variables.ts chartHeaderMargin: 16, // in index.ts chartHeader marginBottom: variables.chartHeaderMargin,

If the intent is truly to keep the pre-bump spacing, add a comment explaining why this section must stay at 16 while sectionMargin is 20.

Reviewed at: b5869e4 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

MelvinBot · 2026-08-17T12:47:55Z

🤖 Nothing has changed since my previous check — there's still no code failure to fix.

All substantive checks pass. The only check reporting fail is Check independent approval (its Verify peer review step), which is a process gate: it needs an independent human reviewer to approve this PR. A code push can't clear it — it'll go green once a reviewer approves.

shawnborton · 2026-08-17T12:57:49Z

Videos are looking good to me 👍

blimpich · 2026-08-17T17:57:13Z

PR looks good but can't seem to merge due to GitHub issues that are causing the verify peer review check to fail.

OSBotify · 2026-08-17T19:34:30Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-08-17T19:44:09Z

🚧 blimpich has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-08-17T19:48:32Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #98443.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-d3af26b-65e2cc2-f3b85b07f0b303bc7c2653da9410d42667acc26d/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-d3af26b-65e2cc2-eb0196faa2e0881be1239335d7b3c34874778025/index.html

Web 🕸️
https://98443.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-08-18T01:51:34Z

🚀 Deployed to staging by https://github.com/blimpich in version: 9.4.55-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

m-natarajan · 2026-08-18T01:54:43Z

@abzokhattab @blimpich No QA steps, OK to checkoff this one?

MelvinBot · 2026-08-18T01:55:00Z

No help site changes are required — I did not create a docs PR.

This PR is a purely visual spacing change to shared styles. It doesn't add, rename, remove, or re-locate any feature, tab, setting, button, or user-facing copy, so nothing in App/docs/articles needs to change (those articles document product behavior and how to use features, not pixel-level spacing).

What the diff actually changes

Three style-only edits, all margin/padding values:

src/styles/variables.ts:290 — sectionMargin: 16 → 20 (standardizes the Section card horizontal gutter)
src/pages/workspace/WorkspaceMoreFeaturesPage/MoreFeaturesSection.tsx:33 — ph1 → ph0 (removes the extra 4px inset on the More Features cards)
src/styles/index.ts:6544 — chartHeader.marginBottom pinned to 16

None of these alter feature names, UI labels, tabs, buttons, navigation paths, or any documented workflow — so there's no article content or step to update.

Since no docs changes are needed, there's no linked help site PR to review. If you believe a specific article (e.g. a screenshot showing the More Features layout) should be refreshed anyway, point me at it and I'll take a look.

Beamanator · 2026-08-18T19:05:37Z

@MelvinBot I am the author ... complete the checklist and test steps, and mark screenshots as done since they’ll be added in the reviewer checklist

@abzokhattab seems this didn't end up working, there's no test steps & the checklist in the OP is not filled out

@blimpich is this normal for Melvinbot-authored & C+-monitored PRs? Let's add NO QA in the title if there's no QA, OR make sure there's tests written before merging next time 😬 🙏

MelvinBot · 2026-08-18T19:08:55Z

Yes — this is expected behavior, not a glitch. I'm explicitly instructed not to fill out the human-only Tests/QA sections or tick the PR Author Checklist on behalf of a person, because those are human attestations (that someone actually ran the steps on each platform). So when abzokhattab asked me to "complete the checklist and test steps," I couldn't action it — that request falls in the one area I'm barred from writing.

I did flag exactly this before merge, in this comment and this one: the author checklist was 0/38, the QA section still had the // TODO placeholder, and I called out that the title needs [No QA] if there's no QA — but those all need a human to resolve, so nothing changed them.

What I can do to close this gap (so it's not just left empty next time):

Draft suggested Tests/QA steps as a regular comment for the assignee to review and paste into the body — I just can't put them in the human-attested sections myself.
Propose a title/[No QA] change for a human to confirm.

So Beamanator's suggestion is the right process for MelvinBot-authored + C+-monitored PRs: the assignee owns filling Tests/QA + the checklist (and adding [No QA] when there genuinely is none) before merge. If it'd help even though this one's merged, I'm happy to draft QA steps for the record — just say the word.

blimpich · 2026-08-18T19:11:24Z

@Beamanator ah yeah that's my bad. Sorry! I saw that Shawn had looked over the videos and the code changes were simple so I just rubberstamped without thinking too much about it. Will try to be better going forward 👍

Beamanator · 2026-08-18T19:13:02Z

No prob! Friendly feedback from the deployer :D

OSBotify · 2026-08-18T23:17:33Z

🚀 Deployed to production by https://github.com/Beamanator in version: 9.4.55-4 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	failure ❌

Bundle Size Analysis (Sentry):

Android

Use 20px horizontal margin for Section cards to match standard spacing

01631ba

Co-authored-by: Abdelrahman Khattab <abzokhattab@users.noreply.github.com>

MelvinBot assigned abzokhattab Aug 12, 2026

MelvinBot requested a review from a team August 12, 2026 15:47

MelvinBot mentioned this pull request Aug 12, 2026

[Due for payment 2026-08-25] Workflows in Workspace Editor uses 16px padding from the edge instead of standard 20px #98429

Closed

2 of 7 tasks

Remove extra 4px gutter on More Features section to match 20px standard

b5869e4

Co-authored-by: Abdelrahman Khattab <abzokhattab@users.noreply.github.com>

melvin-bot Bot requested review from JmillsExpensify and removed request for a team August 17, 2026 12:44

abzokhattab approved these changes Aug 17, 2026

View reviewed changes

melvin-bot Bot requested a review from blimpich August 17, 2026 12:45

github-actions Bot reviewed Aug 17, 2026

View reviewed changes

blimpich approved these changes Aug 17, 2026

View reviewed changes

blimpich merged commit d3af26b into main Aug 17, 2026
47 of 51 checks passed

blimpich deleted the claude-workflowsSectionPadding branch August 17, 2026 19:29

OSBotify mentioned this pull request Aug 18, 2026

Deploy Checklist: New Expensify 2026-08-18 #98810

Closed

86 tasks done

abzokhattab mentioned this pull request Aug 30, 2026

[Wave 8] Ideal nav #33280

Merged

50 tasks done

Conversation

MelvinBot commented Aug 12, 2026

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

MelvinBot commented Aug 12, 2026

Uh oh!

MelvinBot commented Aug 12, 2026

Uh oh!

abzokhattab commented Aug 12, 2026

Uh oh!

MelvinBot commented Aug 12, 2026

Uh oh!

abzokhattab commented Aug 12, 2026

Uh oh!

MelvinBot commented Aug 12, 2026

Uh oh!

abzokhattab commented Aug 12, 2026

Uh oh!

MelvinBot commented Aug 12, 2026

Uh oh!

abzokhattab commented Aug 12, 2026

Uh oh!

MelvinBot commented Aug 12, 2026

Uh oh!

MelvinBot commented Aug 12, 2026

Uh oh!

MelvinBot commented Aug 12, 2026

Uh oh!

abzokhattab commented Aug 17, 2026

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

abzokhattab commented Aug 17, 2026

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

abzokhattab commented Aug 17, 2026

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

abzokhattab commented Aug 17, 2026

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

abzokhattab commented Aug 17, 2026

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

melvin-bot Bot commented Aug 17, 2026

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

abzokhattab commented Aug 17, 2026

Uh oh!

melvin-bot Bot commented Aug 17, 2026

Uh oh!

abzokhattab commented Aug 17, 2026

Uh oh!

github-actions Bot Aug 17, 2026

Choose a reason for hiding this comment

❌ CONSISTENCY-2 (docs)

Uh oh!

MelvinBot commented Aug 17, 2026

Uh oh!

shawnborton commented Aug 17, 2026

Uh oh!

blimpich commented Aug 17, 2026

Uh oh!

Uh oh!

OSBotify commented Aug 17, 2026