Feature/website card implementation#240
Merged
Merged
Conversation
- Add website card type to CardType union and type definitions - Create WebsiteData interface with url and favicon fields - Implement CreateWebsiteDialog for URL input with favicon fetching - Add WebsitePanelContent component for iframe embedding - Update WorkspaceCard to display website cards with: - Card color, title header, and type badge (like notes) - Favicon in type badge with Globe icon fallback - Default globe detection (16x16 naturalHeight check) - Make website cards non-resizable (isResizable: false) - Set default size to 1x4 (single-column minimum) - Add 'Open' button in WorkspaceHeader and ItemPanelContent - Wire website card creation in WorkspaceHeader and WorkspaceSection - Add Globe icon to SidebarCardList for website cards - Update CardRenderer with website card rendering - Use Google Favicon API with sz=64 parameter for detection
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new "website" item type: UI for creating single-URL website cards, iframe-based website viewer, favicon/name derivation utilities, layout and type-system updates, workspace creation wiring via an onCreate callback, and rendering/UI adjustments to surface website cards across the canvas and sidebar. Changes
Sequence DiagramsequenceDiagram
participant User
participant Dialog as CreateWebsiteDialog
participant Section as WorkspaceSection<br/>(handleWebsiteCreate)
participant Ops as Workspace Operations<br/>(createItems)
participant Canvas as WorkspaceCard/CardRenderer
participant Panel as WebsitePanelContent
User->>Dialog: open dialog & enter URL
Dialog->>Dialog: validate URL, derive name & favicon
User->>Dialog: submit
Dialog->>Section: onCreate(url, name, favicon)
Section->>Ops: createItems("website", { url, favicon }, layout)
Ops->>Canvas: new item emitted / state updated
User->>Canvas: click website card
Canvas->>Panel: route to WebsitePanelContent (item.type == "website")
Panel->>Panel: render sandboxed iframe with item.data.url
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 unit tests (beta)
📝 Coding Plan
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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| const websiteData = item.data as WebsiteData; | ||
| const favicon = websiteData.favicon; | ||
| let hostname = ''; | ||
| try { hostname = new URL(websiteData.url).hostname.replace(/^www\./, ''); } catch {} |
| const [url, setUrl] = useState(""); | ||
| const [name, setName] = useState(""); | ||
| const [isValid, setIsValid] = useState(false); | ||
| const [isLoadingTitle, setIsLoadingTitle] = useState(false); |
| import { QuizContent } from "./QuizContent"; | ||
| import { ImageCardContent } from "./ImageCardContent"; | ||
| import { MoreVertical, Trash2, Palette, CheckCircle2, FolderInput, Copy, X, Pencil, Columns, Link2, PanelRight, SplitSquareHorizontal, Loader2, File, Brain, Mic, AlertCircle } from "lucide-react"; | ||
| import { MoreVertical, Trash2, Palette, CheckCircle2, FolderInput, Copy, X, Pencil, Columns, Link2, PanelRight, SplitSquareHorizontal, Loader2, File, Brain, Mic, AlertCircle, Globe } from "lucide-react"; |
| import Link from "next/link"; | ||
| import { usePathname, useRouter } from "next/navigation"; | ||
| import { Search, X, ChevronDown, ChevronRight, FolderOpen, Plus, Upload, Folder as FolderIcon, Settings, Share2, Play, Brain, File, ImageIcon, Mic, PanelRight, Loader2 } from "lucide-react"; | ||
| import { Search, X, ChevronDown, ChevronRight, FolderOpen, Plus, Upload, Folder as FolderIcon, Settings, Share2, Play, Brain, File, ImageIcon, Mic, PanelRight, Loader2, ExternalLink } from "lucide-react"; |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
8 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/workspace-canvas/WorkspaceCard.tsx">
<violation number="1" location="src/components/workspace-canvas/WorkspaceCard.tsx:801">
P2: Don't treat every 16×16 favicon as a placeholder; many valid site favicons use that size and will be hidden here.</violation>
<violation number="2" location="src/components/workspace-canvas/WorkspaceCard.tsx:817">
P2: Track favicon fallback with React state instead of DOM style mutations; this render-time `display` prop hides the fallback again on the next re-render.</violation>
</file>
<file name="src/components/modals/CreateWebsiteDialog.tsx">
<violation number="1" location="src/components/modals/CreateWebsiteDialog.tsx:93">
P2: Include `name` in the auto-fill effect dependencies so a pending timeout cannot overwrite a manually entered card name.</violation>
</file>
<file name="src/components/workspace-canvas/WebsitePanelContent.tsx">
<violation number="1" location="src/components/workspace-canvas/WebsitePanelContent.tsx:32">
P1: Remove `allow-same-origin` from this sandbox. Combined with `allow-scripts`, it lets same-origin embeds bypass the isolation this panel is supposed to provide.</violation>
<violation number="2" location="src/components/workspace-canvas/WebsitePanelContent.tsx:33">
P2: Don't grant `clipboard-write` to arbitrary embeds unless you explicitly trust the origin.</violation>
</file>
<file name="src/hooks/workspace/use-workspace-operations.ts">
<violation number="1" location="src/hooks/workspace/use-workspace-operations.ts:97">
P2: This whitelist is still missing `quiz`, so `createItem`/`createItems` will silently create a note when called with a valid `quiz` card type.</violation>
</file>
<file name="src/components/workspace-canvas/WorkspaceHeader.tsx">
<violation number="1" location="src/components/workspace-canvas/WorkspaceHeader.tsx:764">
P1: Validate the stored website URL before passing it to `window.open`; otherwise non-HTTP(S) schemes can be executed from a crafted website card.</violation>
</file>
<file name="src/lib/workspace-state/types.ts">
<violation number="1" location="src/lib/workspace-state/types.ts:3">
P2: Adding `website` to `CardType` without updating import validation makes exported website cards fail re-import.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| src={websiteData.url} | ||
| title={item.name || "Website"} | ||
| className="w-full flex-1 min-h-0 border-0" | ||
| sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-popups-to-escape-sandbox" |
Contributor
There was a problem hiding this comment.
P1: Remove allow-same-origin from this sandbox. Combined with allow-scripts, it lets same-origin embeds bypass the isolation this panel is supposed to provide.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/workspace-canvas/WebsitePanelContent.tsx, line 32:
<comment>Remove `allow-same-origin` from this sandbox. Combined with `allow-scripts`, it lets same-origin embeds bypass the isolation this panel is supposed to provide.</comment>
<file context>
@@ -0,0 +1,38 @@
+ src={websiteData.url}
+ title={item.name || "Website"}
+ className="w-full flex-1 min-h-0 border-0"
+ sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-popups-to-escape-sandbox"
+ allow="clipboard-write"
+ referrerPolicy="no-referrer"
</file context>
| <button | ||
| className="h-8 flex items-center justify-center gap-1.5 rounded-md border border-sidebar-border text-muted-foreground hover:text-sidebar-foreground hover:bg-accent transition-colors cursor-pointer px-2" | ||
| aria-label="Open link in new tab" | ||
| onClick={() => window.open(websiteData.url, '_blank', 'noopener,noreferrer')} |
Contributor
There was a problem hiding this comment.
P1: Validate the stored website URL before passing it to window.open; otherwise non-HTTP(S) schemes can be executed from a crafted website card.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/workspace-canvas/WorkspaceHeader.tsx, line 764:
<comment>Validate the stored website URL before passing it to `window.open`; otherwise non-HTTP(S) schemes can be executed from a crafted website card.</comment>
<file context>
@@ -748,6 +754,21 @@ export function WorkspaceHeader({
+ <button
+ className="h-8 flex items-center justify-center gap-1.5 rounded-md border border-sidebar-border text-muted-foreground hover:text-sidebar-foreground hover:bg-accent transition-colors cursor-pointer px-2"
+ aria-label="Open link in new tab"
+ onClick={() => window.open(websiteData.url, '_blank', 'noopener,noreferrer')}
+ >
+ <ExternalLink className="h-4 w-4" />
</file context>
Suggested change
| onClick={() => window.open(websiteData.url, '_blank', 'noopener,noreferrer')} | |
| onClick={() => { | |
| try { | |
| const parsedUrl = new URL(websiteData.url); | |
| if (parsedUrl.protocol === "http:" || parsedUrl.protocol === "https:") { | |
| window.open(parsedUrl.toString(), "_blank", "noopener,noreferrer"); | |
| } else { | |
| toast.error("Invalid website URL"); | |
| } | |
| } catch { | |
| toast.error("Invalid website URL"); | |
| } | |
| }} |
| className="h-5 w-5 shrink-0 rounded" | ||
| onLoad={(e) => { | ||
| // Hide default globe icons (they stay 16x16 even with sz=64) | ||
| if (e.currentTarget.naturalHeight === 16) { |
Contributor
There was a problem hiding this comment.
P2: Don't treat every 16×16 favicon as a placeholder; many valid site favicons use that size and will be hidden here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/workspace-canvas/WorkspaceCard.tsx, line 801:
<comment>Don't treat every 16×16 favicon as a placeholder; many valid site favicons use that size and will be hidden here.</comment>
<file context>
@@ -780,6 +780,48 @@ function WorkspaceCard({
+ className="h-5 w-5 shrink-0 rounded"
+ onLoad={(e) => {
+ // Hide default globe icons (they stay 16x16 even with sz=64)
+ if (e.currentTarget.naturalHeight === 16) {
+ e.currentTarget.style.display = 'none';
+ const fallback = document.getElementById(fallbackId);
</file context>
| <div | ||
| id={fallbackId} | ||
| className="h-5 w-5 shrink-0 flex items-center justify-center" | ||
| style={{ display: favicon ? 'none' : 'flex' }} |
Contributor
There was a problem hiding this comment.
P2: Track favicon fallback with React state instead of DOM style mutations; this render-time display prop hides the fallback again on the next re-render.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/workspace-canvas/WorkspaceCard.tsx, line 817:
<comment>Track favicon fallback with React state instead of DOM style mutations; this render-time `display` prop hides the fallback again on the next re-render.</comment>
<file context>
@@ -780,6 +780,48 @@ function WorkspaceCard({
+ <div
+ id={fallbackId}
+ className="h-5 w-5 shrink-0 flex items-center justify-center"
+ style={{ display: favicon ? 'none' : 'flex' }}
+ >
+ <Globe className="h-5 w-5 shrink-0" />
</file context>
| clearTimeout(debounceTimerRef.current); | ||
| } | ||
| }; | ||
| }, [url, isValid]); |
Contributor
There was a problem hiding this comment.
P2: Include name in the auto-fill effect dependencies so a pending timeout cannot overwrite a manually entered card name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/modals/CreateWebsiteDialog.tsx, line 93:
<comment>Include `name` in the auto-fill effect dependencies so a pending timeout cannot overwrite a manually entered card name.</comment>
<file context>
@@ -35,225 +32,156 @@ function isValidUrl(str: string): boolean {
+ clearTimeout(debounceTimerRef.current);
}
+ };
+ }, [url, isValid]);
- const result = await response.json();
</file context>
Suggested change
| }, [url, isValid]); | |
| }, [url, isValid, name]); |
| title={item.name || "Website"} | ||
| className="w-full flex-1 min-h-0 border-0" | ||
| sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-popups-to-escape-sandbox" | ||
| allow="clipboard-write" |
Contributor
There was a problem hiding this comment.
P2: Don't grant clipboard-write to arbitrary embeds unless you explicitly trust the origin.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/workspace-canvas/WebsitePanelContent.tsx, line 33:
<comment>Don't grant `clipboard-write` to arbitrary embeds unless you explicitly trust the origin.</comment>
<file context>
@@ -0,0 +1,38 @@
+ title={item.name || "Website"}
+ className="w-full flex-1 min-h-0 border-0"
+ sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-popups-to-escape-sandbox"
+ allow="clipboard-write"
+ referrerPolicy="no-referrer"
+ />
</file context>
| import type { CardColor } from './colors'; | ||
|
|
||
| export type CardType = "note" | "pdf" | "flashcard" | "folder" | "youtube" | "quiz" | "image" | "audio"; | ||
| export type CardType = "note" | "pdf" | "flashcard" | "folder" | "youtube" | "quiz" | "image" | "audio" | "website"; |
Contributor
There was a problem hiding this comment.
P2: Adding website to CardType without updating import validation makes exported website cards fail re-import.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/workspace-state/types.ts, line 3:
<comment>Adding `website` to `CardType` without updating import validation makes exported website cards fail re-import.</comment>
<file context>
@@ -1,6 +1,6 @@
import type { CardColor } from './colors';
-export type CardType = "note" | "pdf" | "flashcard" | "folder" | "youtube" | "quiz" | "image" | "audio";
+export type CardType = "note" | "pdf" | "flashcard" | "folder" | "youtube" | "quiz" | "image" | "audio" | "website";
/**
</file context>
![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 (2)
src/components/workspace-canvas/WorkspaceCard.tsx (2)
760-824:⚠️ Potential issue | 🟠 MajorMemoization still ignores website data.
This new UI reads
item.data.favicon, butWorkspaceCardMemoizeddoes not compareitem.datafor"website". URL or favicon updates can therefore be treated as equal and never repaint.🔧 Suggested fix
if (prevProps.item.type === 'audio' && nextProps.item.type === 'audio') { const prevData = prevProps.item.data; const nextData = nextProps.item.data; if (JSON.stringify(prevData) !== JSON.stringify(nextData)) return false; } + if (prevProps.item.type === 'website' && nextProps.item.type === 'website') { + const prevData = prevProps.item.data; + const nextData = nextProps.item.data; + if (JSON.stringify(prevData) !== JSON.stringify(nextData)) return false; + } // Compare layout (use lg breakpoint for comparison)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace-canvas/WorkspaceCard.tsx` around lines 760 - 824, The memoized comparator for WorkspaceCardMemoized is missing checks for website-specific data, so changes to item.data (favicon or url) won't trigger re-renders; update the equality function used by WorkspaceCardMemoized (the custom props/areEqual comparator) to also compare website fields — e.g., when item.type === 'website' include a strict comparison of (item.data as WebsiteData).favicon and (item.data as WebsiteData).url (or deep-compare item.data) in addition to existing item/id/color checks so favicon/url updates force a repaint.
760-824:⚠️ Potential issue | 🟠 MajorPreview-sized website cards still render as empty shells.
This badge path only runs when
!shouldShowPreview, and the file never rendersitem.type === "website"content elsewhere. Once a website card is wide/tall enough, the badge disappears and the body stays blank.🔧 Suggested follow-up
{/* Website Content */} {item.type === "website" && shouldShowPreview && (() => { const websiteData = item.data as WebsiteData; let hostname = ""; try { hostname = new URL(websiteData.url).hostname.replace(/^www\./, ""); } catch {} return ( <div className="flex-1 min-h-0 flex flex-col items-center justify-center gap-3 p-6"> {websiteData.favicon ? ( <img src={websiteData.favicon} alt="" className="size-12 rounded-lg" /> ) : ( <Globe className="size-12 text-muted-foreground" /> )} <span className="text-sm text-muted-foreground"> {hostname || websiteData.url || "Website"} </span> </div> ); })()}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace-canvas/WorkspaceCard.tsx` around lines 760 - 824, The website card currently only renders a small badge when !shouldShowPreview, leaving the preview-sized card body blank; update WorkspaceCard's JSX so that when item.type === 'website' && shouldShowPreview you render the preview content (use the same WebsiteData extraction as the badge: const websiteData = item.data as WebsiteData, compute hostname with new URL(...).hostname.replace(/^www\./, ''), and show the favicon if present otherwise a Globe icon) — place this block in the preview rendering branch of WorkspaceCard (the same area handling other preview content) and reuse the existing favicon/fallback logic (faviconId / fallbackId) to show the image or fallback globe and a hostname/url label.
🧹 Nitpick comments (4)
src/components/modals/CreateWebsiteDialog.tsx (1)
68-68: Remove unused state variable.
isLoadingTitleis declared but never set totrueduring the component lifecycle (only reset tofalseon dialog open). This appears to be leftover from a removed feature.♻️ Remove unused state
const [url, setUrl] = useState(""); const [name, setName] = useState(""); const [isValid, setIsValid] = useState(false); -const [isLoadingTitle, setIsLoadingTitle] = useState(false); const debounceTimerRef = useRef<NodeJS.Timeout | null>(null); // ... // Reset form when dialog opens useEffect(() => { if (open) { setUrl(""); setName(""); setIsValid(false); - setIsLoadingTitle(false); if (debounceTimerRef.current) { clearTimeout(debounceTimerRef.current); } } }, [open]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/modals/CreateWebsiteDialog.tsx` at line 68, Remove the unused state isLoadingTitle (and its setter setIsLoadingTitle) from the CreateWebsiteDialog component: delete the useState declaration for isLoadingTitle and any lines that reference or reset it (e.g., the dialog open/reset logic) so there are no orphaned references; ensure no further logic expects isLoadingTitle to exist and run tests/build to confirm no errors.src/components/workspace-canvas/ItemPanelContent.tsx (1)
82-95: Consider extracting websiteData outside the JSX for cleaner code.The inline IIFE pattern with type import works but is verbose. Consider extracting
websiteDataat the top of the function alongsidepdfData, which would also allow for null-safety checks ifitem.datais undefined.♻️ Suggested refactor
const isPdf = item.type === 'pdf'; const isYouTube = item.type === 'youtube'; const isWebsite = item.type === 'website'; const pdfData = item.data as PdfData; +const websiteData = isWebsite ? (item.data as import("@/lib/workspace-state/types").WebsiteData) : null; // ...then in JSX: -{isWebsite && (() => { - const websiteData = item.data as import("@/lib/workspace-state/types").WebsiteData; - return ( - <button - type="button" - aria-label="Open" - className="inline-flex h-8 w-8 items-center justify-center rounded-md border border-sidebar-border text-muted-foreground hover:text-sidebar-foreground hover:bg-sidebar-accent transition-colors cursor-pointer" - onClick={() => window.open(websiteData.url, '_blank', 'noopener,noreferrer')} - > - <ExternalLink className="h-4 w-4" /> - </button> - ); -})()} +{websiteData?.url && ( + <button + type="button" + aria-label="Open" + className="inline-flex h-8 w-8 items-center justify-center rounded-md border border-sidebar-border text-muted-foreground hover:text-sidebar-foreground hover:bg-sidebar-accent transition-colors cursor-pointer" + onClick={() => window.open(websiteData.url, '_blank', 'noopener,noreferrer')} + > + <ExternalLink className="h-4 w-4" /> + </button> +)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace-canvas/ItemPanelContent.tsx` around lines 82 - 95, The inline IIFE that creates websiteData inside the JSX is verbose and mixes logic with markup; extract const websiteData = item.data as WebsiteData (import type from "@/lib/workspace-state/types" or reuse existing type) near the top of ItemPanelContent (alongside pdfData) and replace the IIFE with a simple conditional render using isWebsite && websiteData && (<button ... onClick={() => window.open(websiteData.url, '_blank', 'noopener,noreferrer')}>...</button>), ensuring you null-check item.data before accessing .url to avoid runtime errors.src/components/workspace-canvas/WorkspaceHeader.tsx (1)
380-384: Code duplication with WorkspaceSection.tsx.This
handleWebsiteCreateimplementation differs slightly from the one inWorkspaceSection.tsx(Line 257-265). WorkspaceSection usesoperations.createItems()while this usesaddItem(). While both work, consider extracting this to a shared hook or ensuring consistency in the approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace-canvas/WorkspaceHeader.tsx` around lines 380 - 384, The two implementations of handleWebsiteCreate diverge (one in WorkspaceHeader using addItem, one in WorkspaceSection using operations.createItems); extract the shared logic into a single hook (e.g., useCreateWebsite) that exposes a createWebsite(name, url, favicon) function and internally delegates to the canonical creation method (choose operations.createItems as the single source of truth or wrap addItem if operations is unavailable), then replace the local handleWebsiteCreate in WorkspaceHeader and the handler in WorkspaceSection to call this hook; reference the existing symbols handleWebsiteCreate, addItem, and operations.createItems when implementing the hook so the components use a consistent creation path.src/components/workspace-canvas/WorkspaceSection.tsx (1)
807-811: Consider adding user feedback if operations is unavailable.If
operationsis undefined whenhandleWebsiteCreateis called, the function silently returns without feedback. While this is unlikely in practice, adding a toast error would improve UX.♻️ Suggested improvement
const handleWebsiteCreate = useCallback((url: string, name: string, favicon?: string) => { - if (!operations) return; + if (!operations) { + toast.error("Unable to create website card"); + return; + } operations.createItems([{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace-canvas/WorkspaceSection.tsx` around lines 807 - 811, handle the silent return in handleWebsiteCreate by adding user feedback when operations is undefined: inside the handleWebsiteCreate function (referenced by the CreateWebsiteDialog onCreate prop) check if operations is falsy and, instead of silently returning, call the app's toast/error notification utility (e.g., toast.error or showToast) with a clear message like "Service unavailable – please try again later" and keep or update showWebsiteDialog via setShowWebsiteDialog as appropriate so the user sees the error; ensure the new notification path is implemented wherever handleWebsiteCreate is defined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/workspace-canvas/CardRenderer.tsx`:
- Around line 115-123: The fallback avatar can render an empty glyph if new
URL(websiteData.url) throws and hostname remains '', so update the try/catch in
CardRenderer (where hostname is derived from new URL(websiteData.url)) to ensure
hostname always has a safe fallback (for example set hostname =
(websiteData.title || websiteData.url || '?').toString() and then extract the
first char via hostname.charAt(0).toUpperCase()); ensure the catch block assigns
that fallback rather than leaving hostname empty so the avatar span always
displays a visible glyph.
In `@src/components/workspace-canvas/WebsitePanelContent.tsx`:
- Around line 16-35: The component currently passes any non-empty
websiteData.url into the iframe src; add a validation step in
WebsitePanelContent (before rendering the iframe) that rejects unsupported URLs
by attempting to construct a new URL(websiteData.url, window.location.href) and
then ensuring url.protocol is "http:" or "https:" and url.origin !==
window.location.origin (or otherwise disallow same-origin app URLs), and if
validation fails return the existing “No URL provided” / unsupported message
instead of rendering the iframe; update the iframe rendering to only set src
when the URL passes this validation (check symbols: websiteData.url, the iframe
src prop, and the component that returns the iframe).
In `@src/components/workspace-canvas/WorkspaceHeader.tsx`:
- Around line 757-770: The Open button can call window.open with an empty URL
because WebsiteData.url may be an empty string; update the render guard in
WorkspaceHeader so the button only renders (or the onClick only runs) when
activeItems[0]?.type === "website" AND the extracted websiteData.url is a
non-empty, trimmed string (e.g., websiteData.url?.trim() !== ''). Use the
WebsiteData object from activeItems to check url before calling window.open (or
disable the button) to defensively prevent opening a blank tab.
In `@src/hooks/workspace/use-workspace-operations.ts`:
- Around line 97-98: The runtime allowlist in use-workspace-operations.ts
currently omits "quiz", causing quiz cards to be coerced to "note"; update the
allowlist used in the createItem/createItems logic (the validTypes array and its
usage where validType is computed) to include "quiz", and refactor by extracting
that array into a shared constant (e.g., CARD_TYPES or VALID_CARD_TYPES)
exported/used by createItem and createItems so both call sites use the same
source of truth to avoid future drift.
In `@src/lib/workspace-state/item-helpers.ts`:
- Line 1: The helper defaultDataFor is missing an explicit branch for "quiz", so
update the imports to include the QuizData type (add QuizData to the import
list) and add a case "quiz" in the defaultDataFor switch that returns a value
matching the QuizData interface (e.g., default questions array and default
settings) instead of falling through to NoteData; ensure the returned object
shape exactly matches QuizData and adjust any other similar switch sites (the
other defaultDataFor-like branches around the same helper) to handle "quiz" the
same way.
In `@src/lib/workspace-state/types.ts`:
- Line 3: The CardType union now includes "website" but
src/lib/ai/tools/workspace-search-utils.ts still falls through to the default
branch when card.type === "website", so update the switch/dispatch that builds
indexable text for cards to handle "website" explicitly: extract and concatenate
the website's title, URL, description/summary, and any cached page text (the
same logic needs to be added in both places the switch is used — the block
around lines ~32-100 and the separate block around ~150-155), replacing the
default-empty behavior with a composed indexable string so website cards
contribute searchable content; ensure you update any helper name used there (the
switch on card.type / the function that returns indexable text) and add a small
unit test to confirm website cards are indexed.
---
Outside diff comments:
In `@src/components/workspace-canvas/WorkspaceCard.tsx`:
- Around line 760-824: The memoized comparator for WorkspaceCardMemoized is
missing checks for website-specific data, so changes to item.data (favicon or
url) won't trigger re-renders; update the equality function used by
WorkspaceCardMemoized (the custom props/areEqual comparator) to also compare
website fields — e.g., when item.type === 'website' include a strict comparison
of (item.data as WebsiteData).favicon and (item.data as WebsiteData).url (or
deep-compare item.data) in addition to existing item/id/color checks so
favicon/url updates force a repaint.
- Around line 760-824: The website card currently only renders a small badge
when !shouldShowPreview, leaving the preview-sized card body blank; update
WorkspaceCard's JSX so that when item.type === 'website' && shouldShowPreview
you render the preview content (use the same WebsiteData extraction as the
badge: const websiteData = item.data as WebsiteData, compute hostname with new
URL(...).hostname.replace(/^www\./, ''), and show the favicon if present
otherwise a Globe icon) — place this block in the preview rendering branch of
WorkspaceCard (the same area handling other preview content) and reuse the
existing favicon/fallback logic (faviconId / fallbackId) to show the image or
fallback globe and a hostname/url label.
---
Nitpick comments:
In `@src/components/modals/CreateWebsiteDialog.tsx`:
- Line 68: Remove the unused state isLoadingTitle (and its setter
setIsLoadingTitle) from the CreateWebsiteDialog component: delete the useState
declaration for isLoadingTitle and any lines that reference or reset it (e.g.,
the dialog open/reset logic) so there are no orphaned references; ensure no
further logic expects isLoadingTitle to exist and run tests/build to confirm no
errors.
In `@src/components/workspace-canvas/ItemPanelContent.tsx`:
- Around line 82-95: The inline IIFE that creates websiteData inside the JSX is
verbose and mixes logic with markup; extract const websiteData = item.data as
WebsiteData (import type from "@/lib/workspace-state/types" or reuse existing
type) near the top of ItemPanelContent (alongside pdfData) and replace the IIFE
with a simple conditional render using isWebsite && websiteData && (<button ...
onClick={() => window.open(websiteData.url, '_blank',
'noopener,noreferrer')}>...</button>), ensuring you null-check item.data before
accessing .url to avoid runtime errors.
In `@src/components/workspace-canvas/WorkspaceHeader.tsx`:
- Around line 380-384: The two implementations of handleWebsiteCreate diverge
(one in WorkspaceHeader using addItem, one in WorkspaceSection using
operations.createItems); extract the shared logic into a single hook (e.g.,
useCreateWebsite) that exposes a createWebsite(name, url, favicon) function and
internally delegates to the canonical creation method (choose
operations.createItems as the single source of truth or wrap addItem if
operations is unavailable), then replace the local handleWebsiteCreate in
WorkspaceHeader and the handler in WorkspaceSection to call this hook; reference
the existing symbols handleWebsiteCreate, addItem, and operations.createItems
when implementing the hook so the components use a consistent creation path.
In `@src/components/workspace-canvas/WorkspaceSection.tsx`:
- Around line 807-811: handle the silent return in handleWebsiteCreate by adding
user feedback when operations is undefined: inside the handleWebsiteCreate
function (referenced by the CreateWebsiteDialog onCreate prop) check if
operations is falsy and, instead of silently returning, call the app's
toast/error notification utility (e.g., toast.error or showToast) with a clear
message like "Service unavailable – please try again later" and keep or update
showWebsiteDialog via setShowWebsiteDialog as appropriate so the user sees the
error; ensure the new notification path is implemented wherever
handleWebsiteCreate is defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff87f94f-00fe-47ec-97e6-ea0cde75521c
📒 Files selected for processing (13)
src/components/modals/CreateWebsiteDialog.tsxsrc/components/workspace-canvas/CardRenderer.tsxsrc/components/workspace-canvas/ItemPanelContent.tsxsrc/components/workspace-canvas/WebsitePanelContent.tsxsrc/components/workspace-canvas/WorkspaceCard.tsxsrc/components/workspace-canvas/WorkspaceHeader.tsxsrc/components/workspace-canvas/WorkspaceSection.tsxsrc/components/workspace/SidebarCardList.tsxsrc/hooks/workspace/use-workspace-operations.tssrc/lib/utils/format-workspace-context.tssrc/lib/workspace-state/grid-layout-helpers.tssrc/lib/workspace-state/item-helpers.tssrc/lib/workspace-state/types.ts
Comment on lines
+115
to
+123
| let hostname = ''; | ||
| try { hostname = new URL(websiteData.url).hostname.replace(/^www\./, ''); } catch {} | ||
| return ( | ||
| <div className="flex-1 min-h-0 flex flex-col items-center justify-center gap-3 p-8"> | ||
| {websiteData.favicon ? ( | ||
| <img src={websiteData.favicon} alt="" className="size-16 rounded-lg" /> | ||
| ) : ( | ||
| <div className="size-16 rounded-lg bg-muted flex items-center justify-center"> | ||
| <span className="text-2xl font-bold text-muted-foreground">{hostname.charAt(0).toUpperCase()}</span> |
There was a problem hiding this comment.
Avoid rendering an empty website placeholder.
If new URL() throws here, hostname stays empty and the fallback avatar renders no glyph. In modal/generic contexts that looks broken instead of degraded.
🔧 Suggested fix
- <span className="text-2xl font-bold text-muted-foreground">{hostname.charAt(0).toUpperCase()}</span>
+ <span className="text-2xl font-bold text-muted-foreground">
+ {(hostname || item.name || "W").charAt(0).toUpperCase()}
+ </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/workspace-canvas/CardRenderer.tsx` around lines 115 - 123, The
fallback avatar can render an empty glyph if new URL(websiteData.url) throws and
hostname remains '', so update the try/catch in CardRenderer (where hostname is
derived from new URL(websiteData.url)) to ensure hostname always has a safe
fallback (for example set hostname = (websiteData.title || websiteData.url ||
'?').toString() and then extract the first char via
hostname.charAt(0).toUpperCase()); ensure the catch block assigns that fallback
rather than leaving hostname empty so the avatar span always displays a visible
glyph.
Comment on lines
+16
to
+35
| if (!websiteData.url) { | ||
| return ( | ||
| <div className="flex-1 min-h-0 flex flex-col items-center justify-center p-6"> | ||
| <div className="rounded-lg p-4 bg-muted/50 border border-border flex items-center justify-center"> | ||
| <span className="text-muted-foreground font-medium">No URL provided</span> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <div className="w-full flex-1 min-h-0 flex flex-col overflow-hidden bg-white"> | ||
| <iframe | ||
| src={websiteData.url} | ||
| title={item.name || "Website"} | ||
| className="w-full flex-1 min-h-0 border-0" | ||
| sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-popups-to-escape-sandbox" | ||
| allow="clipboard-write" | ||
| referrerPolicy="no-referrer" | ||
| /> |
There was a problem hiding this comment.
Reject unsupported URLs before creating the iframe.
Right now any non-empty string becomes src: absolute app URLs, relative paths, and non-HTTP(S) values all get framed. For a shared “website card” feature, that is too permissive to leave to upstream normalization.
🔒 Suggested guard
export function WebsitePanelContent({
item,
isMaximized = false,
}: WebsitePanelContentProps) {
const websiteData = item.data as WebsiteData;
+ const appOrigin = typeof window !== "undefined" ? window.location.origin : null;
+ let parsedUrl: URL | null = null;
+
+ try {
+ parsedUrl = new URL(websiteData.url);
+ } catch {
+ parsedUrl = null;
+ }
- if (!websiteData.url) {
+ if (
+ !parsedUrl ||
+ !["http:", "https:"].includes(parsedUrl.protocol) ||
+ (appOrigin !== null && parsedUrl.origin === appOrigin)
+ ) {
return (
<div className="flex-1 min-h-0 flex flex-col items-center justify-center p-6">
<div className="rounded-lg p-4 bg-muted/50 border border-border flex items-center justify-center">
- <span className="text-muted-foreground font-medium">No URL provided</span>
+ <span className="text-muted-foreground font-medium">Invalid or unsupported URL</span>
</div>
</div>
);
}
return (
<div className="w-full flex-1 min-h-0 flex flex-col overflow-hidden bg-white">
<iframe
- src={websiteData.url}
+ src={parsedUrl.toString()}
title={item.name || "Website"}
className="w-full flex-1 min-h-0 border-0"
sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-popups-to-escape-sandbox"
allow="clipboard-write"
referrerPolicy="no-referrer"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/workspace-canvas/WebsitePanelContent.tsx` around lines 16 -
35, The component currently passes any non-empty websiteData.url into the iframe
src; add a validation step in WebsitePanelContent (before rendering the iframe)
that rejects unsupported URLs by attempting to construct a new
URL(websiteData.url, window.location.href) and then ensuring url.protocol is
"http:" or "https:" and url.origin !== window.location.origin (or otherwise
disallow same-origin app URLs), and if validation fails return the existing “No
URL provided” / unsupported message instead of rendering the iframe; update the
iframe rendering to only set src when the URL passes this validation (check
symbols: websiteData.url, the iframe src prop, and the component that returns
the iframe).
Comment on lines
+757
to
+770
| {/* Open Button - only for website cards */} | ||
| {activeItems[0]?.type === "website" && (() => { | ||
| const websiteData = activeItems[0].data as import("@/lib/workspace-state/types").WebsiteData; | ||
| return ( | ||
| <button | ||
| className="h-8 flex items-center justify-center gap-1.5 rounded-md border border-sidebar-border text-muted-foreground hover:text-sidebar-foreground hover:bg-accent transition-colors cursor-pointer px-2" | ||
| aria-label="Open link in new tab" | ||
| onClick={() => window.open(websiteData.url, '_blank', 'noopener,noreferrer')} | ||
| > | ||
| <ExternalLink className="h-4 w-4" /> | ||
| <span className="text-xs font-medium">Open</span> | ||
| </button> | ||
| ); | ||
| })()} |
There was a problem hiding this comment.
Guard against empty URL.
The default WebsiteData has an empty string for url (per item-helpers.ts). While the dialog validates URLs before creation, a defensive check would prevent opening a blank tab if data is somehow malformed.
🛡️ Suggested fix
{activeItems[0]?.type === "website" && (() => {
const websiteData = activeItems[0].data as import("@/lib/workspace-state/types").WebsiteData;
+ if (!websiteData?.url) return null;
return (
<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
| {/* Open Button - only for website cards */} | |
| {activeItems[0]?.type === "website" && (() => { | |
| const websiteData = activeItems[0].data as import("@/lib/workspace-state/types").WebsiteData; | |
| return ( | |
| <button | |
| className="h-8 flex items-center justify-center gap-1.5 rounded-md border border-sidebar-border text-muted-foreground hover:text-sidebar-foreground hover:bg-accent transition-colors cursor-pointer px-2" | |
| aria-label="Open link in new tab" | |
| onClick={() => window.open(websiteData.url, '_blank', 'noopener,noreferrer')} | |
| > | |
| <ExternalLink className="h-4 w-4" /> | |
| <span className="text-xs font-medium">Open</span> | |
| </button> | |
| ); | |
| })()} | |
| {/* Open Button - only for website cards */} | |
| {activeItems[0]?.type === "website" && (() => { | |
| const websiteData = activeItems[0].data as import("@/lib/workspace-state/types").WebsiteData; | |
| if (!websiteData?.url) return null; | |
| return ( | |
| <button | |
| className="h-8 flex items-center justify-center gap-1.5 rounded-md border border-sidebar-border text-muted-foreground hover:text-sidebar-foreground hover:bg-accent transition-colors cursor-pointer px-2" | |
| aria-label="Open link in new tab" | |
| onClick={() => window.open(websiteData.url, '_blank', 'noopener,noreferrer')} | |
| > | |
| <ExternalLink className="h-4 w-4" /> | |
| <span className="text-xs font-medium">Open</span> | |
| </button> | |
| ); | |
| })()} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/workspace-canvas/WorkspaceHeader.tsx` around lines 757 - 770,
The Open button can call window.open with an empty URL because WebsiteData.url
may be an empty string; update the render guard in WorkspaceHeader so the button
only renders (or the onClick only runs) when activeItems[0]?.type === "website"
AND the extracted websiteData.url is a non-empty, trimmed string (e.g.,
websiteData.url?.trim() !== ''). Use the WebsiteData object from activeItems to
check url before calling window.open (or disable the button) to defensively
prevent opening a blank tab.
Comment on lines
97
to
98
| const validTypes: CardType[] = ["note", "pdf", "flashcard", "folder", "youtube", "image", "audio", "website"]; | ||
| const validType = validTypes.includes(type) ? type : "note"; |
There was a problem hiding this comment.
Don't drop "quiz" from the runtime allowlist.
Both allowlists still omit "quiz", so quiz cards are silently coerced to "note" in createItem and createItems. That breaks quiz creation while making the failure hard to spot.
🔧 Suggested fix
- const validTypes: CardType[] = ["note", "pdf", "flashcard", "folder", "youtube", "image", "audio", "website"];
+ const validTypes: CardType[] = ["note", "pdf", "flashcard", "folder", "youtube", "quiz", "image", "audio", "website"];
@@
- const validTypes: CardType[] = ["note", "pdf", "flashcard", "folder", "youtube", "image", "audio", "website"];
+ const validTypes: CardType[] = ["note", "pdf", "flashcard", "folder", "youtube", "quiz", "image", "audio", "website"];Pulling this into a shared constant would also prevent the two call sites from drifting again.
Also applies to: 188-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/workspace/use-workspace-operations.ts` around lines 97 - 98, The
runtime allowlist in use-workspace-operations.ts currently omits "quiz", causing
quiz cards to be coerced to "note"; update the allowlist used in the
createItem/createItems logic (the validTypes array and its usage where validType
is computed) to include "quiz", and refactor by extracting that array into a
shared constant (e.g., CARD_TYPES or VALID_CARD_TYPES) exported/used by
createItem and createItems so both call sites use the same source of truth to
avoid future drift.
| @@ -1,4 +1,4 @@ | |||
| import type { CardType, ItemData, NoteData, PdfData, FlashcardData, FolderData, YouTubeData, ImageData, AudioData } from "./types"; | |||
| import type { CardType, ItemData, NoteData, PdfData, FlashcardData, FolderData, YouTubeData, ImageData, AudioData, WebsiteData } from "./types"; | |||
There was a problem hiding this comment.
Handle "quiz" explicitly in defaultDataFor.
The switch still has no case "quiz", so defaultDataFor("quiz") returns NoteData from the default branch. Any quiz creation path that relies on this helper gets an invalid data shape.
🔧 Suggested fix
-import type { CardType, ItemData, NoteData, PdfData, FlashcardData, FolderData, YouTubeData, ImageData, AudioData, WebsiteData } from "./types";
+import type { CardType, ItemData, NoteData, PdfData, FlashcardData, FolderData, YouTubeData, QuizData, ImageData, AudioData, WebsiteData } from "./types";
@@
case "image":
return { url: "" } as ImageData;
case "audio":
return { fileUrl: "", filename: "", processingStatus: "uploading" } as AudioData;
+ case "quiz":
+ return { questions: [] } as QuizData;
case "website":
return { url: "" } as WebsiteData;
- default:
- return { field1: "" } as NoteData;
+ default: {
+ const exhaustiveCheck: never = type;
+ return exhaustiveCheck;
+ }
}
}Also applies to: 37-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/workspace-state/item-helpers.ts` at line 1, The helper defaultDataFor
is missing an explicit branch for "quiz", so update the imports to include the
QuizData type (add QuizData to the import list) and add a case "quiz" in the
defaultDataFor switch that returns a value matching the QuizData interface
(e.g., default questions array and default settings) instead of falling through
to NoteData; ensure the returned object shape exactly matches QuizData and
adjust any other similar switch sites (the other defaultDataFor-like branches
around the same helper) to handle "quiz" the same way.
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib/ai/tools/workspace-search-utils.ts">
<violation number="1" location="src/lib/ai/tools/workspace-search-utils.ts:95">
P2: Website body matches are now searchable, but `readWorkspace` still has no matching website formatter, so the reported line numbers won't round-trip.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| try { | ||
| domain = new URL(url).hostname.replace(/^www\./, ""); | ||
| } catch {} | ||
| const content = `URL: ${url}\nDomain: ${domain}`; |
Contributor
There was a problem hiding this comment.
P2: Website body matches are now searchable, but readWorkspace still has no matching website formatter, so the reported line numbers won't round-trip.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/ai/tools/workspace-search-utils.ts, line 95:
<comment>Website body matches are now searchable, but `readWorkspace` still has no matching website formatter, so the reported line numbers won't round-trip.</comment>
<file context>
@@ -85,6 +85,16 @@ export function extractSearchableText(item: Item, items: Item[]): SearchableText
+ try {
+ domain = new URL(url).hostname.replace(/^www\./, "");
+ } catch {}
+ const content = `URL: ${url}\nDomain: ${domain}`;
+ return body(content);
+ }
</file context>
This was referenced Apr 1, 2026
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