-
Notifications
You must be signed in to change notification settings - Fork 18
feat(download): add download page with platform detection #180
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.
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
Conversation
- Add /download page with auto-detect for macOS, Windows, Linux - Add download button to Header and user avatar dropdown - Create fixed-name aliases in release workflow for stable URLs - Remove .deb builds (no auto-update support)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request introduces a new download page feature for desktop application distribution. It adds platform-aware download functionality with header integration, creates an unauthenticated-accessible download route, adds SVG icon components, implements browser platform detection, and modifies the GitHub Actions workflow to generate fixed-name release artifact aliases for Linux AppImages and Windows installers. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as App/Router
participant Auth as Auth State
participant Header as Header Component
participant DownloadPage as Download Page
participant PlatformDetect as Platform Detection
participant Browser as Browser API
User->>App: Navigate to /download
App->>Auth: Check authentication status
Auth-->>App: Return auth state
App->>DownloadPage: Render with auth status
DownloadPage->>Header: Render conditional header<br/>(hideDownload=true if unauth)
DownloadPage->>PlatformDetect: Initialize platform detection
PlatformDetect->>Browser: Access navigator.userAgent<br/>navigator.platform
Browser-->>PlatformDetect: Return device info
PlatformDetect->>PlatformDetect: Detect platform<br/>(macOS/Windows/Linux x64/ARM64)
PlatformDetect-->>DownloadPage: Platform detected
DownloadPage->>DownloadPage: Render download cards<br/>with platform-specific links
DownloadPage-->>User: Display download options
sequenceDiagram
actor User
participant Header as Header Component
participant Sidebar as Sidebar Nav
participant DownloadPage as Download Page
User->>Header: Click Download (unauthenticated)
Header-->>User: Navigate /download
User->>DownloadPage: Load page
DownloadPage-->>User: Show platform cards<br/>+ download links
User->>Sidebar: Click Download in menu<br/>(authenticated)
Sidebar-->>User: Navigate /download
User->>DownloadPage: Load with auth header
DownloadPage-->>User: Show auth-aware layout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@app/components/Header.tsx`:
- Around line 86-92: The icon-only mobile download button (conditional block
using hideDownload, the Button component wrapping Link and the Download icon in
Header.tsx) lacks accessible labeling; add an accessible name by supplying an
aria-label on the interactive element (e.g., Button or Link) such as
aria-label="Download" or add a visually-hidden text label (screen-reader-only
span) next to the Download icon so screen readers announce the purpose; ensure
the label is localized if applicable and update tests/components that expect
accessible attributes accordingly.
In `@app/download/DownloadPageContent.tsx`:
- Around line 85-98: DownloadPageContent renders Convex's
Authenticated/Unauthenticated blocks but the Header in the Unauthenticated block
calls useAuth from `@workos-inc/authkit-nextjs`, which can desynchronize from
Convex during transitions; update this by ensuring both auth systems are
synchronized or consolidating to a single provider: either (A) make
DownloadPageContent read the WorkOS auth state (via useAuth) alongside Convex
and only render Header when both providers agree, or (B) refactor Header to
accept an explicit auth prop (e.g., isAuthenticated or user) and have
DownloadPageContent pass WorkOS state into Header so Header no longer calls
useAuth directly, or (C) migrate authentication to one unified provider and
remove the mixed usage of Authenticated/Unauthenticated and useAuth to avoid
mismatched UI during transitions. Ensure changes reference DownloadPageContent,
Authenticated, Unauthenticated, Header, and useAuth.
In `@app/download/DownloadSection.tsx`:
- Around line 76-81: The helper that returns the default platform object
currently sets downloadUrl: downloadLinks.macos which causes unknown-platform
users to get a macOS installer; change the default to a neutral value (e.g.,
downloadUrl: undefined or empty string) and keep platform: "unknown" and
displayName as-is, and then update the DownloadSection component to
conditionally render the primary download button only when downloadUrl is truthy
(use the existing downloadLinks and the helper that returns the platform object
to locate the change). This ensures unknown platforms either see no primary
download button (so they must use "All Downloads") or a neutral state instead of
a macOS DMG.
🧹 Nitpick comments (3)
app/download/icons/LinuxIcon.tsx (1)
1-14: Non-standard viewBox differs from other icons.The
viewBox="0 65 85 135 120"is different from the0 0 24 24viewBox used in the other icon components (AppleIcon, WindowsIcon, DeviceIcon). While this works because it crops the source SVG appropriately, it's worth noting the inconsistency. If the icon renders correctly at the intended sizes, this is acceptable..github/workflows/desktop-build.yml (1)
516-533: Consider adding a fixed-name alias for macOS DMG.The workflow creates fixed-name aliases for Linux (
HackerAI-linux-x64.AppImage,HackerAI-linux-arm64.AppImage) and Windows (HackerAI-windows-x64.exe), but macOS still uses the versionedHackerAI-universal.dmgfilename.While
HackerAI-universal.dmghappens to not include a version number, this is coincidental. For consistency and future-proofing (if versioned DMG names are ever introduced), consider explicitly creating aHackerAI-macos-universal.dmgalias:Suggested addition
# Windows exe WIN_EXE=$(ls release-files/*_x64-setup.exe 2>/dev/null | head -1 || echo "") if [ -n "$WIN_EXE" ] && [ -f "$WIN_EXE" ]; then cp "$WIN_EXE" release-files/HackerAI-windows-x64.exe fi + # macOS universal DMG (explicit alias for consistency) + MACOS_DMG=$(ls release-files/HackerAI-universal.dmg 2>/dev/null || echo "") + if [ -n "$MACOS_DMG" ] && [ -f "$MACOS_DMG" ]; then + cp "$MACOS_DMG" release-files/HackerAI-macos-universal.dmg + fiapp/download/DownloadPageContent.tsx (1)
111-122: Consider addingdownloadattribute for explicit download behavior.While browsers typically trigger downloads for binary file types (.dmg, .exe, .AppImage), adding the
downloadattribute makes the intent explicit and can improve UX in some edge cases.Optional enhancement
return ( <a href={href} + download className="flex items-center gap-3 rounded-md border bg-background p-4 transition-colors hover:bg-accent" >
| {!hideDownload && ( | ||
| <Button asChild variant="ghost" size="sm" className="rounded-[10px]"> | ||
| <Link href="/download"> | ||
| <Download className="h-4 w-4" /> | ||
| </Link> | ||
| </Button> | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add accessibility attributes to the icon-only mobile download button.
The mobile download button only shows an icon without text. Screen reader users won't know what this button does. Add an aria-label or use a visually-hidden label.
Suggested fix
{!hideDownload && (
- <Button asChild variant="ghost" size="sm" className="rounded-[10px]">
+ <Button asChild variant="ghost" size="sm" className="rounded-[10px]" aria-label="Download">
<Link href="/download">
<Download className="h-4 w-4" />
</Link>
</Button>
)}📝 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.
| {!hideDownload && ( | |
| <Button asChild variant="ghost" size="sm" className="rounded-[10px]"> | |
| <Link href="/download"> | |
| <Download className="h-4 w-4" /> | |
| </Link> | |
| </Button> | |
| )} | |
| {!hideDownload && ( | |
| <Button asChild variant="ghost" size="sm" className="rounded-[10px]" aria-label="Download"> | |
| <Link href="/download"> | |
| <Download className="h-4 w-4" /> | |
| </Link> | |
| </Button> | |
| )} |
🤖 Prompt for AI Agents
In `@app/components/Header.tsx` around lines 86 - 92, The icon-only mobile
download button (conditional block using hideDownload, the Button component
wrapping Link and the Download icon in Header.tsx) lacks accessible labeling;
add an accessible name by supplying an aria-label on the interactive element
(e.g., Button or Link) such as aria-label="Download" or add a visually-hidden
text label (screen-reader-only span) next to the Download icon so screen readers
announce the purpose; ensure the label is localized if applicable and update
tests/components that expect accessible attributes accordingly.
| export function DownloadPageContent() { | ||
| return ( | ||
| <div className="min-h-screen bg-background"> | ||
| <Authenticated> | ||
| <AuthenticatedHeader /> | ||
| <DownloadContent /> | ||
| </Authenticated> | ||
| <Unauthenticated> | ||
| <Header hideDownload /> | ||
| <DownloadContent /> | ||
| </Unauthenticated> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how Convex auth is configured and if it's synced with WorkOS
echo "=== Looking for Convex auth provider setup ==="
rg -n "ConvexProviderWithAuth|ConvexReactClient" --type=tsx --type=ts -C3
echo ""
echo "=== Looking for auth synchronization between WorkOS and Convex ==="
rg -n "useAuth|Authenticated|Unauthenticated" --type=tsx --type=ts -C2 | head -100
echo ""
echo "=== Checking app/page.tsx or main layout for auth provider setup ==="
cat app/page.tsx 2>/dev/null || echo "Not found"
echo ""
echo "=== Checking for app layout files ==="
find app -name "layout.*" -o -name "page.tsx" | head -5Repository: hackerai-tech/hackerai
Length of output: 5026
🏁 Script executed:
#!/bin/bash
echo "=== Finding Header.tsx ==="
find . -name "Header.tsx" -type f
echo ""
echo "=== Examining Header.tsx ==="
cat "app/components/Header.tsx" 2>/dev/null || echo "Not found at that path"
echo ""
echo "=== Search for WorkOS auth usage ==="
rg "workos" -i --type ts --type tsx | head -30
echo ""
echo "=== Check DownloadPageContent imports ==="
head -20 "app/download/DownloadPageContent.tsx"Repository: hackerai-tech/hackerai
Length of output: 4966
Clarify the auth provider concern in DownloadPageContent.
This component uses Authenticated/Unauthenticated from convex/react and renders different headers based on Convex state. However, the Header component (shown in the Unauthenticated block) uses useAuth from @workos-inc/authkit-nextjs. While the Convex wrapper prevents most inconsistencies by rendering completely separate headers, misalignment between Convex and WorkOS auth states during transitions could cause the wrong buttons to display in edge cases. Ensure both auth systems are synchronized, or consolidate to a single auth provider.
🤖 Prompt for AI Agents
In `@app/download/DownloadPageContent.tsx` around lines 85 - 98,
DownloadPageContent renders Convex's Authenticated/Unauthenticated blocks but
the Header in the Unauthenticated block calls useAuth from
`@workos-inc/authkit-nextjs`, which can desynchronize from Convex during
transitions; update this by ensuring both auth systems are synchronized or
consolidating to a single provider: either (A) make DownloadPageContent read the
WorkOS auth state (via useAuth) alongside Convex and only render Header when
both providers agree, or (B) refactor Header to accept an explicit auth prop
(e.g., isAuthenticated or user) and have DownloadPageContent pass WorkOS state
into Header so Header no longer calls useAuth directly, or (C) migrate
authentication to one unified provider and remove the mixed usage of
Authenticated/Unauthenticated and useAuth to avoid mismatched UI during
transitions. Ensure changes reference DownloadPageContent, Authenticated,
Unauthenticated, Header, and useAuth.
| return { | ||
| platform: "unknown", | ||
| displayName: "your platform", | ||
| downloadUrl: downloadLinks.macos, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reconsider default download URL for unknown platforms.
When the platform cannot be detected, downloadUrl defaults to downloadLinks.macos. If a user on an unknown platform clicks the primary download button, they'll download a macOS DMG which won't work for them.
Consider either:
- Omitting the download button for unknown platforms (the "All Downloads" section below handles this)
- Using a more neutral approach
Option: Hide download button for unknown platform
return {
platform: "unknown",
displayName: "your platform",
- downloadUrl: downloadLinks.macos,
+ downloadUrl: "",
};Then in the component:
- <Button asChild size="lg" className="mb-4 text-lg">
- <a href={detected.downloadUrl}>
- <DownloadIcon />
- Download for {detected.displayName}
- </a>
- </Button>
+ {detected.downloadUrl && (
+ <Button asChild size="lg" className="mb-4 text-lg">
+ <a href={detected.downloadUrl}>
+ <DownloadIcon />
+ Download for {detected.displayName}
+ </a>
+ </Button>
+ )}🤖 Prompt for AI Agents
In `@app/download/DownloadSection.tsx` around lines 76 - 81, The helper that
returns the default platform object currently sets downloadUrl:
downloadLinks.macos which causes unknown-platform users to get a macOS
installer; change the default to a neutral value (e.g., downloadUrl: undefined
or empty string) and keep platform: "unknown" and displayName as-is, and then
update the DownloadSection component to conditionally render the primary
download button only when downloadUrl is truthy (use the existing downloadLinks
and the helper that returns the platform object to locate the change). This
ensures unknown platforms either see no primary download button (so they must
use "All Downloads") or a neutral state instead of a macOS DMG.
Summary
/downloadpage with auto-detection for macOS, Windows, and Linux platformsChanges
/app/download/with platform detection usinguseSyncExternalStoreHackerAI-linux-x64.AppImage)/downloadTest plan
/downloadpage unauthenticated - should show Header with Sign in/Sign up/downloadpage authenticated - should show "Back to Chat" button🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.