Skip to content

made header better #1676

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ComputelessComputer merged 1 commit into from
Nov 17, 2025
Merged

made header better #1676

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions apps/web/src/routes/_view/route.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ function Header() {

return (
<>
<header className="sticky top-0 bg-white/80 backdrop-blur-sm border-b border-neutral-100 z-50">
<header className="sticky top-0 bg-white/80 backdrop-blur-sm border-b border-neutral-100 z-50 h-[69px]">
<div
className={`${maxWidthClass} mx-auto px-4 laptop:px-0 border-x border-neutral-100 py-4`}
className={`${maxWidthClass} mx-auto px-4 laptop:px-0 border-x border-neutral-100 h-full`}
>
<div className="flex items-center justify-between">
<div className="flex items-center justify-between h-full">
Comment on lines +94 to +98
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix mobile menu positioning to match new header height.

The header height has been changed to h-[69px], but the mobile menu overlay (line 265) and mobile menu (line 269) still use top-[65px]. This creates a 4px misalignment where the mobile menu overlaps with the header.

Apply this diff to fix the alignment:

-        <div
-          className="fixed top-[65px] left-0 right-0 bottom-0 bg-black/20 z-40 sm:hidden animate-in fade-in duration-200"
+        <div
+          className="fixed top-[69px] left-0 right-0 bottom-0 bg-black/20 z-40 sm:hidden animate-in fade-in duration-200"
           onClick={() => setIsMenuOpen(false)}
         />

-        <div className="fixed top-[65px] left-0 right-0 bg-white border-b border-neutral-100 shadow-lg z-50 sm:hidden animate-in slide-in-from-top duration-300 max-h-[calc(100vh-65px)] overflow-y-auto">
+        <div className="fixed top-[69px] left-0 right-0 bg-white border-b border-neutral-100 shadow-lg z-50 sm:hidden animate-in slide-in-from-top duration-300 max-h-[calc(100vh-69px)] overflow-y-auto">

Note: Also update max-h-[calc(100vh-65px)] to max-h-[calc(100vh-69px)] on line 269 for correct overflow calculation.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In apps/web/src/routes/_view/route.tsx around lines 94-98 (header) and the
mobile menu code at lines ~265-269, the header height was changed to h-[69px]
but the mobile overlay and menu still use top-[65px] and
max-h-[calc(100vh-65px)]; update the mobile overlay and mobile menu top values
from top-[65px] to top-[69px], and change max-h-[calc(100vh-65px)] to
max-h-[calc(100vh-69px)] so the mobile menu aligns with the new header height
and the overflow calculation is correct.

<div className="hidden sm:flex items-center gap-5">
<Link
to="/"
Expand Down Expand Up @@ -170,7 +170,7 @@ function Header() {
</div>
<Link
to="/docs"
className="text-sm text-neutral-600 hover:text-neutral-800 transition-all hover:underline decoration-dotted"
className="hidden md:block text-sm text-neutral-600 hover:text-neutral-800 transition-all hover:underline decoration-dotted"
>
Docs
</Link>
Expand All @@ -188,7 +188,7 @@ function Header() {
</Link>
<Link
to="/enterprise"
className="text-sm text-neutral-600 hover:text-neutral-800 transition-all hover:underline decoration-dotted"
className="hidden md:block text-sm text-neutral-600 hover:text-neutral-800 transition-all hover:underline decoration-dotted"
>
Enterprise
</Link>
Expand Down Expand Up @@ -235,16 +235,16 @@ function Header() {
<a
href="/download/apple-silicon"
download
className="px-3 h-8 flex items-center text-xs bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all"
className="px-4 h-8 flex items-center text-sm bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all"
>
{platformCTA.label}
</a>
) : (
<button
onClick={handleCTAClick}
className="px-3 h-8 flex items-center text-xs bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all"
className="px-4 h-8 flex items-center text-sm bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all"
>
{platform === "mobile" ? "Remind me" : platformCTA.label}
{platformCTA.label}
</button>
Comment on lines +238 to 248
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Verify label consistency between mobile header and mobile menu CTAs.

The styling changes look good, but there's a potential label inconsistency:

  • Line 247 (mobile header CTA): Uses platformCTA.label directly
  • Lines 385-387 (mobile menu CTA): Uses conditional logic platform === "mobile" ? "Get reminder" : platformCTA.label

If the user is on a mobile platform, they might see different labels for the same action in the header versus the mobile menu.

Run the following script to check the implementation of getPlatformCTA and verify what label is returned for the mobile platform:

If platformCTA.label for mobile platform is not "Get reminder", consider standardizing the label logic by applying this diff to line 385-387:

-                    {platform === "mobile"
-                      ? "Get reminder"
-                      : platformCTA.label}
+                    {platformCTA.label}

Or, if the conditional logic in the mobile menu is correct, revert the change at line 247:

-                  {platformCTA.label}
+                  {platform === "mobile"
+                    ? "Get reminder"
+                    : platformCTA.label}

🏁 Script executed:

#!/bin/bash
# Description: Find the implementation of getPlatformCTA to verify mobile platform label

# Search for getPlatformCTA function definition
ast-grep --pattern 'function getPlatformCTA($$$) {
  $$$
}'

# Also search for export const/function getPlatformCTA
ast-grep --pattern 'export $_ getPlatformCTA = $$$'

# Search with rg as backup
rg -n "getPlatformCTA.*=.*\(" -A 20 --type=ts --type=tsx

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Description: Find getPlatformCTA function implementation

# Search for getPlatformCTA across all files
rg -n "getPlatformCTA" -A 30 --type=ts

# If that doesn't work, try without type specification
rg -n "getPlatformCTA" -A 30

Length of output: 21669

🏁 Script executed:

#!/bin/bash
# Find lines 385-387 in route.tsx to see the mobile menu CTA
sed -n '380,390p' apps/web/src/routes/_view/route.tsx

# Also check if there's a similar pattern around those lines in index.tsx
sed -n '380,390p' apps/web/src/routes/_view/index.tsx

Length of output: 896

Fix label inconsistency between mobile header and mobile menu CTAs.

Mobile users will see different labels for the same action:

  • Header CTA (line 247): "Join waitlist" (from platformCTA.label)
  • Mobile menu CTA (lines 385-387): "Get reminder" (from conditional logic)

Apply the conditional logic to line 247 to match the mobile menu:

-                  {platformCTA.label}
+                  {platform === "mobile"
+                    ? "Get reminder"
+                    : platformCTA.label}

Or standardize both to use platformCTA.label if "Get reminder" is the outdated logic.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
className="px-4 h-8 flex items-center text-sm bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all"
>
{platformCTA.label}
</a>
) : (
<button
onClick={handleCTAClick}
className="px-3 h-8 flex items-center text-xs bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all"
className="px-4 h-8 flex items-center text-sm bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all"
>
{platform === "mobile" ? "Remind me" : platformCTA.label}
{platformCTA.label}
</button>
className="px-4 h-8 flex items-center text-sm bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all"
>
{platformCTA.label}
</a>
) : (
<button
onClick={handleCTAClick}
className="px-4 h-8 flex items-center text-sm bg-linear-to-t from-stone-600 to-stone-500 text-white rounded-full shadow-md active:scale-[98%] transition-all"
>
{platform === "mobile"
? "Get reminder"
: platformCTA.label}
</button>
🤖 Prompt for AI Agents 
In apps/web/src/routes/_view/route.tsx around lines 238 to 248, the header CTA
uses platformCTA.label while the mobile menu CTA uses a conditional that renders
"Get reminder" in some cases; update the header anchor/button at line ~247 to
use the same conditional expression as the mobile menu so both CTAs render the
identical computed label (or alternatively standardize both places to always use
platformCTA.label if you decide "Get reminder" is outdated). Ensure you replace
the direct {platformCTA.label} with the same conditional logic used in the
mobile menu so the labels remain consistent across header and mobile menu.

)}
<button
Expand Down
Loading