Skip to content

fix(web): make the homepage terminal preview more compact - #5889

Merged
Hmbown merged 2 commits into
mainfrom
fix/site-terminal-size-20260905
Sep 5, 2026
Merged

fix(web): make the homepage terminal preview more compact#5889
Hmbown merged 2 commits into
mainfrom
fix/site-terminal-size-20260905

Conversation

@Hmbown

@Hmbown Hmbown commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

The homepage terminal stretched across the full desktop content column and overwhelmed the introduction. Cap it at 896px and center it; keep mobile full width and preserve the screenshot's aspect ratio. Match the responsive image sizes to the rendered width.

Testing

  • Website npm test: 407 passed, 0 failed across 47 files.
  • Website npm run lint: no errors; two existing image warnings in untouched navigation/footer components.
  • Browser verified at 1280px (896px terminal) and 390px (343px terminal, no horizontal overflow).

No-Issue: direct founder request to reduce the homepage terminal preview size.


Note

Low Risk
Presentation-only homepage CSS and responsive image hints; no auth, data, or routing changes.

Overview
The homepage hero terminal no longer spans the full content column on desktop. CSS caps .folio-shot at 56rem (~896px), centers it under the intro copy, and keeps it full width below 760px. The hero grid comment now describes a compact, centered shot rather than a full-column product image.

The Next.js Image sizes attribute is updated to match (58rem breakpoint, 56rem max) so the browser picks appropriately sized assets for the smaller rendered width.

Reviewed by Cursor Bugbot for commit 26e0355. Bugbot is set up for automated code reviews on this repo. Configure here.

Cap the terminal at 896px and center it below the introduction, instead
of stretching it across the whole 1216px desktop content column. Keep
mobile full width and preserve the image aspect ratio. Match Next image
sizes to the new display width.

Validation:
- Website npm test: 407 passed; 0 failed (47 test files).
- npm run lint: 0 errors; 2 existing no-img-element warnings in the
  untouched header and footer.
- Browser inspection at 1280px: terminal width 896px.
- Browser inspection at 390px: terminal width 343px, no horizontal
  page overflow; full screenshot and caption preserved.
- git diff --check: PASS.

The root npm test/check:web scripts are absent from this Rust repository;
website-specific checks above were run. This is locally verified source,
not a deployment receipt. Leave the tool-generated web/AGENTS.md change
unstaged, per the operator-control boundary.
Copilot AI lite review requested due to automatic review settings September 5, 2026 11:29
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T11:31:24.354286Z 762a1c6 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cursor

cursor Bot commented Sep 5, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_300ced0a-d695-481b-8eb2-afdcd5c87d37)

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 1m 12s —— View job


Review: homepage terminal preview compaction

Summary

Small, well-scoped CSS/markup change (3 lines each file). Verified the mechanics rather than just reading the diff:

  • .folio-shot (web/app/globals.css:4075) changes from width: 100% to width: min(100%, 56rem) with justify-self: center. This rule sits in an unconditional block (not inside a media query — confirmed the enclosing @media (min-width: 901px) at line 3981 closes at line 3989, well before this block starts), so it applies at all viewport widths ≥ the existing mobile breakpoint.
  • The mobile override at web/app/globals.css:4082 (@media (max-width: 760px) { .folio-shot { width: 100%; } }) is untouched and correctly restores full width on mobile, as the PR description claims.
  • .folio-shot img (web/app/globals.css:2108-2113) already uses width: 100%; height: auto;, so the screenshot's aspect ratio is preserved automatically under the new cap — no separate aspect-ratio handling was needed.
  • The sizes attribute in web/app/[locale]/page.tsx:100 changes to (max-width: 928px) calc(100vw - 2rem), 896px. 896px = 56rem (matches the new CSS cap), and 928px = 896px + 2rem (accounting for the page's horizontal padding) — consistent with the actual rendered widths, so Next.js will request appropriately-sized image variants rather than over-fetching.
  • Confirmed no other component reuses .folio-shot or references the old 1248px/40rem sizing, so this change is isolated to the homepage hero and doesn't leave any inconsistent sibling usage.

No correctness issues, no dead code, no test gaps worth flagging — this is a presentation-only tweak with existing coverage (unit tests unaffected) and the PR's own manual browser verification at 1280px/390px is appropriate evidence for a CSS-only change like this. Nothing here touches provider/model/routing logic.

Nit (non-blocking): min(100%, 56rem) and the sizes "896px" value are the same number expressed two ways (56rem vs 896px) — purely stylistic, not worth a change.

Looks good to merge from a correctness standpoint.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are confined to presentational CSS and responsive image hints with low behavioral risk.

Pull request overview

Adjusts the homepage hero “terminal” screenshot presentation so it no longer dominates the desktop content column, while keeping the mobile experience full-width and preserving the screenshot’s aspect ratio. This aligns the hero’s visual hierarchy with the product intro copy and reduces unnecessary image overfetching.

Changes:

  • Cap and center the homepage .folio-shot on desktop (56rem max) while keeping small screens at width: 100%.
  • Update the Next.js Image sizes hint to match the new rendered desktop width.
File summaries
File Description
web/app/globals.css Centers and caps the hero screenshot width on larger viewports; preserves full-width behavior under 760px.
web/app/[locale]/page.tsx Tightens the Image sizes hint to better match the new max rendered width.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread web/app/[locale]/page.tsx Outdated

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codewhale review

The PR caps the homepage terminal preview at 896px, centers it, and updates the Next.js responsive image sizes hint to match the new rendered width. The change is presentation-only and retains full-width behavior on small screens.

Assessment

Approved. The CSS cap and the 928px sizes breakpoint align correctly with the existing 2rem page gutters, and the manual browser verification covers the key responsive breakpoints. No automated tests are needed for this visual-only change.


Advisory review by Codewhale (codewhale review --pr 5889 --post, head 762a1c6dae98d2df08bb10c67fbb53bdc4353671). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.

Validation: website tests 407/407 passed; ESLint 0 errors, 2 existing image warnings.
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
@cursor

cursor Bot commented Sep 5, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_c16bb3eb-a74b-4646-955e-be3905d9697d)

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 1m 25s —— View job


Review: homepage terminal preview compaction

  • Gather context (diff, changed files)
  • Review web/app/globals.css change
  • Review web/app/[locale]/page.tsx change
  • Check for provider/model/route safety concerns (N/A — CSS/markup only)
  • Post findings

Summary

This is a small, well-scoped presentational change (2 files, ±3 lines each across two commits). I traced the cascade rather than just reading the diff.

  • .folio-shot (web/app/globals.css:4075) now sets width: min(100%, 56rem); justify-self: center, in an unconditional rule block (confirmed the preceding @media (min-width: 901px) closes at line 3989, well before this rule). It correctly overrides earlier .folio-shot width declarations (lines 2095, 2811) via source order since all share equal specificity.
  • The mobile override (web/app/globals.css:4082, @media (max-width: 760px)) is untouched and still forces width: 100%, so mobile stays full-width as claimed.
  • .folio-shot img (line 2108-2113) already uses width: 100%; height: auto, so aspect ratio (2760×1494) is preserved automatically under the new cap.
  • Good catch/fix between commits: the first commit used sizes="(max-width: 928px) calc(100vw - 2rem), 896px", and Copilot's review flagged the px/rem unit mismatch against the 56rem CSS cap. The second commit (26e0355) correctly resolved this by switching to sizes="(max-width: 58rem) calc(100vw - 2rem), 56rem" — now unit-consistent with the CSS, so the hint won't drift if a user's root font size differs from 16px.

Minor, non-blocking, pre-existing (not introduced by this PR): there's a .folio-shot { width: min(100%, 44rem); ... } rule inside @media (max-width: 1050px) at web/app/globals.css:2808-2813. Because the unconditional rule at line 4075 comes later in the file with equal specificity, it always wins regardless of viewport width, so the 1050px-specific 44rem value is already shadowed/dead. This predates this PR (the old unconditional rule was width: 100%, which shadowed it the same way) — just flagging for awareness, not something this PR needs to fix.

No correctness bugs, no provider/model/routing concerns (none apply — this is homepage CSS/image-hint only), no dead code introduced, and no test gaps: this is a visual-only change and the PR's manual verification at 1280px/390px is appropriate evidence.

Looks good to merge.

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codewhale review

The change caps the homepage terminal preview at 56rem on desktop, centers it, and updates the Next Image sizes hint to match the new rendered width. The desktop breakpoint and cap are internally consistent.

Findings

  • [INFO] Mobile sizes hint may still overestimate the rendered terminal width (web/app/[locale]/page.tsx:100)
    The updated sizes attribute still uses (max-width: 58rem) calc(100vw - 2rem), 56rem. The PR reports a 390px viewport rendering the terminal at 343px, while calc(100vw - 2rem) resolves to 358px. The gap is likely caused by scrollbar width and/or mobile container padding. This only affects responsive asset selection and has no rendering or layout impact, but the value does not precisely match the browser verification measurement.

Assessment

Low-risk presentation-only change. The desktop sizing is correct and matches the CSS cap. The only note is that the mobile sizes value may remain slightly generous for asset selection; no blocking issues.


Advisory review by Codewhale (codewhale review --pr 5889 --post, head 26e0355128b4c4cc5304dcd51affed0bf3c7372e). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.

Comment thread web/app/[locale]/page.tsx
width={2760}
height={1494}
sizes="(max-width: 1280px) calc(100vw - 2rem), 1248px"
sizes="(max-width: 58rem) calc(100vw - 2rem), 56rem"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[INFO] Mobile sizes hint may still overestimate the rendered terminal width

The updated sizes attribute still uses (max-width: 58rem) calc(100vw - 2rem), 56rem. The PR reports a 390px viewport rendering the terminal at 343px, while calc(100vw - 2rem) resolves to 358px. The gap is likely caused by scrollbar width and/or mobile container padding. This only affects responsive asset selection and has no rendering or layout impact, but the value does not precisely match the browser verification measurement.

@Hmbown
Hmbown merged commit 062e65d into main Sep 5, 2026
33 checks passed
@Hmbown
Hmbown deleted the fix/site-terminal-size-20260905 branch September 5, 2026 12:08
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