feat: complete UI responsive overhaul and API stability improvements - #12
Open
ArslanYM wants to merge 1 commit into
Open
feat: complete UI responsive overhaul and API stability improvements#12ArslanYM wants to merge 1 commit into
ArslanYM wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request updates multiple UI components with improved responsive layouts and enhanced styling (Headers, Hero section, Project pages), adds authentication and authorization guards to API routes, improves error handling and validation in the config generation endpoint, and enhances the Provider with better error handling for user creation with conditional execution based on user state. Changes
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 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📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
web/app/project/_shared/SectionSettings.tsx (1)
51-72:⚠️ Potential issue | 🟡 MinorPremium access check doesn't prevent API call.
The check at line 53-55 shows a toast error for non-premium users but doesn't
return, allowing the API call to proceed. This either wastes resources or the API should handle the restriction server-side.🔧 Proposed fix to return early
async function GenerateNewScreen() { setLoading(true); if (!hasPremiumAccess) { toast.error("Limited feature for paid users only"); + setLoading(false); + return; } try {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/project/_shared/SectionSettings.tsx` around lines 51 - 72, In GenerateNewScreen, the premium check shows a toast but doesn’t stop execution; update the hasPremiumAccess branch to setLoading(false) and return immediately (or otherwise short-circuit) to prevent making the axios.post call when the user lacks access; reference the GenerateNewScreen function and the hasPremiumAccess variable to locate where to add the early return and cleanup.web/app/api/generate-config/route.ts (2)
112-115:⚠️ Potential issue | 🟡 MinorMissing 401 status code for unauthorized response.
The unauthorized response returns JSON without a status code, resulting in a
200 OK. This is inconsistent with the auth guards added to other routes in this PR.🔧 Proposed fix
const user = await currentUser(); if (!user) { - return NextResponse.json({ msg: "Unauthorized" }); + return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/api/generate-config/route.ts` around lines 112 - 115, The unauthorized branch in route handler uses currentUser() and returns NextResponse.json({ msg: "Unauthorized" }) without a 401 status, causing a 200 OK; update the unauthorized return to include a 401 status (e.g., NextResponse.json({ msg: "Unauthorized" }, { status: 401 })) so the route's auth behavior matches other guards and callers can detect authentication failures; change the return where currentUser() is checked to use NextResponse.json with the status option.
11-14:⚠️ Potential issue | 🟠 MajorMissing authentication guard on POST handler.
The
POSThandler processes user input and writes to the database but doesn't verify user authentication. Other routes in this PR (/api/project,/api/user) add early401guards. This endpoint should follow the same pattern to prevent unauthorized access.🛡️ Proposed fix to add auth guard
export async function POST(req: NextRequest) { + const user = await currentUser(); + if (!user || !user.primaryEmailAddress) { + return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + } + const { userInput, deviceType, projectId, oldScreenDescription, theme } = await req.json();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/api/generate-config/route.ts` around lines 11 - 14, The POST handler currently accepts input and writes to the DB without verifying authentication; update the start of the exported async function POST(req: NextRequest) to check the same auth/session guard used elsewhere in this PR (e.g., the logic used in /api/project or /api/user), and if the user is not authenticated return a 401 response immediately; ensure you reference the same authentication utility or session function those routes use and perform the guard before calling await req.json() or any DB operations in POST.web/app/_shared/Hero.tsx (3)
78-92:⚠️ Potential issue | 🟡 MinorMissing error handling for API call failure.
The axios call lacks a
try/catch. While the "pro version" case is handled, network errors or unexpected API failures will throw unhandled exceptions, leavingloadingstuck astrue.🛡️ Proposed fix to add error handling
setLoading(true); const projectId = crypto.randomUUID(); + try { const result = await axios.post("/api/project", { projectId: projectId, userInput: userInput, device: device, }); if (result.data.Message == "Buy the pro version to create more") { toast.error(result.data.Message); setLoading(false); return; } console.log(result.data); setLoading(false); router.push(`/project/${projectId}`); + } catch (error) { + toast.error("Failed to create project"); + setLoading(false); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/_shared/Hero.tsx` around lines 78 - 92, Wrap the axios.post call and subsequent result handling in a try/catch/finally: move the axios.post("/api/project", { projectId, userInput, device }) and the pro-version check (result.data.Message) and router.push(`/project/${projectId}`) into a try block, catch errors from axios and display an error toast (e.g., toast.error(err.message || "Request failed")), and ensure setLoading(false) is called in a finally block so loading is cleared on success or failure; reference the existing result variable, setLoading, and router.push in your changes.
72-74:⚠️ Potential issue | 🟡 MinorValidation logic allows submission with missing input.
The condition
!userInput && !deviceonly returns early if both are missing. If onlyuserInputis empty (the more common case), the function proceeds. This should likely be||to require both fields.🔧 Proposed fix
- if (!userInput && !device) { + if (!userInput || !device) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/_shared/Hero.tsx` around lines 72 - 74, The early-return validation in the Hero component currently checks if (!userInput && !device) which only blocks submission when both fields are missing; change this to use logical OR so it returns when either field is empty (i.e., if (!userInput || !device)) in the relevant submit/handler function in Hero.tsx to prevent submissions with missing input; update the conditional inside the function that contains the current check (search for the exact line with if (!userInput && !device)) and run the component tests or manual form flow to confirm behavior.
64-70:⚠️ Potential issue | 🟠 MajorIncorrect check for user authentication.
The
useUser()hook returns an object{ user, isLoaded, isSignedIn }, not the user directly. The checkif (!user)on line 67 is checking if the object is falsy, which will never be true. The user property should be destructured or accessed correctly.🔧 Proposed fix
- const user = useUser(); + const { user, isSignedIn } = useUser(); async function onCreateProject() { - if (!user) { + if (!isSignedIn || !user) { router.push("/sign-in"); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/_shared/Hero.tsx` around lines 64 - 70, The authentication check in onCreateProject is using useUser() as if it returned the user directly; update the call to destructure the returned object (e.g. const { user, isSignedIn } = useUser()) or access the user via the returned object, and change the gating condition to check the actual user or isSignedIn before calling router.push("/sign-in"); ensure you update the reference in the onCreateProject function to use the correct destructured symbol (user or isSignedIn) instead of the whole useUser() result.web/app/api/project/route.ts (3)
7-38:⚠️ Potential issue | 🟠 MajorMissing authentication guard on POST handler.
The
POSThandler usesuser?.primaryEmailAddresswith optional chaining but doesn't return early if the user is unauthenticated. This could insert a record withuserId: undefinedcast as string, causing data integrity issues.🛡️ Proposed fix
export async function POST(req: NextRequest) { const { userInput, device, projectId } = await req.json(); const user = await currentUser(); + if (!user || !user.primaryEmailAddress) { + return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + } + const { has } = await auth();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/api/project/route.ts` around lines 7 - 38, The POST handler uses currentUser() and then proceeds even if user is null, allowing insertion with an undefined userId; add an authentication guard at the start of POST that checks the result of currentUser() (and/or currentUser().primaryEmailAddress) and returns an early 401/unauthorized NextResponse when no authenticated user is present, before any usage of user?.primaryEmailAddress, and only call db.insert(ProjectTable).values(...) when the user is confirmed; update the POST function, currentUser() check, and any logic around ProjectTable/db.insert to rely on the validated user.
89-101:⚠️ Potential issue | 🟠 MajorMissing authentication and authorization on PUT handler.
The
PUThandler has no authentication check and doesn't verify the user owns the project being updated. Any unauthenticated request can modify any project by providing itsprojectId.🛡️ Proposed fix to add auth and ownership verification
export async function PUT(req: NextRequest) { const { projectName, theme, projectId, screenShot } = await req.json(); + const user = await currentUser(); + if (!user || !user.primaryEmailAddress) { + return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + } + const result = await db .update(ProjectTable) .set({ theme: theme, projectName: projectName, screenShot: (screenShot as string) ?? null, }) - .where(eq(ProjectTable.projectId, projectId)) + .where( + and( + eq(ProjectTable.projectId, projectId), + eq(ProjectTable.userId, user.primaryEmailAddress.emailAddress) + ) + ) .returning(); + + if (result.length === 0) { + return NextResponse.json({ error: "Project not found" }, { status: 404 }); + } return NextResponse.json(result[0]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/api/project/route.ts` around lines 89 - 101, The PUT handler currently allows unauthenticated updates; add authentication and ownership checks by extracting the current user (e.g., via your session/token helper) at the start of PUT, return a 401 if no user, then verify ownership by querying ProjectTable for projectId and comparing its owner/userId to the authenticated user's id (or include owner check in the db.update where clause using eq(ProjectTable.ownerId, user.id)); if the user does not own the project return 403 and do not perform the update, otherwise proceed with the db.update(ProjectTable).set(...).where(...) and returning the updated row.
84-86:⚠️ Potential issue | 🟡 MinorError object exposed without status code.
Returning the raw error object exposes internal details and returns a
200 OKstatus. This should return a sanitized message with a500status.🔧 Proposed fix
} catch (error) { - return NextResponse.json(error); + console.error("Project fetch error:", error); + return NextResponse.json({ error: "Internal server error" }, { status: 500 }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/api/project/route.ts` around lines 84 - 86, In the catch block that currently returns NextResponse.json(error), do not expose the raw error object; instead log the full error internally (e.g., console.error or your logger) and return a sanitized JSON response such as NextResponse.json({ error: 'Internal server error' } , { status: 500 }); update the catch in route.ts to use NextResponse.json with a 500 status and a safe message while keeping internal error details only in server logs.
🧹 Nitpick comments (1)
web/app/api/user/route.ts (1)
21-30: Inconsistent use of optional chaining after auth guard.Line 24 uses
user?.primaryEmailAddress?.emailAddress as string, but the auth guard on lines 10-12 already ensures bothuseranduser.primaryEmailAddressexist. The optional chaining is now unnecessary.♻️ Proposed cleanup
if (users?.length == 0) { const data = { name: user?.fullName ?? " ", - email: user?.primaryEmailAddress?.emailAddress as string, + email: user.primaryEmailAddress.emailAddress, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/api/user/route.ts` around lines 21 - 30, The auth guard already guarantees user and user.primaryEmailAddress are present, so remove unnecessary optional chaining in the data object: replace user?.primaryEmailAddress?.emailAddress as string with a direct access like user.primaryEmailAddress.emailAddress (keeping the string cast if desired) when building data before calling db.insert on usersTable; ensure you reference the same variables (user, data, usersTable, db.insert) so the code reflects the guaranteed non-null fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/app/_shared/Header.tsx`:
- Around line 93-98: The Header component is ignoring configurable auth labels
by hardcoding "Log in" and "Get Started" inside the SignInButton usages; update
the two occurrences (the Button/SignInButton pairs around the SignInButton
imports) to use the props currentLoginText and currentGetStartedText (with
sensible fallbacks, e.g., currentLoginText ?? "Log in" and currentGetStartedText
?? "Get Started") instead of the literal strings so custom labels passed into
Header are respected; ensure both the first pair (variant="outline") and the
second pair (bg-blue-600) are changed and that SignInButton mode="modal" and
asChild behavior remain unchanged.
- Around line 93-98: The Clerk SignInButton is being wrapped by your Button
component (Button asChild > SignInButton), which is unsupported and breaks modal
triggering; instead wrap your Button with SignInButton so SignInButton is the
outer component. Locate the two occurrences around Header.tsx where Button and
SignInButton are composed (the blocks containing Button className="rounded-full
shadow-sm" and Button className="rounded-full shadow-sm bg-blue-600
hover:bg-blue-700" and the similar block at the later occurrence), and change
the nesting so SignInButton mode="modal" is the parent and your Button (with the
same className/props and text content "Log in" or "Get Started") is the child.
In `@web/app/_shared/ProjectList.tsx`:
- Around line 40-41: The current early-return uses {!projects || projects.length
=== 0} which hides the loading indicator during initial fetch; change the render
logic in ProjectList.tsx to treat null/undefined projects as "loading" and only
show the empty-state when projects is an empty array. Locate the projects
variable and the JSX that renders the loader (the loader block currently at the
bottom) and update the condition so: if projects == null show the loader, else
if projects.length === 0 show the empty-state, otherwise render the list.
- Around line 32-36: The useEffect currently only fetches with getProjectList()
when user is truthy but doesn't clear previous projects when user becomes falsy;
update the useEffect to add an else branch that resets the projects state (call
the component's projects setter, e.g. setProjects([]) or setProjectList([])
depending on the state name) so that when user is null/undefined the projects
are cleared instead of lingering.
In `@web/app/project/_shared/SectionSettings.tsx`:
- Around line 154-161: The Snapshot Button calls the optional prop
takeScreenshot directly which can throw if undefined; update the SectionSettings
UI to guard against a missing takeScreenshot by checking its existence before
invoking (e.g., onClick={() => takeScreenshot && takeScreenshot()} or
conditional handler) and make the button visually/operationally disabled when
takeScreenshot is not provided (set disabled prop and adjust className
accordingly); ensure the prop reference is takeScreenshot so you only change the
click handler and button state, leaving the optional typing intact or update the
prop type to required if you intend it always present.
In `@web/provider.tsx`:
- Around line 24-28: The effect currently only creates a new user when `user`
exists, leaving stale `userDetail` when `user` becomes null and also retriggers
on full `user` object changes; update the effect to clear `userDetail` when
`user` is falsy and stabilize the dependency to an immutable identifier (e.g.
`user?.id` or `user?.uid`) so the POST only runs for real identity changes:
inside the effect do if (user?.id) CreateNewUser() else setUserDetail(null), and
change the dependency array from `[user]` to `[user?.id]` (or `[user?.uid]`) so
CreateNewUser and stale context handling are deterministic.
---
Outside diff comments:
In `@web/app/_shared/Hero.tsx`:
- Around line 78-92: Wrap the axios.post call and subsequent result handling in
a try/catch/finally: move the axios.post("/api/project", { projectId, userInput,
device }) and the pro-version check (result.data.Message) and
router.push(`/project/${projectId}`) into a try block, catch errors from axios
and display an error toast (e.g., toast.error(err.message || "Request failed")),
and ensure setLoading(false) is called in a finally block so loading is cleared
on success or failure; reference the existing result variable, setLoading, and
router.push in your changes.
- Around line 72-74: The early-return validation in the Hero component currently
checks if (!userInput && !device) which only blocks submission when both fields
are missing; change this to use logical OR so it returns when either field is
empty (i.e., if (!userInput || !device)) in the relevant submit/handler function
in Hero.tsx to prevent submissions with missing input; update the conditional
inside the function that contains the current check (search for the exact line
with if (!userInput && !device)) and run the component tests or manual form flow
to confirm behavior.
- Around line 64-70: The authentication check in onCreateProject is using
useUser() as if it returned the user directly; update the call to destructure
the returned object (e.g. const { user, isSignedIn } = useUser()) or access the
user via the returned object, and change the gating condition to check the
actual user or isSignedIn before calling router.push("/sign-in"); ensure you
update the reference in the onCreateProject function to use the correct
destructured symbol (user or isSignedIn) instead of the whole useUser() result.
In `@web/app/api/generate-config/route.ts`:
- Around line 112-115: The unauthorized branch in route handler uses
currentUser() and returns NextResponse.json({ msg: "Unauthorized" }) without a
401 status, causing a 200 OK; update the unauthorized return to include a 401
status (e.g., NextResponse.json({ msg: "Unauthorized" }, { status: 401 })) so
the route's auth behavior matches other guards and callers can detect
authentication failures; change the return where currentUser() is checked to use
NextResponse.json with the status option.
- Around line 11-14: The POST handler currently accepts input and writes to the
DB without verifying authentication; update the start of the exported async
function POST(req: NextRequest) to check the same auth/session guard used
elsewhere in this PR (e.g., the logic used in /api/project or /api/user), and if
the user is not authenticated return a 401 response immediately; ensure you
reference the same authentication utility or session function those routes use
and perform the guard before calling await req.json() or any DB operations in
POST.
In `@web/app/api/project/route.ts`:
- Around line 7-38: The POST handler uses currentUser() and then proceeds even
if user is null, allowing insertion with an undefined userId; add an
authentication guard at the start of POST that checks the result of
currentUser() (and/or currentUser().primaryEmailAddress) and returns an early
401/unauthorized NextResponse when no authenticated user is present, before any
usage of user?.primaryEmailAddress, and only call
db.insert(ProjectTable).values(...) when the user is confirmed; update the POST
function, currentUser() check, and any logic around ProjectTable/db.insert to
rely on the validated user.
- Around line 89-101: The PUT handler currently allows unauthenticated updates;
add authentication and ownership checks by extracting the current user (e.g.,
via your session/token helper) at the start of PUT, return a 401 if no user,
then verify ownership by querying ProjectTable for projectId and comparing its
owner/userId to the authenticated user's id (or include owner check in the
db.update where clause using eq(ProjectTable.ownerId, user.id)); if the user
does not own the project return 403 and do not perform the update, otherwise
proceed with the db.update(ProjectTable).set(...).where(...) and returning the
updated row.
- Around line 84-86: In the catch block that currently returns
NextResponse.json(error), do not expose the raw error object; instead log the
full error internally (e.g., console.error or your logger) and return a
sanitized JSON response such as NextResponse.json({ error: 'Internal server
error' } , { status: 500 }); update the catch in route.ts to use
NextResponse.json with a 500 status and a safe message while keeping internal
error details only in server logs.
In `@web/app/project/_shared/SectionSettings.tsx`:
- Around line 51-72: In GenerateNewScreen, the premium check shows a toast but
doesn’t stop execution; update the hasPremiumAccess branch to setLoading(false)
and return immediately (or otherwise short-circuit) to prevent making the
axios.post call when the user lacks access; reference the GenerateNewScreen
function and the hasPremiumAccess variable to locate where to add the early
return and cleanup.
---
Nitpick comments:
In `@web/app/api/user/route.ts`:
- Around line 21-30: The auth guard already guarantees user and
user.primaryEmailAddress are present, so remove unnecessary optional chaining in
the data object: replace user?.primaryEmailAddress?.emailAddress as string with
a direct access like user.primaryEmailAddress.emailAddress (keeping the string
cast if desired) when building data before calling db.insert on usersTable;
ensure you reference the same variables (user, data, usersTable, db.insert) so
the code reflects the guaranteed non-null fields.
🪄 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
Run ID: bf81ec85-2d6f-4bf4-8ff7-25aceb5b9fc7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
web/app/_shared/Header.tsxweb/app/_shared/Hero.tsxweb/app/_shared/ProjectList.tsxweb/app/api/generate-config/route.tsweb/app/api/project/route.tsweb/app/api/user/route.tsweb/app/layout.tsxweb/app/project/[projectid]/page.tsxweb/app/project/_shared/ProjectHeader.tsxweb/app/project/_shared/SectionSettings.tsxweb/components/pages/features.tsxweb/provider.tsx
Comment on lines
+93
to
98
| <Button variant="outline" className="rounded-full shadow-sm" asChild> | ||
| <SignInButton mode="modal">Log in</SignInButton> | ||
| </Button> | ||
| <Button className="rounded-full shadow-sm bg-blue-600 hover:bg-blue-700" asChild> | ||
| <SignInButton mode="modal">Get Started</SignInButton> | ||
| </Button> |
There was a problem hiding this comment.
Header prop API regression: configurable auth labels are ignored.
Line [93]-Line [98] and Line [135]-Line [140] hardcode button text, bypassing currentLoginText and currentGetStartedText. This breaks component customization and API stability.
Suggested fix
- <SignInButton mode="modal">Log in</SignInButton>
+ <SignInButton mode="modal">{currentLoginText}</SignInButton>
...
- <SignInButton mode="modal">Get Started</SignInButton>
+ <SignInButton mode="modal">{currentGetStartedText}</SignInButton>
...
- <SignInButton mode="modal">Log in</SignInButton>
+ <SignInButton mode="modal">{currentLoginText}</SignInButton>
...
- <SignInButton mode="modal">Get Started</SignInButton>
+ <SignInButton mode="modal">{currentGetStartedText}</SignInButton>Also applies to: 135-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/app/_shared/Header.tsx` around lines 93 - 98, The Header component is
ignoring configurable auth labels by hardcoding "Log in" and "Get Started"
inside the SignInButton usages; update the two occurrences (the
Button/SignInButton pairs around the SignInButton imports) to use the props
currentLoginText and currentGetStartedText (with sensible fallbacks, e.g.,
currentLoginText ?? "Log in" and currentGetStartedText ?? "Get Started") instead
of the literal strings so custom labels passed into Header are respected; ensure
both the first pair (variant="outline") and the second pair (bg-blue-600) are
changed and that SignInButton mode="modal" and asChild behavior remain
unchanged.
🧩 Analysis chain
🌐 Web query:
For @clerk/nextjsSignInButton, is this composition officially supported in React:Log in? Or should it be: Log in? Please provide Clerk docs examples for the recommended pattern.
💡 Result:
The second composition is the officially recommended pattern: Log in. The first one (Log in) is not supported, as SignInButton is unstyled and designed to wrap custom buttons as children, not to be forwarded as a child itself. Clerk's official Next.js docs for unstyled SignInButton explicitly state and exemplify: "You can create a custom button by wrapping your own button, or button text, in the component." Example: Custom sign in button The mode="modal" prop is supported (options: 'redirect' | 'modal') to open a modal instead of redirecting. This works with shadcn/ui Button (which supports asChild), as shown in community examples like Reddit posts using Login. No official docs mention an asChild prop on SignInButton for Next.js (unlike Astro SDK). Clerk's shadcn/ui integration examples use similar wrapping for other components.
Citations:
- 1: https://clerk.com/docs/nextjs/reference/components/unstyled/sign-in-button
- 2: https://clerk.com/docs/nextjs/components/unstyled/sign-in-button
- 3: https://clerk.com/docs/react/reference/components/unstyled/sign-in-button
- 4: https://clerk.com/docs/react/components/unstyled/sign-in-button
- 5: https://beta.clerk.com/docs/nextjs/reference/components/unstyled/sign-in-button
- 6: https://clerk.com/docs/guides/customizing-clerk/elements/examples/shadcn-ui
- 7: https://www.reddit.com/r/nextjs/comments/19czb47/clerk_signin_button_not_redirecting_to_signin/
🏁 Script executed:
# Check the file structure and read the specific lines
fd -t f Header.tsx | head -5Repository: ArslanYM/mockup
Length of output: 127
🏁 Script executed:
# Read the specific lines from Header.tsx to verify the composition pattern
cat -n web/app/_shared/Header.tsx | sed -n '90,145p'Repository: ArslanYM/mockup
Length of output: 3140
Fix unsupported Clerk SignInButton composition pattern in two locations.
The pattern <Button asChild><SignInButton> is not officially supported by Clerk. SignInButton is designed to wrap custom buttons, not to be wrapped by them. This will cause the modal triggers to fail.
Change to: <SignInButton mode="modal"><Button>...</Button></SignInButton>
Applies to lines 93-98 and 135-140.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/app/_shared/Header.tsx` around lines 93 - 98, The Clerk SignInButton is
being wrapped by your Button component (Button asChild > SignInButton), which is
unsupported and breaks modal triggering; instead wrap your Button with
SignInButton so SignInButton is the outer component. Locate the two occurrences
around Header.tsx where Button and SignInButton are composed (the blocks
containing Button className="rounded-full shadow-sm" and Button
className="rounded-full shadow-sm bg-blue-600 hover:bg-blue-700" and the similar
block at the later occurrence), and change the nesting so SignInButton
mode="modal" is the parent and your Button (with the same className/props and
text content "Log in" or "Get Started") is the child.
Comment on lines
32
to
+36
| useEffect(() => { | ||
| // eslint-disable-next-line react-hooks/set-state-in-effect | ||
| getProjectList(); | ||
| }, []); | ||
| if (user) { | ||
| getProjectList(); | ||
| } | ||
| }, [user]); |
There was a problem hiding this comment.
Reset project state when auth disappears.
Line [32]-Line [36] fetches only for signed-in users, but doesn’t clear projects when user becomes falsy. This can leave previously loaded projects visible after logout.
Suggested fix
useEffect(() => {
- if (user) {
- getProjectList();
- }
- }, [user]);
+ if (!user?.id) {
+ setProjects([]);
+ setLoading(false);
+ return;
+ }
+ getProjectList();
+ }, [user?.id]);📝 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
| useEffect(() => { | |
| // eslint-disable-next-line react-hooks/set-state-in-effect | |
| getProjectList(); | |
| }, []); | |
| if (user) { | |
| getProjectList(); | |
| } | |
| }, [user]); | |
| useEffect(() => { | |
| if (!user?.id) { | |
| setProjects([]); | |
| setLoading(false); | |
| return; | |
| } | |
| getProjectList(); | |
| }, [user?.id]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/app/_shared/ProjectList.tsx` around lines 32 - 36, The useEffect
currently only fetches with getProjectList() when user is truthy but doesn't
clear previous projects when user becomes falsy; update the useEffect to add an
else branch that resets the projects state (call the component's projects
setter, e.g. setProjects([]) or setProjectList([]) depending on the state name)
so that when user is null/undefined the projects are cleared instead of
lingering.
Comment on lines
+40
to
41
| {!projects || projects.length === 0 ? ( | ||
| <></> |
There was a problem hiding this comment.
Loading indicator is unreachable while list is empty.
Because Line [40]-Line [41] returns early for empty projects, the loader at Line [56]-Line [60] is not shown during the initial fetch (when projects is still empty).
Suggested fix
- {!projects || projects.length === 0 ? (
+ {loading ? (
+ <div className="col-span-full flex flex-col items-center justify-center py-10 gap-3 text-muted-foreground">
+ <Loader className="animate-spin size-8" />
+ <span className="font-medium animate-pulse">Loading projects...</span>
+ </div>
+ ) : !projects || projects.length === 0 ? (
<></>
) : (Also applies to: 56-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/app/_shared/ProjectList.tsx` around lines 40 - 41, The current
early-return uses {!projects || projects.length === 0} which hides the loading
indicator during initial fetch; change the render logic in ProjectList.tsx to
treat null/undefined projects as "loading" and only show the empty-state when
projects is an empty array. Locate the projects variable and the JSX that
renders the loader (the loader block currently at the bottom) and update the
condition so: if projects == null show the loader, else if projects.length === 0
show the empty-state, otherwise render the list.
Comment on lines
154
to
161
| <Button | ||
| onClick={() => { | ||
| takeScreenshot(); | ||
| }} | ||
| size={"sm"} | ||
| onClick={() => takeScreenshot()} | ||
| size={"default"} | ||
| variant={"outline"} | ||
| className="mt-3 cursor-pointer" | ||
| className="flex-1 cursor-pointer rounded-xl bg-white dark:bg-neutral-900 hover:bg-neutral-50 shadow-sm" | ||
| > | ||
| {" "} | ||
| <Camera /> Screenshot | ||
| <Camera className="mr-2 size-4" /> Snapshot | ||
| </Button> |
There was a problem hiding this comment.
Potential runtime error: takeScreenshot may be undefined.
The takeScreenshot prop is typed as optional (takeScreenshot?: any), but it's invoked directly without a null check. This will throw if the prop isn't passed.
🛡️ Proposed fix
<Button
- onClick={() => takeScreenshot()}
+ onClick={() => takeScreenshot?.()}
size={"default"}
variant={"outline"}📝 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
| <Button | |
| onClick={() => { | |
| takeScreenshot(); | |
| }} | |
| size={"sm"} | |
| onClick={() => takeScreenshot()} | |
| size={"default"} | |
| variant={"outline"} | |
| className="mt-3 cursor-pointer" | |
| className="flex-1 cursor-pointer rounded-xl bg-white dark:bg-neutral-900 hover:bg-neutral-50 shadow-sm" | |
| > | |
| {" "} | |
| <Camera /> Screenshot | |
| <Camera className="mr-2 size-4" /> Snapshot | |
| </Button> | |
| <Button | |
| onClick={() => takeScreenshot?.()} | |
| size={"default"} | |
| variant={"outline"} | |
| className="flex-1 cursor-pointer rounded-xl bg-white dark:bg-neutral-900 hover:bg-neutral-50 shadow-sm" | |
| > | |
| <Camera className="mr-2 size-4" /> Snapshot | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/app/project/_shared/SectionSettings.tsx` around lines 154 - 161, The
Snapshot Button calls the optional prop takeScreenshot directly which can throw
if undefined; update the SectionSettings UI to guard against a missing
takeScreenshot by checking its existence before invoking (e.g., onClick={() =>
takeScreenshot && takeScreenshot()} or conditional handler) and make the button
visually/operationally disabled when takeScreenshot is not provided (set
disabled prop and adjust className accordingly); ensure the prop reference is
takeScreenshot so you only change the click handler and button state, leaving
the optional typing intact or update the prop type to required if you intend it
always present.
Comment on lines
24
to
+28
| useEffect(() => { | ||
| CreateNewUser(); | ||
| }, []); | ||
| if (user) { | ||
| CreateNewUser(); | ||
| } | ||
| }, [user]); |
There was a problem hiding this comment.
Clear stale user context on sign-out and stabilize the effect dependency.
Line [24]-Line [28] only handles the signed-in path. If user becomes null, previous userDetail remains in context, which can expose stale identity data in the UI. Also, depending on the full user object can retrigger the POST unnecessarily.
Suggested fix
useEffect(() => {
- if (user) {
- CreateNewUser();
- }
- }, [user]);
+ if (!user?.id) {
+ setUserDetail(undefined);
+ return;
+ }
+ CreateNewUser();
+ }, [user?.id]);📝 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
| useEffect(() => { | |
| CreateNewUser(); | |
| }, []); | |
| if (user) { | |
| CreateNewUser(); | |
| } | |
| }, [user]); | |
| useEffect(() => { | |
| if (!user?.id) { | |
| setUserDetail(undefined); | |
| return; | |
| } | |
| CreateNewUser(); | |
| }, [user?.id]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/provider.tsx` around lines 24 - 28, The effect currently only creates a
new user when `user` exists, leaving stale `userDetail` when `user` becomes null
and also retriggers on full `user` object changes; update the effect to clear
`userDetail` when `user` is falsy and stabilize the dependency to an immutable
identifier (e.g. `user?.id` or `user?.uid`) so the POST only runs for real
identity changes: inside the effect do if (user?.id) CreateNewUser() else
setUserDetail(null), and change the dependency array from `[user]` to
`[user?.id]` (or `[user?.uid]`) so CreateNewUser and stale context handling are
deterministic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements