fix: footer gap (#123) + scrollToAnchor NaN guard (#124)

[yigitdot]

Two independent, pre-existing bugs surfaced during the PR #122 review. One branch, one PR.

#124 — scrollToAnchor jumps to top on NaN scroll-margin

parseFloat(getComputedStyle(el).scrollMarginTop) returns NaN when the computed value serializes to "" (detached/unrendered element). The NaN flows into distance; because NaN === 0 is false the no-op early return is skipped, the rAF runs window.scrollTo(x, NaN), CSSOM-View coerces y to 0, and the page silently animates to the top instead of the target section. Only the mobile-drawer → scrollToAnchor path is affected (desktop nav uses native scrollIntoView).

Fix: extract a pure, unit-tested parseScrollMarginTop helper that clamps non-finite results to 0 (finiteness only — sign is intentionally preserved). Worst case degrades to "scroll to the element ignoring its scroll-margin" — a sane, visible fallback — and the distance === 0 guard regains its meaning. Covered in lib/scroll.test.ts including the "" → 0 bug case and a negative-value contract test.

#123 — Empty band above the footer on tall viewports

Two compounding causes:

1. Footer's own top padding. Dropped pt-16 from Footer — the preceding Frame already carries generous --frame-pad-y (5–7rem) bottom padding on the same bg-paper, so the footer's own top padding was redundant.
2. The closing section's full-height floor. Frame forces every section to min-h: min(100svh, 54rem). The closing (Close) section's content is shorter than that and top-anchored, so the leftover height collapsed into a conspicuous dead band directly above the footer — the part the footer-padding trim alone couldn't fix.

Fix for (2): added an opt-out fill?: boolean prop to Frame (default true, so every other section is unchanged) and set fill={false} on the closing section. It is now content-height; the footer sits one --frame-pad-y below the closing content — tight and visually balanced, no dead band.

Review

Ran a multi-agent review (general/tests/comments/error-handling): no critical or important issues. Addressed the minor nits in 79efe85:

• Frame className composed via [...].filter(Boolean).join(" ") (no stray double-space when fill={false}).
• Added a negative-value test pinning that the helper guards finiteness, not sign.
• Tightened the scroll.ts rationale comment to the demonstrated/tested case.

Verification

• pnpm test — 95 passed
• pnpm lint, pnpm format:check — clean
• pnpm build — static export succeeds
• Manual: footer sits tight under the closing content on tall viewports; Hero/Comparison/Method/Faq/blog still full-bleed; #contact anchor unaffected

Closes #124
Closes #123

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying website with    Cloudflare Pages

Latest commit:	79efe85
Status:	✅  Deploy successful!
Preview URL:	https://f591a313.website-70y.pages.dev
Branch Preview URL:	https://fix-footer-gap-and-scroll-na.website-70y.pages.dev

View logs

[Copilot]

Pull request overview

Fixes two pre-existing bugs surfaced during PR #122 review: a NaN guard in scrollToAnchor that prevented silent scroll-to-top when scrollMarginTop resolves to "", and removal of redundant footer top padding causing an empty band above the footer on tall viewports.

Changes:

• Extract pure parseScrollMarginTop helper that clamps non-finite parse results to 0 and use it in scrollToAnchor.
• Add unit tests covering normal px, empty string (#124 bug case), non-numeric, and whitespace inputs.
• Drop pt-16 from Footer so it sits tight under the preceding Frame's bottom padding.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
lib/scroll.ts	Adds parseScrollMarginTop helper and uses it in scrollToAnchor to prevent NaN propagation.
lib/scroll.test.ts	Adds focused unit tests for parseScrollMarginTop including the bug case.
components/site/Footer.tsx	Removes redundant top padding to eliminate the empty band above the footer.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gemini-code-assist]

Code Review

This pull request addresses a bug causing the page to scroll to the top by introducing the parseScrollMarginTop utility to safely handle non-numeric CSS values, along with corresponding unit tests. Additionally, the footer's top padding was removed. The reviewer recommends retaining some top padding (e.g., pt-10) to avoid layout inconsistencies and tight coupling with preceding elements.

[gemini-code-assist]

Removing pt-16 entirely makes the footer's top spacing dependent on the preceding element's padding. While this fixes the 'empty band' issue when following a Frame component, it creates a tight coupling that may cause the footer to appear cramped or inconsistent if used on pages with different preceding sections. Additionally, the horizontal rule (line 7) will now sit flush against the top edge of the footer container. Consider using a reduced padding (e.g., pt-10) to maintain internal spacing while still addressing the gap issue. Note that the use of the arbitrary-value class px-[var(--frame-gutter)] is the correct pattern for layout-specific tokens in this repository.

Suggested change

	<footer className="bg-paper px-[var(--frame-gutter)] pb-10 text-ink">
	<footer className="bg-paper px-[var(--frame-gutter)] pt-10 pb-10 text-ink">

References

1. Use Tailwind arbitrary-value classes (e.g., px-[var(--variable)]) for layout-specific tokens when it is necessary to maintain specific CSS variable names or follow existing local patterns.

[yigitdot]

Thanks — rejecting this one, the concern doesn't hold for this codebase.

Every route that renders before the layout-level <Footer /> ends with a </Frame>: app/page.tsx (Close), app/blog/page.tsx, and app/blog/[slug]/page.tsx. Frame always applies py-[var(--frame-pad-y)] (clamp 5–7rem) bottom padding on the same bg-paper, so:

1. There is no route where a non-Frame precedes the footer — the "tight coupling" is the documented design system (AGENTS.md: Frame owns section vertical rhythm via --frame-pad-y), not an accident.
2. The rule is at y=0 of the footer's box, but the preceding Frame's 5–7rem same-background bottom padding sits directly above it, so it's not flush against any visible content.

Restoring pt-10 would re-add ~2.5rem of the exact preceding-padding + footer-padding stacking band that #123 asked to remove. Dropping the footer's own top padding is the intended fix here, not a regression.

[yigitdot]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a 'fill' property to the Frame component to allow for content-height sections, updates the Footer to remove top padding, and implements a robust 'parseScrollMarginTop' utility function in 'lib/scroll.ts' to prevent silent page scrolling bugs caused by NaN values from 'getComputedStyle'. Corresponding unit tests for the new utility have been added. I have no further feedback to provide.