Staging#217
Merged
Merged
Conversation
- complete overhaul to the previous ui
- dedicated faq section - footer - features
- navbar polished - cta polished - faq console polished
- new login ui - Authenticating… Session Verified Workspace Located Loading Architecture… Ready.
…verall wibe of the site - Black dark sleek - removed the purple background
- the interview ui - the report card, the analysis - the reference archs
Revamp UI for landing page and dashboard with new features
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Review limit reached
More reviews will be available in 23 minutes and 15 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR performs a comprehensive visual and functional overhaul of SystemCraft. It introduces a new animated landing page ( ChangesSystemCraft Full Redesign
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
✅ Deploy Preview for system-craft-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Fix lint issues in CI pipeline
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/AuthCard.tsx (1)
117-140: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMirror the email flow's
syncFailedlifecycle in the provider path.This handler never clears stale
syncFailedstate before starting, and its catch block never marks provider sync failures. That creates two reachable bad states: after one email sync error, a later successful OAuth login gets stuck because the Lines 83-110 sequence bails out whilesyncFailedis stilltrue; and if/api/usersyncing throws after Firebase auth succeeds, the Lines 75-80 redirect guard can still send the user to/dashboardwithout a synced app user.🤖 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 `@components/AuthCard.tsx` around lines 117 - 140, The provider sign-in flow in AuthCard should mirror the email flow’s syncFailed lifecycle: clear any stale syncFailed state before starting the OAuth sign-in, and set syncFailed to true if syncUserWithDB fails after Firebase auth succeeds. Update the sign-in handler around signInFn, syncUserWithDB, and the catch/finally path so the redirect guard and post-login state checks behave consistently with the email flow.components/canvas/DesignCanvas.tsx (1)
378-390: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep
setSelectedNodesynchronized on every selection path.Only the normal node-selection flow updates the panels context. Undo/redo, delete, impacted-node clicks, and connection selection change local ids without refreshing/clearing
setSelectedNode, so the properties panel can show stale node data after the visible selection changed.Also applies to: 710-722, 1009-1015, 1133-1143
🤖 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 `@components/canvas/DesignCanvas.tsx` around lines 378 - 390, Keep the properties-panel selection state in sync with all selection changes by updating or clearing setSelectedNode everywhere a selection is changed, not just in the normal node-select path. In handleUndo, handleRedo, the delete/impacted-node handlers, and the connection-selection logic in DesignCanvas, make sure the same selection transitions also refresh the panels context so stale node details are not left behind. Use the existing selection handlers and local selection state in DesignCanvas to locate each path and apply the same setSelectedNode synchronization consistently.
🟡 Minor comments (15)
Copilot/.frontendskills/Auth.design.md-533-539 (1)
533-539: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix truncated/orphaned content at document end.
Lines 533-539 appear to be incomplete sentences or notes pasted without context ("A subtle 'Initializing Workspace...' animation before the form appears..."). These lack section headers and seem like draft notes that should be integrated into Section 14 (Loading & Interaction States) or removed.
🤖 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 `@Copilot/.frontendskills/Auth.design.md` around lines 533 - 539, The end of the Auth.design.md document contains orphaned draft notes with no section context, so either move them into the Loading & Interaction States section or remove them entirely. Use the existing Section 14 / “Loading & Interaction States” area and the nearby document structure to integrate the “Initializing Workspace…”, live status indicator, and post-login sequence notes as proper bullets or prose instead of leaving standalone fragments. Ensure the final document ends cleanly with complete, context-rich content rather than truncated sentences.app/practice/page.tsx-20-20 (1)
20-20: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winGate the template fetch on auth resolution.
useRequireAuth()now makes unauthenticated visits a redirect-only path, but the mount effect at Lines 25-50 still calls/api/templatesbefore auth settles. That wastes a request on every redirect and will surface noisy errors if the API later requires auth. Start the load only after!authLoading && isAuthenticated.Also applies to: 52-63
🤖 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 `@app/practice/page.tsx` at line 20, Gate the template loading effect in page.tsx on auth resolution: the current mount flow still fetches /api/templates before useRequireAuth() finishes, even though unauthenticated users will redirect. Update the effect(s) that load templates so they only run once authLoading is false and isAuthenticated is true, and keep the existing fetch logic in the same template-loading path so no request is made during redirect-only visits.components/dashboard/Sidebar.tsx-177-181 (1)
177-181: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRender the settings modal once outside
sidebarContent.Because
sidebarContentis reused for both the desktop aside (Line 188) and the mobile aside (Line 210), puttingProfileSettingsModalinside that subtree mounts two dialog instances whenever the mobile drawer is open. That leaves duplicate modal DOM and duplicate form ids on the page. Hoist the modal next to the two asides and render it once.🤖 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 `@components/dashboard/Sidebar.tsx` around lines 177 - 181, The ProfileSettingsModal is currently rendered inside sidebarContent, which causes it to mount twice when that shared subtree is used by both the desktop and mobile asides. Move the ProfileSettingsModal render out of sidebarContent in Sidebar so it sits alongside the two aside blocks and is instantiated only once, while keeping isProfileModalOpen and setIsProfileModalOpen handling unchanged.components/dashboard/DesignCard.tsx-265-286 (1)
265-286: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHide or disable the unimplemented menu items.
"Rename" and "Duplicate" currently just close the menu, so they look functional but do nothing. That is a confusing dead-end in a primary card action menu.
🤖 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 `@components/dashboard/DesignCard.tsx` around lines 265 - 286, The “Rename” and “Duplicate” actions in DesignCard’s menu are currently dead-end buttons that only close the dropdown, so either hide them or disable them until the handlers are implemented. Update the menu item rendering in DesignCard so these options are not presented as active actions unless their onClick logic actually performs rename/duplicate behavior, and keep the menu’s existing close behavior only for valid actions.app/dashboard/page.tsx-311-325 (1)
311-325: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWire these layout controls or present them as disabled placeholders.
These buttons and the sort
<select>currently have noonClick/onChangebehavior, so they advertise grid/list and sorting options that never change the view. Either connect them to local state or mark them as coming soon to avoid a dead control surface.🤖 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 `@app/dashboard/page.tsx` around lines 311 - 325, The layout controls in the dashboard header are currently decorative only, so either wire the grid/list buttons and the sort select to real state in dashboard/page.tsx or clearly disable/label them as coming soon. Update the relevant controls in the layout controls block to use handlers and state for view mode and sorting, or make them non-interactive placeholders so they do not imply functionality that is not implemented.app/dashboard/analytics/page.tsx-139-145 (1)
139-145: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPoint this CTA at the interview flow.
This empty state tells the user to “Start Interview”, but the link still routes to
/dashboard. That makes the first-run path misleading; send it to/interviewor rename the CTA to match the dashboard destination.🤖 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 `@app/dashboard/analytics/page.tsx` around lines 139 - 145, The CTA in the analytics empty state is misleading because the Link labeled “Start Interview” still points to the dashboard. Update the Link in the analytics page empty-state section so its href matches the interview flow destination, or if you keep the current destination, rename the CTA text to match; use the Link element and the “Start Interview” label as the locating symbols.components/dashboard/Header.tsx-25-38 (1)
25-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMatch the keyboard hints to the behavior you actually implement.
The palette footer promises
↑↓navigation and⏎execution, but this handler only supportsCtrl/Cmd+KandEscape. Either add active-item arrow/Enter handling or remove those hints for now; as-is the palette advertises controls that do nothing.Also applies to: 280-284
🤖 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 `@components/dashboard/Header.tsx` around lines 25 - 38, The command palette shortcuts in Header should match the behavior actually implemented: the current handleKeyDown/useEffect only supports Cmd/Ctrl+K and Escape, so either add real ↑↓ active-item navigation and Enter/⏎ execution in the command palette logic or remove those footer hints until they work. Update the relevant Header.tsx keyboard handling and any palette item selection/submit handlers so the advertised controls in the footer are functional.components/dashboard/Header.tsx-188-201 (1)
188-201: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDon’t ship a dead “Help & Support” action.
Line 198 renders this as a clickable button, but it has no handler or destination, so users can never complete the action. Wire it to a route/modal or render it as non-interactive until the target exists.
🤖 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 `@components/dashboard/Header.tsx` around lines 188 - 201, The Header dropdown’s “Help & Support” control is interactive but currently has no behavior attached. Update the button in Header so it either uses the same open/close pattern as the Settings action to launch the appropriate modal/route, or render it as a non-interactive element until that destination exists. Use the existing dropdown button markup near the Settings button as the reference point and keep the user flow consistent with the current setIsDropdownOpen handling.components/canvas/CanvasPanelsContext.tsx-67-75 (1)
67-75: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHydrate
activeViewbefore the first render. On reloads that should reopen whiteboard, the provider still starts inarchitecture, soDesignCanvasmounts the wrong branch once before the effect switches it over. Gate the UI until the saved value is loaded, or restore it during initialization.🤖 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 `@components/canvas/CanvasPanelsContext.tsx` around lines 67 - 75, The activeView state in CanvasPanelsContext currently initializes to architecture and only reads sessionStorage inside useEffect, so DesignCanvas renders the wrong branch before hydration. Update CanvasPanelsContext so the saved canvasActiveView is restored during state initialization (or add a loading gate until it is read) using the activeView/setActiveViewRaw state and the existing sessionStorage key, ensuring the first render matches the persisted tab.Source: Linters/SAST tools
components/canvas/DesignCanvas.tsx-300-301 (1)
300-301: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the hard-coded geometry to match the new 160×64 node cards.
The renderer now uses
w-[160px] h-[64px], but auto-fit, edge anchors, and drop centering still use the old144/56/72/28values. That leaves links anchored inside the card, mis-centers drops, and underestimates the read-only bounding box.Also applies to: 415-418, 445-446, 469-470, 998-998
🤖 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 `@components/canvas/DesignCanvas.tsx` around lines 300 - 301, The geometry constants in DesignCanvas are still using the old node-card size, so update the hard-coded width/height and any derived anchor/centering values to match the new 160×64 cards. Fix the related calculations in the auto-fit logic, edge anchor math, drop centering, and read-only bounding box paths by adjusting the constants and expressions around NODE_WIDTH, NODE_HEIGHT, and the other old 72/28-based offsets so they reference the new card dimensions consistently.app/globals.css-183-185 (1)
183-185: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGate smooth scrolling behind
prefers-reduced-motion.This forces animated anchor jumps for users who explicitly disabled motion, and the new landing-page section links will hit this path immediately.
Suggested fix
-html { - scroll-behavior: smooth; -} +@media (prefers-reduced-motion: no-preference) { + html { + scroll-behavior: smooth; + } +}🤖 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 `@app/globals.css` around lines 183 - 185, The global html scroll-behavior rule is forcing smooth scrolling for all users, including those who prefer reduced motion. Update the html rule in globals.css to apply smooth scrolling only when the user has not requested reduced motion, using a prefers-reduced-motion media query around the existing scroll-behavior setting so the landing-page section links still work without animating for motion-sensitive users.components/landing/engine/InfrastructureEnvironment.tsx-8-8 (1)
8-8: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHide the ambient background from the accessibility tree.
This subtree is decorative only, but
BlueprintGridcontains real SVG text nodes. Withoutaria-hidden, screen readers can pick up that noise before the actual landing-page content.Suggested fix
- <div className="fixed inset-0 w-full h-full bg-[`#020306`] overflow-hidden pointer-events-none select-none z-0"> + <div + aria-hidden="true" + className="fixed inset-0 w-full h-full bg-[`#020306`] overflow-hidden pointer-events-none select-none z-0" + >🤖 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 `@components/landing/engine/InfrastructureEnvironment.tsx` at line 8, The decorative ambient background in InfrastructureEnvironment should be hidden from assistive tech because BlueprintGrid and its SVG text are non-content noise. Update the top-level wrapper in InfrastructureEnvironment to include aria-hidden so the entire background subtree is excluded from the accessibility tree while keeping the visual effect unchanged.components/landing/SystemCraftLanding.tsx-944-962 (1)
944-962: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose accordion state to assistive tech.
These buttons toggle content visually, but they never announce whether the panel is open. Add
aria-expandedand wire each trigger to its panel witharia-controls/id.Suggested fix
<button + type="button" onClick={() => setOpenIndex(isOpen ? null : i)} + aria-expanded={isOpen} + aria-controls={`${activeCategory}-faq-${i}`} className="flex w-full items-center justify-between p-4 text-left text-sm text-cyan-400/80 transition-all duration-[400ms] ease-[cubic-bezier(0.16,1,0.3,1)] hover:bg-cyan-950/10 hover:text-cyan-300" > ... <motion.div + id={`${activeCategory}-faq-${i}`} initial={{ height: 0, opacity: 0 }} animate={{ height: "auto", opacity: 1 }} exit={{ height: 0, opacity: 0 }}🤖 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 `@components/landing/SystemCraftLanding.tsx` around lines 944 - 962, Update the FAQ accordion trigger in SystemCraftLanding so assistive tech can detect its state: the button that toggles `setOpenIndex(isOpen ? null : i)` should expose `aria-expanded`, and each panel rendered inside `AnimatePresence`/`motion.div` should have a stable `id` that the button references via `aria-controls`. Keep the identifiers tied to the FAQ item index or similar unique value so the trigger and its corresponding panel stay linked even if the list changes.components/landing/SystemCraftLanding.tsx-768-770 (1)
768-770: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse Tailwind arbitrary easing syntax here
cubic-bezier(0.16,1,0.3,1)is a bare class token, so Tailwind won’t apply it. These transitions fall back to the default timing curve; useease-[cubic-bezier(0.16,1,0.3,1)]here and in the matching category buttons below.Suggested fix
- className={`group relative flex items-center px-6 py-4 text-left transition-all duration-[400ms] cubic-bezier(0.16,1,0.3,1) ${ + className={`group relative flex items-center px-6 py-4 text-left transition-all duration-[400ms] ease-[cubic-bezier(0.16,1,0.3,1)] ${🤖 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 `@components/landing/SystemCraftLanding.tsx` around lines 768 - 770, The transition class in SystemCraftLanding’s item and matching category button styles is using a bare cubic-bezier token that Tailwind will ignore. Update the className strings around the active item styling and the category button variants to use Tailwind’s arbitrary easing syntax with ease-[cubic-bezier(0.16,1,0.3,1)] so the intended timing curve is applied consistently.components/landing/SystemCraftLanding.tsx-1071-1100 (1)
1071-1100: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRead reduced-motion from the media query on render
setReduceMotion(matches)runs after the first paint, so reduced-motion users still see the animated state briefly, and the component won’t react if the OS setting changes later.useReducedMotion()or a subscribed media-query hook avoids both.🤖 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 `@components/landing/SystemCraftLanding.tsx` around lines 1071 - 1100, The reduced-motion flag in SystemCraftLanding is being set only after mount via the current useEffect, so the initial render can still animate and later OS preference changes are missed. Replace the local reduceMotion state + matchMedia setup in SystemCraftLanding with a subscribed media-query hook such as useReducedMotion() or an equivalent reactive hook, and keep the animation/interval effects keyed off that value so the phase sequence and cluster rotation stop immediately when reduced motion is enabled.Source: Linters/SAST tools
🧹 Nitpick comments (3)
Copilot/.frontendskills/Updates.md (1)
246-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix hyphenation: "slow-moving gradients".
Change "slow moving gradients" to "slow-moving gradients" for correct compound modifier grammar. This prevents propagation to code comments or derived documentation.
🤖 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 `@Copilot/.frontendskills/Updates.md` at line 246, The phrase in the Updates.md content should use the correct compound modifier form: change “slow moving gradients” to “slow-moving gradients.” Update the wording where it appears in the documentation so any copied or derived text also uses the hyphenated form.components/auth/AuthLayout.tsx (1)
1-24: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winKeep the auth shell server-rendered.
Neither this component nor
components/auth/SystemStatusPanel.tsxuses hooks or browser APIs, so this client boundary ships the decorative auth chrome to the browser unnecessarily. Moving the client boundary down toAuthCardpreserves behavior with less JS on every auth visit.♻️ Proposed change
-"use client";If
components/auth/SystemStatusPanel.tsxstays hook-free, drop its client directive too.🤖 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 `@components/auth/AuthLayout.tsx` around lines 1 - 24, AuthLayout is marked as a client component even though it and SystemStatusPanel do not use hooks or browser APIs, which unnecessarily pushes the auth shell to the client. Remove the client boundary from AuthLayout and keep the shell server-rendered, then move any needed client-only behavior down to AuthCard so the existing layout and rendering stay the same with less JS. Also verify SystemStatusPanel remains hook-free and remove its client directive too if it is still present.components/dashboard/DesignCard.tsx (1)
91-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove or use the dead mock metric memos.
mockLatencyandmockComplexityare never read, so this adds extra memoization work and leaves the file with active lint/typecheck warnings. Either surface them in the card UI or delete them for now.🤖 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 `@components/dashboard/DesignCard.tsx` around lines 91 - 98, The DesignCard component has dead mock metric memo values that are computed but never used. In DesignCard, either wire mockLatency and mockComplexity into the rendered UI so they are actually displayed, or remove the useMemo blocks entirely if they are not needed yet. Keep the fix localized to the DesignCard component and ensure no unused-variable lint/typecheck warnings remain.Source: Linters/SAST tools
🤖 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 `@app/dashboard/reference-architectures/`[id]/page.tsx:
- Around line 351-365: The markdown rendering path in the reference architecture
page is passing raw model output from renderMarkdown() directly into
sanitizeHtml(), which can still allow unsafe allowed tags/styles through to
dangerouslySetInnerHTML. Update the content pipeline in the page component to
escape or normalize the model text before markdown conversion, or replace the
current renderMarkdown()/sanitizeHtml() flow with a vetted
markdown-plus-sanitizer pipeline so only renderer-generated tags survive. Keep
the fix localized to the markdown body rendering block and preserve the
streaming indicator behavior.
In `@app/practice/`[id]/page.tsx:
- Around line 181-186: The back control in the practice page header is
implemented as a clickable div with router.push, which makes it inaccessible to
keyboard users. Replace the div with a real button or link in the same spot, and
wire the navigation through the existing router.push('/practice') behavior so
the control remains focusable and operable. Keep the visual styling and the
arrow_back icon, but ensure the accessible element is the one handling the
action.
In `@app/practice/page.tsx`:
- Around line 65-69: Add a mobile sidebar trigger for the practice page because
Sidebar is mounted without any way to open its drawer on mobile. Update
app/practice/page.tsx to include a toggle that drives useSidebar().isOpen, or
reuse the same sidebar toggle behavior used in components/dashboard/Header.tsx,
so mobile users can open navigation and settings from this page.
In `@components/AuthCard.tsx`:
- Around line 123-126: The null-check after calling signInFn in AuthCard should
not be treated as a failed login, because signInWithGoogle and signInWithGitHub
can return null when initiating a redirect flow. Update the sign-in handling so
redirect-based providers bypass the failure branch, and only
setSignInError/setAuthPhase('idle') when a real authentication error occurs; use
the existing signInFn, provider, and auth phase logic in AuthCard to keep the
flow in progress during redirects.
In `@components/canvas/DesignCanvas.tsx`:
- Around line 725-759: In DesignCanvas’s keyboard handler inside the useEffect
block, undo/redo shortcuts are still firing even when an input, textarea, or
contenteditable editor owns focus, so they affect the canvas graph while editing
text. Add the same focused-editor guard used for Delete/Backspace and tool
shortcuts before handling the Ctrl/Cmd+Z and Ctrl/Cmd+Y branches, and keep the
logic in handleKeyDown from triggering handleUndo or handleRedo when the
whiteboard text tool or any inline editor is active.
- Around line 544-547: The drag offset calculation in DesignCanvas’s node drag
handler is using viewport coordinates from e.clientX/e.clientY, so it ignores
the canvas element’s position and pan state. Update the drag math in the
relevant drag-start logic (and the matching drag update path referenced by the
same node-drag code) to convert pointer coordinates into canvas space by
subtracting canvasRef.getBoundingClientRect() offsets and accounting for
panOffset before applying zoom. Make the change in the DesignCanvas drag
handlers so setDragOffset and the related position updates use canvas-local
coordinates consistently.
In `@components/canvas/PropertiesPanel.tsx`:
- Around line 130-149: The inspector controls in PropertiesPanel are missing
accessible names and state, so update the node configuration UI to associate
each visible label with its control using matching id/htmlFor on the relevant
select and slider inputs, and expose the read-replica toggle with switch
semantics via role="switch" and aria-checked (or replace it with a native
checkbox). Apply the same accessibility wiring to the other controls in this
panel, especially the node count/storage sliders and the select/toggle elements
in the referenced section, so screen readers announce each control clearly.
In `@components/dashboard/DesignCard.tsx`:
- Around line 146-337: The DesignCard component currently nests the menu trigger
and dropdown actions inside the outer Link, which mixes interactive controls
with navigation. Update DesignCard so the clickable card surface remains inside
the Link, but the menu button and dropdown are rendered outside that linked
region; use the existing menuRef, isMenuOpen, setIsMenuOpen, and
setIsConfirmOpen logic to preserve behavior while separating navigation from
card actions.
---
Outside diff comments:
In `@components/AuthCard.tsx`:
- Around line 117-140: The provider sign-in flow in AuthCard should mirror the
email flow’s syncFailed lifecycle: clear any stale syncFailed state before
starting the OAuth sign-in, and set syncFailed to true if syncUserWithDB fails
after Firebase auth succeeds. Update the sign-in handler around signInFn,
syncUserWithDB, and the catch/finally path so the redirect guard and post-login
state checks behave consistently with the email flow.
In `@components/canvas/DesignCanvas.tsx`:
- Around line 378-390: Keep the properties-panel selection state in sync with
all selection changes by updating or clearing setSelectedNode everywhere a
selection is changed, not just in the normal node-select path. In handleUndo,
handleRedo, the delete/impacted-node handlers, and the connection-selection
logic in DesignCanvas, make sure the same selection transitions also refresh the
panels context so stale node details are not left behind. Use the existing
selection handlers and local selection state in DesignCanvas to locate each path
and apply the same setSelectedNode synchronization consistently.
---
Minor comments:
In `@app/dashboard/analytics/page.tsx`:
- Around line 139-145: The CTA in the analytics empty state is misleading
because the Link labeled “Start Interview” still points to the dashboard. Update
the Link in the analytics page empty-state section so its href matches the
interview flow destination, or if you keep the current destination, rename the
CTA text to match; use the Link element and the “Start Interview” label as the
locating symbols.
In `@app/dashboard/page.tsx`:
- Around line 311-325: The layout controls in the dashboard header are currently
decorative only, so either wire the grid/list buttons and the sort select to
real state in dashboard/page.tsx or clearly disable/label them as coming soon.
Update the relevant controls in the layout controls block to use handlers and
state for view mode and sorting, or make them non-interactive placeholders so
they do not imply functionality that is not implemented.
In `@app/globals.css`:
- Around line 183-185: The global html scroll-behavior rule is forcing smooth
scrolling for all users, including those who prefer reduced motion. Update the
html rule in globals.css to apply smooth scrolling only when the user has not
requested reduced motion, using a prefers-reduced-motion media query around the
existing scroll-behavior setting so the landing-page section links still work
without animating for motion-sensitive users.
In `@app/practice/page.tsx`:
- Line 20: Gate the template loading effect in page.tsx on auth resolution: the
current mount flow still fetches /api/templates before useRequireAuth()
finishes, even though unauthenticated users will redirect. Update the effect(s)
that load templates so they only run once authLoading is false and
isAuthenticated is true, and keep the existing fetch logic in the same
template-loading path so no request is made during redirect-only visits.
In `@components/canvas/CanvasPanelsContext.tsx`:
- Around line 67-75: The activeView state in CanvasPanelsContext currently
initializes to architecture and only reads sessionStorage inside useEffect, so
DesignCanvas renders the wrong branch before hydration. Update
CanvasPanelsContext so the saved canvasActiveView is restored during state
initialization (or add a loading gate until it is read) using the
activeView/setActiveViewRaw state and the existing sessionStorage key, ensuring
the first render matches the persisted tab.
In `@components/canvas/DesignCanvas.tsx`:
- Around line 300-301: The geometry constants in DesignCanvas are still using
the old node-card size, so update the hard-coded width/height and any derived
anchor/centering values to match the new 160×64 cards. Fix the related
calculations in the auto-fit logic, edge anchor math, drop centering, and
read-only bounding box paths by adjusting the constants and expressions around
NODE_WIDTH, NODE_HEIGHT, and the other old 72/28-based offsets so they reference
the new card dimensions consistently.
In `@components/dashboard/DesignCard.tsx`:
- Around line 265-286: The “Rename” and “Duplicate” actions in DesignCard’s menu
are currently dead-end buttons that only close the dropdown, so either hide them
or disable them until the handlers are implemented. Update the menu item
rendering in DesignCard so these options are not presented as active actions
unless their onClick logic actually performs rename/duplicate behavior, and keep
the menu’s existing close behavior only for valid actions.
In `@components/dashboard/Header.tsx`:
- Around line 25-38: The command palette shortcuts in Header should match the
behavior actually implemented: the current handleKeyDown/useEffect only supports
Cmd/Ctrl+K and Escape, so either add real ↑↓ active-item navigation and Enter/⏎
execution in the command palette logic or remove those footer hints until they
work. Update the relevant Header.tsx keyboard handling and any palette item
selection/submit handlers so the advertised controls in the footer are
functional.
- Around line 188-201: The Header dropdown’s “Help & Support” control is
interactive but currently has no behavior attached. Update the button in Header
so it either uses the same open/close pattern as the Settings action to launch
the appropriate modal/route, or render it as a non-interactive element until
that destination exists. Use the existing dropdown button markup near the
Settings button as the reference point and keep the user flow consistent with
the current setIsDropdownOpen handling.
In `@components/dashboard/Sidebar.tsx`:
- Around line 177-181: The ProfileSettingsModal is currently rendered inside
sidebarContent, which causes it to mount twice when that shared subtree is used
by both the desktop and mobile asides. Move the ProfileSettingsModal render out
of sidebarContent in Sidebar so it sits alongside the two aside blocks and is
instantiated only once, while keeping isProfileModalOpen and
setIsProfileModalOpen handling unchanged.
In `@components/landing/engine/InfrastructureEnvironment.tsx`:
- Line 8: The decorative ambient background in InfrastructureEnvironment should
be hidden from assistive tech because BlueprintGrid and its SVG text are
non-content noise. Update the top-level wrapper in InfrastructureEnvironment to
include aria-hidden so the entire background subtree is excluded from the
accessibility tree while keeping the visual effect unchanged.
In `@components/landing/SystemCraftLanding.tsx`:
- Around line 944-962: Update the FAQ accordion trigger in SystemCraftLanding so
assistive tech can detect its state: the button that toggles
`setOpenIndex(isOpen ? null : i)` should expose `aria-expanded`, and each panel
rendered inside `AnimatePresence`/`motion.div` should have a stable `id` that
the button references via `aria-controls`. Keep the identifiers tied to the FAQ
item index or similar unique value so the trigger and its corresponding panel
stay linked even if the list changes.
- Around line 768-770: The transition class in SystemCraftLanding’s item and
matching category button styles is using a bare cubic-bezier token that Tailwind
will ignore. Update the className strings around the active item styling and the
category button variants to use Tailwind’s arbitrary easing syntax with
ease-[cubic-bezier(0.16,1,0.3,1)] so the intended timing curve is applied
consistently.
- Around line 1071-1100: The reduced-motion flag in SystemCraftLanding is being
set only after mount via the current useEffect, so the initial render can still
animate and later OS preference changes are missed. Replace the local
reduceMotion state + matchMedia setup in SystemCraftLanding with a subscribed
media-query hook such as useReducedMotion() or an equivalent reactive hook, and
keep the animation/interval effects keyed off that value so the phase sequence
and cluster rotation stop immediately when reduced motion is enabled.
In `@Copilot/.frontendskills/Auth.design.md`:
- Around line 533-539: The end of the Auth.design.md document contains orphaned
draft notes with no section context, so either move them into the Loading &
Interaction States section or remove them entirely. Use the existing Section 14
/ “Loading & Interaction States” area and the nearby document structure to
integrate the “Initializing Workspace…”, live status indicator, and post-login
sequence notes as proper bullets or prose instead of leaving standalone
fragments. Ensure the final document ends cleanly with complete, context-rich
content rather than truncated sentences.
---
Nitpick comments:
In `@components/auth/AuthLayout.tsx`:
- Around line 1-24: AuthLayout is marked as a client component even though it
and SystemStatusPanel do not use hooks or browser APIs, which unnecessarily
pushes the auth shell to the client. Remove the client boundary from AuthLayout
and keep the shell server-rendered, then move any needed client-only behavior
down to AuthCard so the existing layout and rendering stay the same with less
JS. Also verify SystemStatusPanel remains hook-free and remove its client
directive too if it is still present.
In `@components/dashboard/DesignCard.tsx`:
- Around line 91-98: The DesignCard component has dead mock metric memo values
that are computed but never used. In DesignCard, either wire mockLatency and
mockComplexity into the rendered UI so they are actually displayed, or remove
the useMemo blocks entirely if they are not needed yet. Keep the fix localized
to the DesignCard component and ensure no unused-variable lint/typecheck
warnings remain.
In `@Copilot/.frontendskills/Updates.md`:
- Line 246: The phrase in the Updates.md content should use the correct compound
modifier form: change “slow moving gradients” to “slow-moving gradients.” Update
the wording where it appears in the documentation so any copied or derived text
also uses the hyphenated form.
🪄 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: c458cf64-470c-44f7-9ace-335035150610
⛔ Files ignored due to path filters (3)
Copilot/.frontendskills/SystemCraft Landing Page Creative Direction.docxis excluded by!**/*.docxCopilot/.frontendskills/SystemCraft Landing Page Creative Direction.pdfis excluded by!**/*.pdfpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (46)
Copilot/.frontendskills/Agents.mdCopilot/.frontendskills/Auth.design.mdCopilot/.frontendskills/Canvas.mdCopilot/.frontendskills/Updates.mdapp/api/designs/route.tsapp/api/reference-architectures/analyze/route.tsapp/canvas/[id]/page.tsxapp/dashboard/analytics/page.tsxapp/dashboard/layout.tsxapp/dashboard/page.tsxapp/dashboard/reference-architectures/[id]/page.tsxapp/dashboard/reference-architectures/page.tsxapp/dashboard/report-card/page.tsxapp/globals.cssapp/interview/[id]/result/page.tsxapp/interview/page.tsxapp/layout.tsxapp/login/page.tsxapp/page.tsxapp/practice/[id]/page.tsxapp/practice/page.tsxapp/signup/page.tsxcomponents/AuthCard.tsxcomponents/auth/AuthLayout.tsxcomponents/auth/SystemStatusPanel.tsxcomponents/canvas/AIFeedbackPanel.tsxcomponents/canvas/CanvasHeader.tsxcomponents/canvas/CanvasPanelsContext.tsxcomponents/canvas/ComponentPalette.tsxcomponents/canvas/DesignCanvas.tsxcomponents/canvas/PropertiesPanel.tsxcomponents/canvas/SimulationControls.tsxcomponents/canvas/Whiteboard.tsxcomponents/canvas/WhiteboardClient.tsxcomponents/dashboard/CreateDesignCard.tsxcomponents/dashboard/DesignCard.tsxcomponents/dashboard/Header.tsxcomponents/dashboard/Hero.tsxcomponents/dashboard/Sidebar.tsxcomponents/landing/SystemCraftLanding.tsxcomponents/landing/engine/InfrastructureEnvironment.tsxcomponents/landing/engine/renderers/BlueprintGrid.tsxcomponents/landing/engine/renderers/LightingRenderer.tsxcomponents/landing/engine/renderers/NoiseRenderer.tsxpackage.jsonsrc/lib/redis.ts
Comment on lines
+181
to
+186
| <div | ||
| className="flex items-center justify-center size-8 rounded-lg border border-white/[0.05] bg-white/[0.02] text-white/40 hover:text-white hover:bg-white/[0.04] transition-all cursor-pointer" | ||
| onClick={() => router.push('/practice')} | ||
| > | ||
| <span className="material-symbols-outlined text-[16px]">arrow_back</span> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a real button or link for the back control.
This is clickable navigation implemented as a div, so it is not focusable or keyboard-operable. That makes the header back action inaccessible.
Suggested fix
- <div
- className="flex items-center justify-center size-8 rounded-lg border border-white/[0.05] bg-white/[0.02] text-white/40 hover:text-white hover:bg-white/[0.04] transition-all cursor-pointer"
- onClick={() => router.push('/practice')}
- >
+ <button
+ type="button"
+ className="flex items-center justify-center size-8 rounded-lg border border-white/[0.05] bg-white/[0.02] text-white/40 hover:text-white hover:bg-white/[0.04] transition-all cursor-pointer"
+ onClick={() => router.push('/practice')}
+ aria-label="Back to practice templates"
+ >
<span className="material-symbols-outlined text-[16px]">arrow_back</span>
- </div>
+ </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.
Suggested change
| <div | |
| className="flex items-center justify-center size-8 rounded-lg border border-white/[0.05] bg-white/[0.02] text-white/40 hover:text-white hover:bg-white/[0.04] transition-all cursor-pointer" | |
| onClick={() => router.push('/practice')} | |
| > | |
| <span className="material-symbols-outlined text-[16px]">arrow_back</span> | |
| </div> | |
| <button | |
| type="button" | |
| className="flex items-center justify-center size-8 rounded-lg border border-white/[0.05] bg-white/[0.02] text-white/40 hover:text-white hover:bg-white/[0.04] transition-all cursor-pointer" | |
| onClick={() => router.push('/practice')} | |
| aria-label="Back to practice templates" | |
| > | |
| <span className="material-symbols-outlined text-[16px]">arrow_back</span> | |
| </button> |
🤖 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 `@app/practice/`[id]/page.tsx around lines 181 - 186, The back control in the
practice page header is implemented as a clickable div with router.push, which
makes it inaccessible to keyboard users. Replace the div with a real button or
link in the same spot, and wire the navigation through the existing
router.push('/practice') behavior so the control remains focusable and operable.
Keep the visual styling and the arrow_back icon, but ensure the accessible
element is the one handling the action.
Comment on lines
146
to
337
| <Link href={`/canvas/${id}`}> | ||
| <div className="group flex flex-col rounded-xl bg-white dark:bg-dashboard-card border border-slate-200 dark:border-transparent hover:border-primary/30 dark:hover:border-primary/50 hover:shadow-xl dark:hover:shadow-primary/5 transition-all overflow-hidden cursor-pointer h-full"> | ||
| <div className="h-40 w-full bg-slate-100 dark:bg-dashboard-surface relative overflow-hidden"> | ||
| <div | ||
| className="absolute inset-0 bg-cover bg-center opacity-80 group-hover:opacity-100 transition-opacity" | ||
| style={{ backgroundImage: `url("${thumbnailUrl}")` }} | ||
| /> | ||
| <div className="absolute top-3 right-3" ref={menuRef}> | ||
| <div className="group flex flex-col rounded-xl bg-[#0c0d16]/30 border border-white/[0.04] hover:border-white/[0.12] hover:bg-[#0c0d16]/50 shadow-[0_4px_20px_rgba(0,0,0,0.4)] hover:shadow-[0_20px_40px_rgba(0,0,0,0.6),inset_0_1px_0_rgba(255,255,255,0.02)] transition-all duration-300 overflow-hidden cursor-pointer h-full select-none relative"> | ||
|
|
||
| {/* Top highlight */} | ||
| <div className="absolute inset-x-0 top-0 h-px bg-gradient-to-r from-transparent via-white/[0.05] group-hover:via-white/[0.1] to-transparent transition-all duration-300" /> | ||
|
|
||
| {/* Living Topology Preview Canvas */} | ||
| <div className="h-34 w-full bg-[#060810]/80 relative overflow-hidden flex items-center justify-center p-2"> | ||
| <svg | ||
| viewBox="0 0 100 70" | ||
| className="absolute inset-0 h-full w-full opacity-55 group-hover:opacity-85 transition-opacity duration-300" | ||
| aria-hidden="true" | ||
| > | ||
| <defs> | ||
| <linearGradient id={`edgeGrad-${id}`} x1="0%" x2="100%" y1="0%" y2="0%"> | ||
| <stop offset="0%" stopColor="#22d3ee" stopOpacity="0.2" /> | ||
| <stop offset="100%" stopColor="#6366f1" stopOpacity="0.08" /> | ||
| </linearGradient> | ||
| </defs> | ||
|
|
||
| {/* Connections */} | ||
| {edges.map((edge, i) => { | ||
| const from = nodes[edge.from]; | ||
| const to = nodes[edge.to]; | ||
| if (!from || !to) return null; | ||
| return ( | ||
| <line | ||
| key={i} | ||
| x1={from.x} y1={from.y} | ||
| x2={to.x} y2={to.y} | ||
| stroke={`url(#edgeGrad-${id})`} | ||
| strokeWidth="0.55" | ||
| /> | ||
| ); | ||
| })} | ||
|
|
||
| {/* Packets */} | ||
| {edges.map((edge, i) => { | ||
| const from = nodes[edge.from]; | ||
| const to = nodes[edge.to]; | ||
| if (!from || !to) return null; | ||
| const dur = 3.5 + i * 1.2; | ||
| return ( | ||
| <circle | ||
| key={`p-${i}`} | ||
| r="0.5" | ||
| fill="#22d3ee" | ||
| opacity="0.5" | ||
| > | ||
| <animateMotion | ||
| dur={`${dur}s`} | ||
| repeatCount="indefinite" | ||
| path={`M${from.x},${from.y} L${to.x},${to.y}`} | ||
| /> | ||
| </circle> | ||
| ); | ||
| })} | ||
|
|
||
| {/* Nodes */} | ||
| {nodes.map((node, i) => ( | ||
| <g key={i}> | ||
| {/* Breath pulse */} | ||
| <circle | ||
| cx={node.x} cy={node.y} r="3.5" | ||
| fill="none" | ||
| stroke="#22d3ee" | ||
| strokeWidth="0.25" | ||
| opacity="0.06" | ||
| > | ||
| <animate | ||
| attributeName="r" | ||
| values="3.2;4.2;3.2" | ||
| dur="6s" | ||
| repeatCount="indefinite" | ||
| /> | ||
| <animate | ||
| attributeName="opacity" | ||
| values="0.03;0.1;0.03" | ||
| dur="6s" | ||
| repeatCount="indefinite" | ||
| /> | ||
| </circle> | ||
| <circle | ||
| cx={node.x} cy={node.y} r="1.6" | ||
| fill="#080a12" | ||
| stroke="rgba(34,211,238,0.2)" | ||
| strokeWidth="0.4" | ||
| /> | ||
| </g> | ||
| ))} | ||
| </svg> | ||
|
|
||
| {/* Micro Node labels */} | ||
| {nodes.map((node, i) => ( | ||
| <span | ||
| key={i} | ||
| className="absolute text-[5px] font-mono tracking-wider text-white/20 uppercase pointer-events-none -translate-x-1/2" | ||
| style={{ left: `${node.x}%`, top: `${node.y + 4}%` }} | ||
| > | ||
| {node.label} | ||
| </span> | ||
| ))} | ||
|
|
||
| {/* Menu overlay toggles */} | ||
| <div className="absolute top-2.5 right-2.5" ref={menuRef}> | ||
| <button | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| setIsMenuOpen(!isMenuOpen); | ||
| }} | ||
| className="bg-black/40 backdrop-blur-md hover:bg-black/60 text-white p-1.5 rounded-lg opacity-0 group-hover:opacity-100 transition-opacity cursor-pointer" | ||
| className="bg-[#0c0d16]/80 hover:bg-[#0c0d16] text-white/50 hover:text-white/80 p-1.5 rounded-lg border border-white/[0.05] shadow-md transition-all cursor-pointer" | ||
| > | ||
| <span className="material-symbols-outlined text-[18px]">more_horiz</span> | ||
| <span className="material-symbols-outlined text-[15px]">more_horiz</span> | ||
| </button> | ||
|
|
||
| {/* Dropdown Menu */} | ||
| {isMenuOpen && ( | ||
| <div className="absolute right-0 top-full mt-1 w-40 bg-white dark:bg-dashboard-card rounded-lg shadow-xl border border-slate-200 dark:border-border-dark overflow-hidden z-50"> | ||
| <div className="absolute right-0 top-full mt-1.5 w-36 bg-[#0c0d16] rounded-xl shadow-[0_10px_30px_rgba(0,0,0,0.5)] border border-white/[0.06] overflow-hidden z-50 p-1 space-y-0.5 animate-in fade-in slide-in-from-top-1 duration-150"> | ||
| <button | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| // Could add rename functionality here | ||
| setIsMenuOpen(false); | ||
| }} | ||
| className="w-full px-3 py-2 flex items-center gap-2 text-left text-sm text-slate-700 dark:text-slate-300 hover:bg-slate-100 dark:hover:bg-surface-highlight-dark transition-colors" | ||
| className="w-full px-2.5 py-1.5 flex items-center gap-2 rounded-lg text-left text-xs font-mono uppercase tracking-wider text-white/50 hover:bg-white/[0.02] hover:text-white/80 transition-colors" | ||
| > | ||
| <span className="material-symbols-outlined text-[18px]">edit</span> | ||
| <span className="material-symbols-outlined text-[15px]">edit</span> | ||
| <span>Rename</span> | ||
| </button> | ||
| <button | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| // Could add duplicate functionality here | ||
| setIsMenuOpen(false); | ||
| }} | ||
| className="w-full px-3 py-2 flex items-center gap-2 text-left text-sm text-slate-700 dark:text-slate-300 hover:bg-slate-100 dark:hover:bg-surface-highlight-dark transition-colors" | ||
| className="w-full px-2.5 py-1.5 flex items-center gap-2 rounded-lg text-left text-xs font-mono uppercase tracking-wider text-white/50 hover:bg-white/[0.02] hover:text-white/80 transition-colors" | ||
| > | ||
| <span className="material-symbols-outlined text-[18px]">content_copy</span> | ||
| <span className="material-symbols-outlined text-[15px]">content_copy</span> | ||
| <span>Duplicate</span> | ||
| </button> | ||
| <div className="border-t border-slate-200 dark:border-border-dark" /> | ||
| <div className="border-t border-white/[0.04] my-1" /> | ||
| <button | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| setIsMenuOpen(false); | ||
| setIsConfirmOpen(true); | ||
| }} | ||
| className="w-full px-3 py-2 flex items-center gap-2 text-left text-sm text-red-600 dark:text-red-400 hover:bg-red-50 dark:hover:bg-red-500/10 transition-colors" | ||
| className="w-full px-2.5 py-1.5 flex items-center gap-2 rounded-lg text-left text-xs font-mono uppercase tracking-wider text-red-400 hover:bg-red-500/[0.06] transition-colors" | ||
| > | ||
| <span className="material-symbols-outlined text-[18px]">delete</span> | ||
| <span className="material-symbols-outlined text-[15px]">delete</span> | ||
| <span>Delete</span> | ||
| </button> | ||
| </div> | ||
| )} | ||
| </div> | ||
| {nodeCount !== undefined && nodeCount > 0 && ( | ||
| <div className="absolute bottom-3 left-3 bg-black/60 backdrop-blur-sm text-white text-xs px-2 py-1 rounded-md"> | ||
| {nodeCount} node{nodeCount !== 1 ? 's' : ''} | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Health status led dot */} | ||
| <div className="absolute bottom-2.5 left-3 flex items-center gap-1.5 px-2 py-0.5 rounded-full bg-black/45 border border-white/[0.03] text-[7px] font-mono tracking-widest uppercase"> | ||
| <span className={`inline-block h-1.5 w-1.5 rounded-full ${mockHealth.dot} opacity-70 animate-pulse`} /> | ||
| <span className="text-white/30">{mockHealth.label}</span> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Description / metadata body */} | ||
| <div className="p-4 flex flex-col flex-1"> | ||
| <div className="flex justify-between items-start mb-2"> | ||
| <h4 className="text-base font-bold text-slate-900 dark:text-white line-clamp-1 group-hover:text-primary transition-colors">{title}</h4> | ||
| </div> | ||
| <div className="flex items-center gap-2 mb-4"> | ||
| <span className={`px-2 py-0.5 rounded text-[11px] font-mono font-medium ${statusColor}`}>{status}</span> | ||
| <h4 className="text-xs font-bold tracking-tight text-white/85 group-hover:text-cyan-400 transition-colors font-display line-clamp-1"> | ||
| {title} | ||
| </h4> | ||
|
|
||
| {/* Simple metadata */} | ||
| <div className="mt-2 text-[9px] font-mono tracking-wider uppercase text-white/40 flex items-center gap-1.5"> | ||
| <span>{nodeCount} Nodes</span> | ||
| <span className="text-white/20">·</span> | ||
| <span>{connectionCount} Links</span> | ||
| </div> | ||
| <div className="mt-auto flex items-center justify-between text-xs text-slate-400 dark:text-text-card-footer-dark"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className="material-symbols-outlined text-[14px]">schedule</span> | ||
|
|
||
| {/* Footer status pill & schedule edited timestamp */} | ||
| <div className="mt-auto pt-4 flex items-center justify-between"> | ||
| <span className={`px-2 py-0.5 rounded text-[8px] font-mono tracking-widest uppercase border ${statusColor}`}> | ||
| {status} | ||
| </span> | ||
| <div className="flex items-center gap-1.5 text-[8px] font-mono uppercase tracking-wider text-white/20"> | ||
| <span className="material-symbols-outlined text-[12px]">schedule</span> | ||
| <span>Edited {editedTime}</span> | ||
| </div> | ||
| {reviewers && ( | ||
| <div className="flex -space-x-1.5"> | ||
| <div className="size-5 rounded-full ring-2 ring-white dark:ring-dashboard-card bg-orange-400" title="Reviewer 1"></div> | ||
| <div className="size-5 rounded-full ring-2 ring-white dark:ring-dashboard-card bg-blue-400" title="Reviewer 2"></div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| </div> | ||
| </Link> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Split the clickable card from the menu actions.
The outer Link wraps the menu trigger and dropdown buttons, which nests interactive controls inside a link. That markup is invalid and tends to break keyboard/focus behavior for the card actions. Keep the preview/body linked, but move the menu controls outside that linked region.
🤖 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 `@components/dashboard/DesignCard.tsx` around lines 146 - 337, The DesignCard
component currently nests the menu trigger and dropdown actions inside the outer
Link, which mixes interactive controls with navigation. Update DesignCard so the
clickable card surface remains inside the Link, but the menu button and dropdown
are rendered outside that linked region; use the existing menuRef, isMenuOpen,
setIsMenuOpen, and setIsConfirmOpen logic to preserve behavior while separating
navigation from card actions.
Fix Playwright tests and resolve lint issues
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
Bug Fixes