Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a NotFound page component, introduces a new /auth route with email-OTP and Google OAuth flows, refactors account and header UI/logic, removes the old auth view, updates root route to use the NotFound component, and adds Google font variables to styles. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthRoute as /auth Route
participant Server
participant AppNav as App Navigation
rect rgb(220,235,255)
Note over AuthRoute: Email OTP flow
User->>AuthRoute: Submit email
AuthRoute->>Server: doAuth(method: "email_otp", flow)
alt Email sent
Server-->>AuthRoute: success
AuthRoute->>User: show success message
else Error
Server-->>AuthRoute: error
AuthRoute->>User: show error
end
end
rect rgb(220,255,230)
Note over AuthRoute: Google OAuth flow
User->>AuthRoute: Click "Sign in with Google"
AuthRoute->>Server: doAuth(method: "oauth", provider: "google", flow)
alt Redirect URL returned
Server-->>AuthRoute: { url }
AuthRoute->>AppNav: navigate to returned url
else No URL / Error
Server-->>AuthRoute: error
AuthRoute->>User: show error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
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 |
ComputelessComputer
force-pushed
the
jj-branch-27
branch
from
7c23038 to
de178e8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
apps/web/src/routes/_view/route.tsx (1)
25-50: Header updates look good.Consider:
- Add
<nav aria-label="primary">for clearer landmarks.- Extract a shared Header to avoid duplication with NotFound’s header.
Also applies to: 73-78
apps/web/src/components/not-found.tsx (1)
34-86: Deduplicate Header and Footer across public pages.This Header/Footer duplicates
_view/route.tsx. Extract shared<SiteHeader/>and<SiteFooter/>to prevent drift and ease updates.Also applies to: 88-108
apps/web/src/routes/_view/index.tsx (1)
311-319: Add explicit image attributes for better perf.Provide intrinsic size and lazy hints to limit CLS and network priority.
- <img - src="/hyprnote_with_noise.png" - alt="Hyprnote" - className="size-36 mx-auto rounded-[40px] border border-neutral-200" - /> + <img + src="/hyprnote_with_noise.png" + alt="Hyprnote" + width="144" + height="144" + loading="lazy" + decoding="async" + fetchpriority="low" + className="size-36 mx-auto rounded-[40px] border border-neutral-200" + />apps/web/src/routes/_view/app/account.tsx (2)
8-11: Guard the account route for signed-out users.Redirect unauthenticated users before rendering. Prefer
beforeLoadfor route-level protection.-export const Route = createFileRoute("/_view/app/account")({ - component: Component, - loader: async ({ context }) => ({ user: context.user }), -}); +import { redirect } from "@tanstack/react-router"; +export const Route = createFileRoute("/_view/app/account")({ + beforeLoad: async ({ context }) => { + if (!context.user) { + throw redirect({ to: "/auth", search: { flow: "web" } }); + } + return { user: context.user }; + }, + component: Component, + loader: async ({ context }) => ({ user: context.user }), +});
94-150: Plan controls are stubbed; wire to real state and mutations.
currentPlanand trial handlers are placeholders. Consider deriving plan from loader/context and using mutations for trial/upgrade to avoid a sharedloadingboolean across actions.apps/web/src/routes/auth.tsx (2)
55-84: Consider consistent loading state management.The Google sign-in handler manually resets
loadingin each error path (lines 70, 76, 82), while the email handler uses afinallyblock. Although the current code works correctly (success redirects immediately), using a try-finally pattern here would be more maintainable and consistent.Apply this diff to align the pattern:
const handleGoogleSignIn = async () => { setLoading(true); setMessage(null); try { const result = await doAuth({ data: { method: "oauth", provider: "google", flow, }, }); if (!result) { setMessage({ type: "error", text: "No response from server" }); - setLoading(false); return; } if (result.error) { setMessage({ type: "error", text: result.message || "Failed to sign in with Google" }); - setLoading(false); } else if (result.url) { window.location.href = result.url; } } catch (error) { setMessage({ type: "error", text: "An unexpected error occurred" }); + } finally { - setLoading(false); + setLoading(false); } };
127-137: Consider usingcnutility for conditional classes.Per the coding guidelines, when classNames have conditional logic, prefer using
cnfrom@hypr/utilsand pass an array of class segments split by logical grouping.As per coding guidelines.
Example refactor:
import { cn } from "@hypr/utils"; // In the component: {message && ( <div className={cn([ "mt-4 p-3 rounded-lg text-sm", message.type === "success" ? ["bg-green-50", "text-green-800", "border", "border-green-200"] : ["bg-red-50", "text-red-800", "border", "border-red-200"] ])} > {message.text} </div> )}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/web/public/hyprnote_with_noise.pngis excluded by!**/*.pngapps/web/src/routeTree.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (8)
apps/web/src/components/not-found.tsx(1 hunks)apps/web/src/routes/__root.tsx(2 hunks)apps/web/src/routes/_view/app/account.tsx(2 hunks)apps/web/src/routes/_view/auth.tsx(0 hunks)apps/web/src/routes/_view/index.tsx(1 hunks)apps/web/src/routes/_view/route.tsx(3 hunks)apps/web/src/routes/auth.tsx(1 hunks)apps/web/src/styles.css(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/routes/_view/auth.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/simple.mdc)
After a substantial amount of TypeScript changes, run
pnpm -r typecheck
Files:
apps/web/src/routes/_view/index.tsxapps/web/src/routes/_view/route.tsxapps/web/src/routes/auth.tsxapps/web/src/routes/_view/app/account.tsxapps/web/src/components/not-found.tsxapps/web/src/routes/__root.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/simple.mdc)
**/*.{tsx,jsx}: When many classNames have conditional logic in React components, usecnfrom@hypr/utils
When usingcn, always pass an array of class segments
When usingcn, split entries by logical grouping for readability
Usemotion/reactinstead offramer-motion
Files:
apps/web/src/routes/_view/index.tsxapps/web/src/routes/_view/route.tsxapps/web/src/routes/auth.tsxapps/web/src/routes/_view/app/account.tsxapps/web/src/components/not-found.tsxapps/web/src/routes/__root.tsx
🧬 Code graph analysis (3)
apps/web/src/routes/auth.tsx (1)
apps/web/src/functions/auth.ts (1)
doAuth(11-52)
apps/web/src/routes/_view/app/account.tsx (2)
apps/web/src/routes/__root.tsx (1)
Route(14-31)apps/web/src/functions/auth.ts (1)
signOutFn(67-76)
apps/web/src/routes/__root.tsx (1)
apps/web/src/components/not-found.tsx (1)
NotFoundDocument(3-32)
🔇 Additional comments (4)
apps/web/src/routes/_view/app/account.tsx (1)
214-246: Sign out flow looks solid.Mutation, pending disable, and navigation on success/error are handled cleanly. ✓ Integration route
/_view/app/integrationconfirmed to exist.apps/web/src/routes/__root.tsx (1)
1-1: Typecheck verification requires manual execution in your development environment.The sandbox environment lacks the necessary tools (deno, installed dependencies) to run
pnpm -r typecheck. The code changes appear structurally sound, but please run the typecheck locally in your repository to ensure type safety across the refactored imports and NotFound component integration:pnpm -r typecheckapps/web/src/routes/auth.tsx (2)
23-53: Email sign-in handler looks solid.The error handling is comprehensive with try-catch-finally ensuring the loading state is always reset. The defensive null check on line 37 is appropriate given the server function's implementation.
92-92: Verify custom Tailwind configuration forrounded-4xl.The class
rounded-4xlis not part of standard Tailwind CSS (which maxes out atrounded-3xl). Ensure your Tailwind config extends the theme with this value, or use a standard class.
2 participants
No description provided.