Redesign Header to Match New Design & Fix Hero Section Overlap#67
Conversation
|
CodeAnt AI is reviewing your PR. |
github-actions
Bot
added
bug
📝 WalkthroughWalkthroughMarketing routes now render dedicated animated placeholder pages. The marketing navbar uses full-page paths, updated branding, responsive mobile navigation, and a Sign In CTA. The hero adds top padding to prevent overlap with the header. ChangesMarketing routing and navigation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Visitor
participant MarketingNavBar
participant AppRouter
participant PlaceholderPage
Visitor->>MarketingNavBar: Select marketing link
MarketingNavBar->>AppRouter: Navigate to route
AppRouter->>PlaceholderPage: Render matching page
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
CodeAnt AI finished reviewing your PR. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| </Link> | ||
| ) | ||
| )} | ||
| <nav className="hidden md:flex items-center gap-1 absolute left-1/2 -translate-x-1/2"> |
There was a problem hiding this comment.
relative anchor
The <nav> uses absolute left-1/2 -translate-x-1/2 but its closest ancestor that creates a positioning context is the <header> (fixed), not the pill motion.div. motion.div has no position: relative, so left: 50% is 50% of the full viewport, not 50% of the pill. It currently renders correctly only because the pill itself is centered within the full-width header — but if the pill's centering ever changes (e.g. left-aligned layout, padding offset) the nav will misalign. Adding relative to the motion.div pins the positioning to the pill and makes the intent explicit.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/components/layouts/MarketingLayout.tsx
Line: 94
Comment:
**Nav absolute positioning lacks a `relative` anchor**
The `<nav>` uses `absolute left-1/2 -translate-x-1/2` but its closest ancestor that creates a positioning context is the `<header>` (`fixed`), not the pill `motion.div`. `motion.div` has no `position: relative`, so `left: 50%` is 50% of the full viewport, not 50% of the pill. It currently renders correctly only because the pill itself is centered within the full-width header — but if the pill's centering ever changes (e.g. left-aligned layout, padding offset) the nav will misalign. Adding `relative` to the `motion.div` pins the positioning to the pill and makes the intent explicit.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| return ( | ||
| <div className="min-h-screen bg-[#0C1220] flex items-center justify-center p-6 text-[#E7EBF3] selection:bg-[#4FE0C8]/30 pt-[calc(var(--header-height,80px)+2rem)]"> | ||
| <div | ||
| className="pointer-events-none absolute inset-0 opacity-40 mix-blend-screen" | ||
| style={{ | ||
| background: "radial-gradient(ellipse 60% 80% at 50% 0%, rgba(79,224,200,0.1), transparent 70%)", | ||
| }} | ||
| /> |
There was a problem hiding this comment.
relative on the parent
The inner <div className="pointer-events-none absolute inset-0 ..."> is positioned relative to the nearest positioned ancestor, which here is NOT the outer wrapper div (it has no positioning class). The element will be positioned relative to the viewport (or a higher ancestor), causing the gradient to potentially bleed into the header and footer areas rendered by MarketingLayout. Adding relative to the outer container div fixes the containment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/pages/PlaceholderPage.tsx
Line: 14-21
Comment:
**Background overlay escapes its container without `relative` on the parent**
The inner `<div className="pointer-events-none absolute inset-0 ...">` is positioned relative to the nearest positioned ancestor, which here is NOT the outer wrapper div (it has no positioning class). The element will be positioned relative to the viewport (or a higher ancestor), causing the gradient to potentially bleed into the header and footer areas rendered by `MarketingLayout`. Adding `relative` to the outer container div fixes the containment.
How can I resolve this? If you propose a fix, please make it concise.| export const DocsPage = () => <PlaceholderPage title="Documentation" description="Comprehensive guides and references for the Kepler platform." />; | ||
| export const SignInPage = () => <PlaceholderPage title="Sign In" description="Sign in to your Kepler account to access the dashboard." />; |
There was a problem hiding this comment.
SignInPage provides no authentication path
The "Sign In" button in the header now navigates to /signin, which renders a PlaceholderPage with a document icon and a single "Back to Home" button. There is no login form, redirect to an auth provider, or any mechanism to actually authenticate. Users who click "Sign In" will land on a dead-end page with no actionable path forward. This is especially notable because the old header routed users directly to /dashboard — this PR replaces that functional navigation with a non-functional placeholder.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/pages/PlaceholderPage.tsx
Line: 59-60
Comment:
**`SignInPage` provides no authentication path**
The "Sign In" button in the header now navigates to `/signin`, which renders a `PlaceholderPage` with a document icon and a single "Back to Home" button. There is no login form, redirect to an auth provider, or any mechanism to actually authenticate. Users who click "Sign In" will land on a dead-end page with no actionable path forward. This is especially notable because the old header routed users directly to `/dashboard` — this PR replaces that functional navigation with a non-functional placeholder.
How can I resolve this? If you propose a fix, please make it concise.| <div className="pointer-events-none absolute inset-0 bg-[linear-gradient(90deg,#05070C_0%,rgba(5,7,12,0.85)_38%,rgba(5,7,12,0.25)_62%,transparent_85%)] sm:bg-[linear-gradient(90deg,#05070C_0%,rgba(5,7,12,0.85)_38%,rgba(5,7,12,0.25)_62%,transparent_85%)]" /> | ||
|
|
||
| <div className="relative z-10 mx-auto w-full max-w-[1280px] px-6 sm:px-8 lg:px-10"> | ||
| <div className="relative z-10 mx-auto w-full max-w-[1280px] px-6 sm:px-8 lg:px-10 pt-[120px] sm:pt-[160px] pb-12"> |
There was a problem hiding this comment.
The new header height is approximately pt-4 (16px) + pill height (~44px) ≈ 60px on mobile and pt-6 (24px) + pill (~48px) ≈ 72px on sm:+. Using pt-[120px] / pt-[160px] adds roughly 48–88px of extra blank space above the hero content on top of the clearance already provided by the flex centering. The PlaceholderPage already uses a --header-height CSS variable approach as a fallback (var(--header-height, 80px)); a consistent approach here — or simply tighter values like pt-[80px] sm:pt-[96px] — would remove the visual gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/components/Hero.tsx
Line: 42
Comment:
**Hero top padding substantially overshoots the actual header height**
The new header height is approximately `pt-4` (16px) + pill height (~44px) ≈ 60px on mobile and `pt-6` (24px) + pill (~48px) ≈ 72px on `sm:+`. Using `pt-[120px]` / `pt-[160px]` adds roughly 48–88px of extra blank space above the hero content on top of the clearance already provided by the flex centering. The `PlaceholderPage` already uses a `--header-height` CSS variable approach as a fallback (`var(--header-height, 80px)`); a consistent approach here — or simply tighter values like `pt-[80px] sm:pt-[96px]` — would remove the visual gap.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/layouts/MarketingLayout.tsx`:
- Around line 148-160: Update MarketingLayout’s mobile backdrop and navigation
animation props to honor the existing useReducedMotion() state, disabling
opacity and transform transitions when reduced motion is requested. Apply the
same reduced-motion gating to PlaceholderPage’s entrance transition; modify both
frontend/src/components/layouts/MarketingLayout.tsx:148-160 and
frontend/src/pages/PlaceholderPage.tsx:23-26.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 82528c0b-c946-4c61-b733-2160bad526c7
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
frontend/src/App.tsxfrontend/src/components/Hero.tsxfrontend/src/components/layouts/MarketingLayout.tsxfrontend/src/pages/PlaceholderPage.tsx
| <motion.div | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| transition={{ duration: 0.2 }} | ||
| className="fixed inset-0 bg-[#0C1220]/40 backdrop-blur-sm z-[-1] pointer-events-auto md:hidden" | ||
| onClick={() => setMenuOpen(false)} | ||
| /> | ||
| <motion.nav | ||
| initial={{ height: 0, opacity: 0 }} | ||
| animate={{ height: "auto", opacity: 1 }} | ||
| exit={{ height: 0, opacity: 0 }} | ||
| transition={{ duration: 0.25 }} | ||
| className="md:hidden overflow-hidden border-t border-white/10" | ||
| initial={{ opacity: 0, y: -10, scale: 0.95 }} | ||
| animate={{ opacity: 1, y: 0, scale: 1 }} | ||
| exit={{ opacity: 0, y: -10, scale: 0.95 }} | ||
| transition={{ duration: 0.2, ease: "easeOut" }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== MarketingLayout outline ==\n'
ast-grep outline frontend/src/components/layouts/MarketingLayout.tsx --view expanded || true
printf '\n== PlaceholderPage outline ==\n'
ast-grep outline frontend/src/pages/PlaceholderPage.tsx --view expanded || true
printf '\n== Search reduced motion usage ==\n'
rg -n "useReducedMotion|reduced motion|reduce" frontend/src -g '!**/node_modules/**' || true
printf '\n== Read relevant slices ==\n'
sed -n '1,260p' frontend/src/components/layouts/MarketingLayout.tsx | nl -ba | sed -n '1,260p'
printf '\n----\n'
sed -n '1,200p' frontend/src/pages/PlaceholderPage.tsx | nl -ba | sed -n '1,200p'Repository: 7-Blocks/Kepler
Length of output: 4865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
def show(path, start, end):
print(f"\n== {path} [{start}-{end}] ==")
lines = Path(path).read_text().splitlines()
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i]}")
show("frontend/src/components/layouts/MarketingLayout.tsx", 60, 180)
show("frontend/src/pages/PlaceholderPage.tsx", 1, 120)
PY
printf '\n== PlaceholderPage reduced-motion search ==\n'
rg -n "useReducedMotion|prefers-reduced-motion|reduce" frontend/src/pages/PlaceholderPage.tsx || trueRepository: 7-Blocks/Kepler
Length of output: 9539
Honor reduced-motion for the new marketing animations.
MarketingLayout already reads useReducedMotion(), but the mobile backdrop/panel still animate unconditionally. PlaceholderPage has the same issue with its entrance transition. Gate both with the reduced-motion state so users requesting less motion don’t get the transform animation.
📍 Affects 2 files
frontend/src/components/layouts/MarketingLayout.tsx#L148-L160(this comment)frontend/src/pages/PlaceholderPage.tsx#L23-L26
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/layouts/MarketingLayout.tsx` around lines 148 - 160,
Update MarketingLayout’s mobile backdrop and navigation animation props to honor
the existing useReducedMotion() state, disabling opacity and transform
transitions when reduced motion is requested. Apply the same reduced-motion
gating to PlaceholderPage’s entrance transition; modify both
frontend/src/components/layouts/MarketingLayout.tsx:148-160 and
frontend/src/pages/PlaceholderPage.tsx:23-26.
2 participants
Description
This PR redesigns the website header to match the latest design mockup and resolves the hero section overlap caused by the fixed header.
Changes Made
Type of Change
Related Issue
Fixes #60
Checklist
Screenshots
Additional Notes
This PR focuses on aligning the header with the latest UI design while improving the overall user experience by eliminating the hero section overlap. The implementation preserves responsiveness across all supported screen sizes.
Summary by CodeRabbit
New Features
Style
Greptile Summary
This PR redesigns the marketing header from a dark pill-style navbar into a white rounded container with centered navigation, a purple "Sign In" CTA, and an improved mobile drawer with backdrop overlay. It also adds top padding to the Hero section to prevent content from hiding beneath the fixed header, and introduces five new placeholder pages for the updated nav links.
MarketingNavBaris rebuilt withbg-white/95 backdrop-blur-md rounded-full, centered nav links via absolute positioning, a purple "Sign In" button replacing the old "Launch" CTA, and a new three-bar animated hamburger with a full-screen backdrop on mobile.pt-[120px] sm:pt-[160px] pb-12is added to the Hero content wrapper; the values overshoot the actual header height (~60–72px) and will leave visible extra blank space above the hero text./product,/solutions,/developers,/docs,/signin) each render a genericPlaceholderPage; the/signinroute in particular replaces the previously functional/dashboardshortcut with a dead-end page that has no authentication path.Confidence Score: 4/5
Safe to merge with awareness that the Sign In button now leads to a dead-end placeholder rather than any auth flow, and that the hero section has more top clearance than the header actually requires.
The visual redesign is well-structured and the responsiveness logic is sound. The nav absolute-positioning pattern works at all current breakpoints due to the pill being centered in a full-width header, so there is no runtime breakage. The more notable concern is the
/signinroute: the old header sent users directly to/dashboard, but the new Sign In button routes to a placeholder page with no authentication mechanism, silently removing the only user path to the app. The hero padding overshoot is cosmetic but noticeably adds blank space on mobile.PlaceholderPage.tsxdeserves a second look — particularlySignInPage, which replaced a functional dashboard shortcut with a page that offers users no way to authenticate or navigate to the app.MarketingLayout.tsxshould haverelativeadded to the pill container to make the centered nav's positioning intent explicit.Important Files Changed
relativeanchor on the pill container, nav links updated to new routes, mobile drawer replaced with a modal-style overlay, and the "Launch" CTA replaced with "Sign In" navigating to/signin.pt-[120px] sm:pt-[160px] pb-12to the content wrapper to clear the fixed header; padding values are larger than the actual header height (~60–72px), creating noticeable extra blank space above the hero text.absolute inset-0withoutrelativeon the parent, andSignInPageprovides no authentication mechanism./product,/solutions,/developers,/docs,/signin) inside theMarketingLayoutwrapper, all pointing to the new placeholder pages."peer": trueannotations to several packages and removes two optional@emnapientries; no version changes to any dependencies.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User visits site] --> B[MarketingLayout renders] B --> C[MarketingNavBar fixed header] C --> D{Screen size?} D -- md+ desktop --> E[Pill navbar with centered absolute nav] D -- mobile --> F[Hamburger button] F --> G[Mobile drawer + backdrop overlay] E --> H{Nav link clicked} G --> H H -- /product --> I[ProductPage placeholder] H -- /solutions --> J[SolutionsPage placeholder] H -- /developers --> K[DevelopersPage placeholder] H -- /docs --> L[DocsPage placeholder] H -- /dashboard --> M[Dashboard app] H -- Sign In button --> N[SignInPage placeholder] N --> O[Back to Home button only - no auth form] style N fill:#ffd6d6,stroke:#cc0000 style O fill:#ffd6d6,stroke:#cc0000%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[User visits site] --> B[MarketingLayout renders] B --> C[MarketingNavBar fixed header] C --> D{Screen size?} D -- md+ desktop --> E[Pill navbar with centered absolute nav] D -- mobile --> F[Hamburger button] F --> G[Mobile drawer + backdrop overlay] E --> H{Nav link clicked} G --> H H -- /product --> I[ProductPage placeholder] H -- /solutions --> J[SolutionsPage placeholder] H -- /developers --> K[DevelopersPage placeholder] H -- /docs --> L[DocsPage placeholder] H -- /dashboard --> M[Dashboard app] H -- Sign In button --> N[SignInPage placeholder] N --> O[Back to Home button only - no auth form] style N fill:#ffd6d6,stroke:#cc0000 style O fill:#ffd6d6,stroke:#cc0000Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Redesign Header to Match New Design & Fi..." | Re-trigger Greptile