Skip to content

fix(feed): give every feed layout a horizontal gutter - #6516

Draft
tsahimatsliah wants to merge 8 commits into
mainfrom
fix/feed-side-padding
Draft

fix(feed): give every feed layout a horizontal gutter#6516
tsahimatsliah wants to merge 8 commits into
mainfrom
fix/feed-side-padding

Conversation

@tsahimatsliah

@tsahimatsliah tsahimatsliah commented Aug 24, 2026

Copy link
Copy Markdown
Member

The problem

Feed cards run flush into the sidebar on one side and the window edge on the other — no gap between the grid and the frame.

Measured on production daily.dev/posts at 956px:

grid left  64  = sidebar right edge  → gap 0
grid right 956 = viewport width      → gap 0

The cause

The page container the feed sits in forces !px-0 (FeedPageLayoutList), which overrides the px-4 tablet:px-8 it inherits from BasePageContainer. That makes the className passed to FeedContainer in MainFeedLayout the only horizontal inset the feed gets.

That inset was shouldUseListFeedLayout && !isFinder && 'laptop:px-6' — scoped both to the laptop breakpoint and to the list layout. So every other case had none:

  • any width below laptop, in either layout
  • the grid layout, at any width

The fix

Every non-finder feed gets px-4 tablet:px-6. Laptop keeps the 24px it already had, so the only change is adding a gutter where there was none. Search results stay flush, as before.

Notes

Found while reviewing the sponsored strip mock-up in #6490, which drew a line along the bottom of the feed and made the missing gutter obvious. This fix is independent of that work and is also applied there so the demo isn't blocked on this merging.

🤖 Generated with Claude Code

Preview domain

https://fix-feed-side-padding.preview.app.daily.dev

Feed cards ran flush into the sidebar on one side and the window edge
on the other: no gap at all between the grid and the frame.

The page container the feed sits in forces `!px-0` (FeedPageLayoutList),
so the className passed to FeedContainer is the only horizontal inset
the feed gets — and it was scoped to `laptop:` and to the list layout.
Every other case (any width below laptop, and the grid layout at any
width) therefore had none.

Every non-finder feed now gets the same `px-4 tablet:px-6`. Laptop
keeps the 24px it already had, so the only change is adding a gutter
where there was none. Search results stay flush, as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
daily-webapp Ready Ready Preview Aug 27, 2026 1:01pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
storybook Ignored Ignored Aug 27, 2026 1:01pm

Request Review

tsahimatsliah added a commit that referenced this pull request Aug 24, 2026
Feed cards ran flush into the sidebar on one side and the window edge
on the other: no gap at all between the grid and the frame.

The page container the feed sits in forces `!px-0` (FeedPageLayoutList),
so the className passed to FeedContainer is the only horizontal inset
the feed gets — and it was scoped to `laptop:` and to the list layout.
Every other case (any width below laptop, and the grid layout at any
width) therefore had none.

Every non-finder feed now gets the same `px-4 tablet:px-6`. Laptop
keeps the 24px it already had, so the only change is adding a gutter
where there was none. Search results stay flush, as before.

Cherry-picked from #6516, which carries this fix on its own against
main. Duplicated here so the demo is not blocked on that merging; the
change is byte-identical, so the merge is a no-op once #6516 lands.

Mockup-to-eng-pass: 1
The gutter added to the feed left the chrome above it behind: cards
sat 24px in while the Explore breadcrumbs and the sort tabs stayed
flush against the sidebar, so the column had two different left edges.

Both come from FeedExploreHeader, whose `className.container` wraps
the breadcrumbs and the tab strip together, so one prop settles both —
on the laptop header and on the mobile sticky one.

The gutter is now a single `feedGutter` constant used by all three
call sites, because three copies of the same padding string is how
they drift apart again.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tsahimatsliah added a commit that referenced this pull request Aug 24, 2026
The gutter added to the feed left the chrome above it behind: cards
sat 24px in while the Explore breadcrumbs and the sort tabs stayed
flush against the sidebar, so the column had two different left edges.

Both come from FeedExploreHeader, whose `className.container` wraps
the breadcrumbs and the tab strip together, so one prop settles both —
on the laptop header and on the mobile sticky one.

The gutter is now a single `feedGutter` constant used by all three
call sites, because three copies of the same padding string is how
they drift apart again.

Cherry-picked from #6516. Byte-identical, so that PR merging is a
no-op here.

Mockup-to-eng-pass: 1
The breadcrumbs started flush against the site header — 0px above
them — and then sat 16px clear of the tab strip, so the two read as
one loose block instead of a heading with its tabs under it.

16px above the breadcrumbs, 8px between them and the tabs. The 16px
from the tabs down to the cards is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tsahimatsliah added a commit that referenced this pull request Aug 24, 2026
The breadcrumbs started flush against the site header — 0px above
them — and then sat 16px clear of the tab strip, so the two read as
one loose block instead of a heading with its tabs under it.

16px above the breadcrumbs, 8px between them and the tabs. The 16px
from the tabs down to the cards is unchanged.

Cherry-picked from #6516. Byte-identical, so that PR merging is a
no-op here.

Mockup-to-eng-pass: 1
The previous commits added a gutter on FeedContainer while FeedPage
was already applying `laptop:p-10` in the legacy layout. Both were in
the same ancestry, so they stacked — and because `isV2` resolves after
mount, which of them applied changed as the flag settled. The feed
visibly jumped between a 64px inset and a 24px one depending on
whether you loaded the page, refreshed it, or navigated to it.

FeedPage now sets the inset for both variants and is the only place
that does: `feedGutterV2` under v2, the existing `pageMainClassNames`
otherwise. v2 previously set nothing at all, which is what left the
cards flush against the frame.

The breadcrumbs and tab strip sit outside FeedPage, so they take the
same value through `useFeedGutter` rather than a hardcoded copy — the
value depends on a flag that resolves after mount, so a literal at the
call site is wrong by construction. It is in the memo's deps for the
same reason: a stale gutter there would reintroduce exactly the
mismatch this fixes.

FeedContainer goes back to its original className.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tsahimatsliah added a commit that referenced this pull request Aug 24, 2026
The previous commits added a gutter on FeedContainer while FeedPage
was already applying `laptop:p-10` in the legacy layout. Both were in
the same ancestry, so they stacked — and because `isV2` resolves after
mount, which of them applied changed as the flag settled. The feed
visibly jumped between a 64px inset and a 24px one depending on
whether you loaded the page, refreshed it, or navigated to it.

FeedPage now sets the inset for both variants and is the only place
that does: `feedGutterV2` under v2, the existing `pageMainClassNames`
otherwise. v2 previously set nothing at all, which is what left the
cards flush against the frame.

The breadcrumbs and tab strip sit outside FeedPage, so they take the
same value through `useFeedGutter` rather than a hardcoded copy, and
it is in the memo's deps so it cannot go stale.

Cherry-picked from #6516. Byte-identical, so that PR merging is a
no-op here.

Mockup-to-eng-pass: 1
`isV2` is not stable across client-side navigation: it reads true on
load and false after a route change. Any inset keyed to it therefore
changed as you moved around the app — the feed sat 24px in on arrival
and 40px in after clicking a tab, and refreshing "fixed" it by landing
on the other state.

The feed now keeps one inset, `pageMainClassNames`, whatever the
layout experiment is doing. v2's special case is gone: it set nothing
at all, which is what left the cards flush against the frame to begin
with.

The breadcrumbs and tab strip take the horizontal half of the same
value through the `feedGutter` constant, so they cannot drift from the
cards.

The instability in `useLayoutVariant` is untouched and still worth a
look on its own — this change just stops the feed's width depending
on it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tsahimatsliah added a commit that referenced this pull request Aug 24, 2026
`isV2` is not stable across client-side navigation: it reads true on
load and false after a route change. Any inset keyed to it therefore
changed as you moved around the app — the feed sat 24px in on arrival
and 40px in after clicking a tab, and refreshing "fixed" it by landing
on the other state.

The feed now keeps one inset, `pageMainClassNames`, whatever the
layout experiment is doing. v2's special case is gone: it set nothing
at all, which is what left the cards flush against the frame.

The breadcrumbs and tab strip take the horizontal half of the same
value through the `feedGutter` constant.

Cherry-picked from #6516. Byte-identical, so that PR merging is a
no-op here.

Mockup-to-eng-pass: 1
…share

The feed renders through two different page containers depending on
layout and route: FeedPage, which carries `pageMainClassNames`, and
FeedPageLayoutList, which forces `!px-0`. Which one you get flips with
`isV2` — itself unstable across client-side navigation — so an inset
placed on either covered some routes and not others, and stacked with
the other where both applied. That is the whole bug: 64px on one
arrival, 24px on another, 0px after a route change.

FeedContainer is the only element common to both, so the inset lives
there now and nowhere else. FeedPage keeps its vertical padding and
gives up its horizontal, which it could not apply everywhere anyway.

One value, `px-4 tablet:px-6 laptop:px-10`, for every feed route and
both layout variants — 40px at laptop, matching what the legacy path
already gave. The breadcrumbs and tab strip share the constant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tsahimatsliah added a commit that referenced this pull request Aug 24, 2026
…share

The feed renders through two different page containers depending on
layout and route: FeedPage, which carries `pageMainClassNames`, and
FeedPageLayoutList, which forces `!px-0`. Which one you get flips with
`isV2` — itself unstable across client-side navigation — so an inset
placed on either covered some routes and not others, and stacked with
the other where both applied. That is the whole bug: 64px on one
arrival, 24px on another, 0px after a route change.

FeedContainer is the only element common to both, so the inset lives
there now and nowhere else. FeedPage keeps its vertical padding and
gives up its horizontal, which it could not apply everywhere anyway.

One value, `px-4 tablet:px-6 laptop:px-10`, for every feed route and
both layout variants — 40px at laptop, matching what the legacy path
already gave. The breadcrumbs and tab strip share the constant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit a366dd8)

Cherry-picked from #6516. Byte-identical, so that PR merging is a
no-op here.

Mockup-to-eng-pass: 1
Explore showed a large gap above the breadcrumbs on arrival that a
reload "fixed" — the same shape of bug as the horizontal inset, in the
other axis.

`disableTopPadding` was keyed to `shouldUseListFeedLayout`, and that
value is not stable: `enableSsrSafeLayout` forces the list layout
before mount and hands over to the real one after, so the page's 40px
top padding was present in one state and zeroed in the other. The
header sat 104px down or 64px down depending on how you arrived.

Explore now keeps that padding in both layouts. It is the one feed
that renders a breadcrumb and tab header above the cards, and with the
padding zeroed that header is jammed against the site header.

The `pt-4` added earlier to the Explore header goes away with it. It
was compensating for this missing padding, which is why the gap read
as too large in the state where both applied. Production has no such
compensation, and Explore now matches it: 104px to the breadcrumbs,
64px of header offset plus the page's 40px.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tsahimatsliah added a commit that referenced this pull request Aug 24, 2026
Explore showed a large gap above the breadcrumbs on arrival that a
reload "fixed" — the same shape of bug as the horizontal inset, in the
other axis.

`disableTopPadding` was keyed to `shouldUseListFeedLayout`, and that
value is not stable: `enableSsrSafeLayout` forces the list layout
before mount and hands over to the real one after, so the page's 40px
top padding was present in one state and zeroed in the other. The
header sat 104px down or 64px down depending on how you arrived.

Explore now keeps that padding in both layouts. It is the one feed
that renders a breadcrumb and tab header above the cards, and with the
padding zeroed that header is jammed against the site header.

The `pt-4` added earlier to the Explore header goes away with it. It
was compensating for this missing padding, which is why the gap read
as too large in the state where both applied. Production has no such
compensation, and Explore now matches it: 104px to the breadcrumbs,
64px of header offset plus the page's 40px.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cherry-picked from #6516. Byte-identical, so that PR merging is a
no-op here.

Mockup-to-eng-pass: 1
@tsahimatsliah
tsahimatsliah marked this pull request as draft August 26, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants