Restore mobile header layout + fix desktop spacing#821
Conversation
Stats grid and CTA render in two responsive positions: - Desktop (sm+): inside flex-1 column next to cover (hidden on mobile) - Mobile: full-width below cover+info row (hidden on desktop) Also: sm:gap-6 between cover and info, mt-6 between info rows and stats on desktop for more breathing room. Fixes #820 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The responsive split fixes the main layout regression, and the desktop spacing changes are correct. One explicit acceptance criterion from issue #820 is still unmet: the mobile CTA is placed below the stats, but it is not full-width.
Findings
- [medium] The mobile CTA still renders as the shared inline-width
AddPlotButton, so it does not satisfy issue #820's requirement that the mobile button be full-width below the stats grid.- File:
src/components/AddPlotButton.tsx:46 - File:
src/app/story/[storylineId]/page.tsx:393 - Suggestion: Let the mobile usage opt into a full-width button or render a mobile-specific class variant instead of reusing the inline-width desktop button unchanged.
- File:
Decision
Requesting changes until the mobile CTA matches the full-width layout required by issue #820.
Apply w-full to AddPlotButton's anchor/div children on mobile via parent selector [&_a]:w-full [&_div]:w-full. Desktop keeps inline-block auto-width. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up change resolves my remaining mobile-layout blocker. The mobile CTA now gets full-width treatment through the mobile-only wrapper, while the desktop path keeps the inline-width button and the responsive stats placement remains intact.
Findings
- [resolved] The mobile CTA now matches issue #820's full-width requirement.
- File:
src/app/story/[storylineId]/page.tsx:393 - Suggestion: None.
- File:
Decision
Approving. My prior blocker on the mobile CTA width is addressed.
2 participants
Summary
Fixes mobile layout regression from #819 and adds desktop spacing improvements.
Approach: Responsive dual rendering
Stats grid and CTA button render in two positions using
hidden/sm:blockandsm:hidden:flex-1column next to cover —hidden sm:blocksm:hiddenStats content is extracted into a variable (
statsGrid) to avoid JSX duplication.Spacing fixes
gap-4→gap-4 sm:gap-6(24px on desktop)mt-3→mt-6on desktop for more breathing roomTest Plan
Fixes #820
🤖 Generated with Claude Code