feat: add feedback form and table #2740
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a full feedback system: client FeedbackModal (drafts, attachments, compression, uploads), upload/compression utilities, TRPC feedback router (submit, list), DB feedback schema with attachments, email template + sender, env/runtime bindings, UI triggers/icons, and related exports/rewires. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant UI as Client UI (FeedbackModal)
participant IMG as ImageCompression
participant UP as UploadService (Supabase)
participant API as TRPC feedbackRouter
participant DB as Database
participant EM as Email/Webhook
U->>UI: Open "Send Feedback"
UI->>UI: Load draft, capture pageUrl & userAgent
U->>UI: Enter message, optionally files
alt compressible images
UI->>IMG: compressMultipleImages(...)
IMG-->>UI: compressed files / progress
end
alt attachments present
UI->>UP: uploadFeedbackAttachments(files)
UP-->>UI: signed URLs (attachments) / progress
end
U->>UI: Submit
UI->>API: submit({message,email?,pageUrl,userAgent,attachments,metadata})
API->>DB: insert feedback (rate check)
DB-->>API: inserted id
par Non-blocking side effects
API->>EM: send email (if configured)
API->>EM: post webhook (if configured)
end
API-->>UI: { success: true, id }
UI->>UI: clear draft, toast, close modal
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Updates to Preview Branch (feat/feedback-form) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| @@ -0,0 +1,161 @@ | |||
| import { env } from '@/env'; | |||
| import { trackEvent } from '@/utils/analytics/server'; | |||
| import { callUserWebhook } from '@/utils/n8n/webhook'; | |||
There was a problem hiding this comment.
Remove the unused import 'callUserWebhook' to keep the code base clean.
| import { callUserWebhook } from '@/utils/n8n/webhook'; |
| return ( | ||
| <AnimatePresence> | ||
| {isOpen && ( | ||
| <motion.div |
There was a problem hiding this comment.
To improve accessibility, add ARIA attributes (e.g. role="dialog" and aria-modal="true") to the modal container so screen readers can properly identify it.
| <motion.div | |
| <motion.div role="dialog" aria-modal="true" |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| if (env.N8N_WEBHOOK_URL && env.N8N_API_KEY) { | ||
| await fetch(env.N8N_WEBHOOK_URL, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'n8n-api-key': env.N8N_API_KEY, | ||
| 'Content-Type': 'application/json' | ||
| }, | ||
| body: JSON.stringify({ | ||
| type: 'feedback', | ||
| message: feedback.message, | ||
| userEmail: feedback.email, | ||
| userName: userId ? ctx.user?.user_metadata?.name || ctx.user?.user_metadata?.display_name : 'Anonymous', | ||
| pageUrl: feedback.pageUrl, | ||
| submittedAt: feedback.createdAt.toISOString(), | ||
| feedbackId: feedback.id, | ||
| }), | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to send N8N webhook:', error); | ||
| // Don't throw error - webhook failure shouldn't block feedback submission | ||
| } |
There was a problem hiding this comment.
The code imports callUserWebhook utility but then uses a direct fetch call instead. Consider using the imported utility function to maintain consistent webhook handling patterns across the codebase:
await callUserWebhook({
type: 'feedback',
message: feedback.message,
userEmail: feedback.email,
// other properties...
});This would leverage any existing error handling, authentication, and logging mechanisms already established in the utility function.
| if (env.N8N_WEBHOOK_URL && env.N8N_API_KEY) { | |
| await fetch(env.N8N_WEBHOOK_URL, { | |
| method: 'POST', | |
| headers: { | |
| 'n8n-api-key': env.N8N_API_KEY, | |
| 'Content-Type': 'application/json' | |
| }, | |
| body: JSON.stringify({ | |
| type: 'feedback', | |
| message: feedback.message, | |
| userEmail: feedback.email, | |
| userName: userId ? ctx.user?.user_metadata?.name || ctx.user?.user_metadata?.display_name : 'Anonymous', | |
| pageUrl: feedback.pageUrl, | |
| submittedAt: feedback.createdAt.toISOString(), | |
| feedbackId: feedback.id, | |
| }), | |
| }); | |
| } | |
| } catch (error) { | |
| console.error('Failed to send N8N webhook:', error); | |
| // Don't throw error - webhook failure shouldn't block feedback submission | |
| } | |
| if (env.N8N_WEBHOOK_URL && env.N8N_API_KEY) { | |
| await callUserWebhook({ | |
| type: 'feedback', | |
| message: feedback.message, | |
| userEmail: feedback.email, | |
| userName: userId ? ctx.user?.user_metadata?.name || ctx.user?.user_metadata?.display_name : 'Anonymous', | |
| pageUrl: feedback.pageUrl, | |
| submittedAt: feedback.createdAt.toISOString(), | |
| feedbackId: feedback.id, | |
| }); | |
| } | |
| } catch (error) { | |
| console.error('Failed to send N8N webhook:', error); | |
| // Don't throw error - webhook failure shouldn't block feedback submission | |
| } | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| if (!ctx.user || ctx.user.email !== '[email protected]') { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
Security concern: Using a hardcoded email address ([email protected]) for admin access control creates a security vulnerability. This approach is both inflexible and insecure - if the email needs to change or is compromised, it requires a code deployment to fix.
Consider implementing:
- A proper role-based authorization system
- An environment variable for admin emails (
[email protected],[email protected]) - A database table for admin users with appropriate access controls
This would improve security posture and make administration more maintainable in the long term.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (20)
packages/email/src/templates/feedback-notification.tsx (2)
69-70: Stabilize date formatting; prefer explicit locale/timezone for deterministic emails.Relying on toLocaleString() yields environment-dependent output. Render a consistent timestamp (e.g., UTC).
- <strong>Submitted:</strong> {submittedAt.toLocaleString()} + <strong>Submitted:</strong>{' '} + {new Intl.DateTimeFormat('en-US', { + dateStyle: 'medium', + timeStyle: 'short', + timeZone: 'UTC', + }).format(submittedAt)}{' '} + (UTC)
34-35: Nit: normalize preview text to avoid newlines/whitespace bloating the email preview.Trimming and collapsing whitespace improves previews in most clients.
- const previewText = `New feedback: ${message.substring(0, 100)}${message.length > 100 ? '...' : ''}`; + const normalized = message.replace(/\s+/g, ' ').trim(); + const previewText = `New feedback: ${normalized.slice(0, 100)}${normalized.length > 100 ? '...' : ''}`;packages/db/src/schema/feedback/index.ts (1)
1-1: Local feedback barrel added — matches schema structure.Solid. If this module exports both types and values, consider explicit named re-exports for API stability in the future, but not required now.
packages/email/src/index.ts (1)
2-2: Re-exporting feedback email utilities — consider explicit export to avoid accidental surface growth.If
./feedbackexports more than the notifier function in the future, wildcard re-export could unintentionally expand the public API. Optional tweak:-export * from './feedback'; +export { sendFeedbackNotificationEmail } from './feedback';apps/web/client/src/server/api/root.ts (1)
6-6: VerifyfeedbackRoutersecurity, abuse protections, and PII handlingBefore wider rollout, tighten the new
feedbackRouterin apps/web/client/src/server/api/routers/feedback.ts:
Access control (list endpoint, lines 143–145):
Currentlylist: publicProcedure.query(…)Switch to an auth-guarded or admin-only procedure (e.g.
protectedProcedure) with an explicit role/permission check so only authorized users can fetch feedback.Abuse protection (submit mutation, lines 14–33):
The built-in rate limit only applies whenctx.user?.idis present. Anonymous submissions are currently unlimited. Add a coarse client key (e.g. IP address or device fingerprint) and enforce short-window counters (e.g. Redis) for anonymous callers to prevent spam.Privacy (analytics identifier, lines 120–123):
Emitting raw emails indistinctId: userId || \anonymous-${feedback.email}`leaks PII. Hash any email before use (for example,sha256(feedback.email)) or fall back to a generic"anon"` key.These changes are non-blocking for the initial mapping but strongly recommended before promoting this router to production.
apps/web/client/src/app/project/[id]/_components/left-panel/help-dropdown/index.tsx (3)
1-1: Add "use client" directive if this component isn’t already under a client boundaryThis file uses React hooks and event handlers; ensure it’s compiled as a client component. If a parent doesn’t already enforce a client boundary, add the directive.
+"use client"; + import { FeedbackModal } from '@/components/feedback';
108-117: Localize the “Send Feedback” menu itemHardcoded English string; align with the rest of the menu using next-intl keys.
<DropdownMenuItem className="text-sm" onClick={() => { setIsDropdownOpen(false); setIsFeedbackModalOpen(true); }} > <Icons.MessageSquare className="w-4 h-4 mr-2" /> - Send Feedback + {t(transKeys.help.menu.sendFeedback)} </DropdownMenuItem>Follow-up: add
help.menu.sendFeedbackto your i18n keys/translations.
158-161: Prefill FeedbackModal with the signed-in user’s email (optional)Passing
userEmailhides the email field and reduces friction. If you have access to the auth context here, passuser?.email.Example (requires adding the auth hook/import):
// at top: // import { useAuthContext } from '@/app/auth/auth-context'; // inside component: // const { user } = useAuthContext(); <FeedbackModal isOpen={isFeedbackModalOpen} onClose={() => setIsFeedbackModalOpen(false)} userEmail={user?.email} />Happy to push a patch if you confirm the preferred source of the user email in this scope.
apps/web/client/src/components/feedback/feedback-trigger.tsx (2)
10-17: Localize tooltip and consider platform-aware shortcut labelHardcoded English and macOS glyph. Suggest using next-intl and a dynamic shortcut label (
⌘⇧Fon macOS,Ctrl+Shift+Felsewhere).-import { FeedbackButton } from './feedback-button'; +import { FeedbackButton } from './feedback-button'; +import { useTranslations } from 'next-intl'; export function FeedbackTrigger() { const { user } = useAuthContext(); - + const t = useTranslations(); + const isMac = typeof navigator !== 'undefined' && /Mac|iPhone|iPad|iPod/.test(navigator.platform); + const shortcut = isMac ? '⌘⇧F' : 'Ctrl+Shift+F'; return ( <FeedbackButton userEmail={user?.email} variant="outline" size="sm" showText={false} - tooltip="Send feedback (⌘⇧F)" + tooltip={t('feedback.trigger.tooltip', { shortcut })} className="fixed bottom-4 right-4 z-40 shadow-lg" /> ); }Add
feedback.trigger.tooltipto i18n as “Send feedback ({shortcut})”.
6-19: Check product UX for duplicate entry pointsYou now have both a floating trigger and a Help menu item to open the same modal. If this is intentional, great; otherwise, consider feature-flagging one or unifying to avoid redundancy.
apps/web/client/src/env.ts (1)
55-58: Server-only feedback email vars look good; keep them server-only and consider centralizing defaults
- FEEDBACK_FROM_EMAIL and FEEDBACK_TO_EMAIL are correctly added to the server schema and are not client-exposed (no NEXT_PUBLIC prefix). LGTM.
- Optional: You’re also defaulting these in packages/email. To avoid drift, consider centralizing the defaults in one place (e.g., the email package) and treating these as required when email sending is enabled. You could enforce “if RESEND_API_KEY is set, then FEEDBACK_* must also be set” with a runtime assertion in the email sender.
Also applies to: 137-140
apps/web/client/src/components/feedback/feedback-button.tsx (2)
29-39: Guard the global shortcut when focus is in inputs/editorsPrevent opening the modal while the user is typing in an input/textarea/contentEditable (common apps bind Ctrl/Cmd+Shift+F). Add a focus guard before handling the key combo.
useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { + const t = e.target as HTMLElement | null; + if (t && (t.tagName === 'INPUT' || t.tagName === 'TEXTAREA' || t.isContentEditable || t.closest('[role="textbox"]'))) { + return; + } if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key.toLowerCase() === 'f') { e.preventDefault(); setIsModalOpen(true); } };
62-65: Platform-aware shortcut hint in tooltipShow “Ctrl+Shift+F” on Windows/Linux and “⌘⇧F” on macOS.
- <div className="text-xs text-foreground-secondary mt-1"> - ⌘⇧F - </div> + <div className="text-xs text-foreground-secondary mt-1"> + {typeof navigator !== 'undefined' && /Mac|iPhone|iPad|iPod/.test(navigator.platform) + ? '⌘⇧F' + : 'Ctrl+Shift+F'} + </div>packages/email/src/feedback.ts (2)
11-15: Avoid logging full rendered HTML in production dry runs (PII risk)Gate the dryRun HTML dump to non-production only. This prevents accidental logging of sensitive content.
if (dryRun) { const rendered = await render(FeedbackNotificationEmail(feedbackParams)); - console.log(rendered); + if (process.env.NODE_ENV !== 'production') { + console.log(rendered); + } return; }
20-25: Add reply-to so maintainers can directly reply to the userSet reply_to to the reporter’s email when provided. Resend supports a reply_to field on send(). (resend.com)
return await client.emails.send({ from: `Onlook Feedback <${fromEmail}>`, to: toEmail, subject: `New Feedback from ${userName || userEmail || 'Anonymous User'}`, react: FeedbackNotificationEmail(feedbackParams), + reply_to: userEmail ?? undefined, });apps/web/client/src/components/feedback/feedback-modal.tsx (2)
124-146: Add basic ARIA for dialog semanticsImprove accessibility by adding dialog semantics and labeling.
- <motion.div + <motion.div className="bg-background border border-border rounded-2xl max-w-md w-full shadow-2xl" initial={{ scale: 0.95 }} animate={{ scale: 1 }} exit={{ scale: 0.95 }} transition={{ duration: 0.2, ease: [0.25, 0.46, 0.45, 0.94] }} - onClick={(e) => e.stopPropagation()} + onClick={(e) => e.stopPropagation()} + role="dialog" + aria-modal="true" + aria-labelledby="feedback-modal-title" > @@ - <h2 className="text-xl font-semibold text-foreground flex items-center gap-2"> + <h2 id="feedback-modal-title" className="text-xl font-semibold text-foreground flex items-center gap-2">Also applies to: 134-137
86-98: Prefer checking TRPC error codes instead of string-matchingUsing errorMessage.includes('TOO_MANY_REQUESTS') is brittle. Prefer checking the TRPC client error code if available (e.g., error.data?.code === 'TOO_MANY_REQUESTS').
If you want, I can wire a lightweight isTRPCClientError guard and update the branches accordingly.
apps/web/client/src/server/api/routers/feedback.ts (1)
142-150: Hard-coded admin checkReturning data only when ctx.user.email === '[email protected]' is brittle. Prefer a protected procedure with role/permission checks (or RLS-backed service endpoint). For now this route is effectively off for most users, but it’s better to wire a proper guard.
packages/db/src/schema/feedback/feedback.ts (2)
14-14: Prefer unknown over any for metadata to improve type safetyUsing any leaks unsafety downstream. unknown preserves flexibility without sacrificing type guarantees.
Apply this diff:
- metadata: jsonb('metadata').$type<Record<string, any>>().default({}).notNull(), + metadata: jsonb('metadata').$type<Record<string, unknown>>().default({}).notNull(), @@ - metadata: z.record(z.any()).default({}), + metadata: z.record(z.unknown()).default({}),Also applies to: 29-29
25-30: Harden validation: trim inputs, accept empty strings gracefully, support relative URLs, and bound userAgent
- Users often submit empty string for optional fields; preprocess to undefined.
- Trim message to avoid “whitespace-only” passing min(1).
- Allow relative paths for pageUrl (common when reporting in-app pages).
- Bound userAgent length to a sane max to avoid oversized payloads.
Apply this diff:
export const feedbackInsertSchema = createInsertSchema(feedbacks, { - message: z.string().min(1, 'Message is required').max(5000, 'Message is too long'), - email: z.string().email('Invalid email format').optional(), - pageUrl: z.string().url('Invalid URL format').optional(), - metadata: z.record(z.any()).default({}), + message: z.string().trim().min(1, 'Message is required').max(5000, 'Message is too long'), + email: z.preprocess( + (v) => (typeof v === 'string' && v.trim() === '' ? undefined : v), + z.string().email('Invalid email format').optional() + ), + pageUrl: z.preprocess( + (v) => (typeof v === 'string' && v.trim() === '' ? undefined : v), + z.union([z.string().url('Invalid URL format'), z.string().startsWith('/', { message: 'Invalid URL format' })]).optional() + ), + userAgent: z.string().max(1024, 'User-Agent is too long').optional(), + metadata: z.record(z.unknown()).default({}), });Optional follow-up (outside this file): consider stripping sensitive query params (e.g., tokens) from pageUrl server-side before persisting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (21)
apps/backend/supabase/migrations/0017_unusual_black_panther.sql(1 hunks)apps/backend/supabase/migrations/meta/0017_snapshot.json(1 hunks)apps/backend/supabase/migrations/meta/_journal.json(1 hunks)apps/web/client/.env.example(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/help-dropdown/index.tsx(4 hunks)apps/web/client/src/components/feedback/feedback-button.tsx(1 hunks)apps/web/client/src/components/feedback/feedback-modal.tsx(1 hunks)apps/web/client/src/components/feedback/feedback-trigger.tsx(1 hunks)apps/web/client/src/components/feedback/index.ts(1 hunks)apps/web/client/src/env.ts(2 hunks)apps/web/client/src/server/api/root.ts(2 hunks)apps/web/client/src/server/api/routers/feedback.ts(1 hunks)apps/web/client/src/server/api/routers/index.ts(1 hunks)packages/db/src/schema/feedback/feedback.ts(1 hunks)packages/db/src/schema/feedback/index.ts(1 hunks)packages/db/src/schema/index.ts(1 hunks)packages/db/src/schema/user/user.ts(2 hunks)packages/email/src/feedback.ts(1 hunks)packages/email/src/index.ts(1 hunks)packages/email/src/templates/feedback-notification.tsx(1 hunks)packages/email/src/templates/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
apps/web/client/src/server/api/root.ts (1)
apps/web/client/src/server/api/routers/feedback.ts (1)
feedbackRouter(13-161)
apps/web/client/src/components/feedback/feedback-trigger.tsx (2)
apps/web/client/src/app/auth/auth-context.tsx (1)
useAuthContext(66-72)apps/web/client/src/components/feedback/feedback-button.tsx (1)
FeedbackButton(18-79)
apps/web/client/src/app/project/[id]/_components/left-panel/help-dropdown/index.tsx (1)
apps/web/client/src/components/feedback/feedback-modal.tsx (1)
FeedbackModal(21-231)
apps/web/client/src/components/feedback/feedback-button.tsx (4)
packages/ui/src/components/button.tsx (1)
Button(57-57)packages/ui/src/components/icons/index.tsx (1)
Icons(136-3590)packages/ui/src/components/tooltip.tsx (3)
Tooltip(72-72)TooltipTrigger(72-72)TooltipContent(72-72)apps/web/client/src/components/feedback/feedback-modal.tsx (1)
FeedbackModal(21-231)
packages/db/src/schema/user/user.ts (1)
packages/db/src/schema/feedback/feedback.ts (1)
feedbacks(7-16)
packages/email/src/feedback.ts (2)
packages/email/src/types/send-email.ts (1)
SendEmailParams(7-7)packages/email/src/templates/feedback-notification.tsx (2)
FeedbackNotificationEmailProps(15-23)FeedbackNotificationEmail(25-119)
apps/web/client/src/server/api/routers/feedback.ts (5)
apps/web/client/src/server/api/trpc.ts (1)
createTRPCRouter(88-88)packages/db/src/schema/feedback/feedback.ts (3)
feedbackSubmitSchema(32-38)feedbacks(7-16)NewFeedback(41-41)apps/web/client/src/env.ts (1)
env(4-151)packages/email/src/feedback.ts (1)
sendFeedbackNotificationEmail(5-26)apps/web/client/src/utils/analytics/server.ts (1)
trackEvent(26-32)
apps/web/client/src/components/feedback/feedback-modal.tsx (3)
apps/web/client/src/trpc/react.tsx (1)
api(23-23)packages/ui/src/components/sonner.tsx (1)
toast(19-19)packages/db/src/schema/feedback/feedback.ts (1)
FeedbackSubmitInput(42-42)
packages/db/src/schema/feedback/feedback.ts (1)
packages/db/src/schema/user/user.ts (1)
users(13-25)
🪛 dotenv-linter (3.3.0)
apps/web/client/.env.example
[warning] 55-55: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 56-56: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Supabase Preview
🔇 Additional comments (16)
apps/backend/supabase/migrations/meta/_journal.json (1)
124-130: Migration artifacts validated – approving code changes
- apps/backend/supabase/migrations/0017_unusual_black_panther.sql exists
- apps/backend/supabase/migrations/meta/0017_snapshot.json exists
- All entries in
_journal.jsonconsistently use"version": "7"No further action required.
apps/backend/supabase/migrations/meta/0017_snapshot.json (2)
957-1016: Unique constraint on (invitee_email, project_id) was removed and replaced by a non-unique index — confirm product intent.Dropping project_invitations_invitee_email_project_id_unique removes a guard against duplicate invitations to the same email for a given project. If duplicates were a bug, keep a unique constraint or unique index.
If uniqueness is still desired, add:
create unique index if not exists project_invitations_invitee_email_project_id_unique on public.project_invitations (invitee_email, project_id);
322-399: Incorrect: Server‐side DB operations bypass RLSAll feedback mutations and queries in feedbackRouter use
ctx.db(Drizzle) connected viaprocess.env.SUPABASE_DATABASE_URL, which is a direct Postgres connection as a superuser (BYPASSRLS) — so RLS on public.feedbacks does not apply to these routes. No policy changes are needed for these server‐only operations.Likely an incorrect or invalid review comment.
packages/email/src/templates/feedback-notification.tsx (1)
65-67: PII check: confirm policy for forwarding userEmail to recipients.You surface userEmail to the notification recipient. Ensure this aligns with your privacy policy and consent expectations, especially if feedback can be anonymous or from non-authenticated users.
packages/email/src/templates/index.ts (1)
1-1: Barrel export addition looks good.Re-exporting the template here keeps the templates API coherent.
apps/backend/supabase/migrations/0017_unusual_black_panther.sql (1)
15-28: Re “project_invitations” uniqueness: dropping composite unique in favor of a non-unique index changes semantics.If the goal is to keep one active invite per (invitee_email, project_id), preserve uniqueness (constraint or unique index). If duplicates are intentional, ignore.
Option to preserve uniqueness:
-ALTER TABLE "project_invitations" DROP CONSTRAINT "project_invitations_invitee_email_project_id_unique";--> statement-breakpoint +ALTER TABLE "project_invitations" DROP CONSTRAINT "project_invitations_invitee_email_project_id_unique";--> statement-breakpoint +-- Recreate as an explicit unique index (more flexible than named constraint) +CREATE UNIQUE INDEX IF NOT EXISTS "project_invitations_invitee_email_project_id_unique" + ON "public"."project_invitations" ("invitee_email","project_id");--> statement-breakpointapps/web/client/src/server/api/routers/index.ts (1)
4-4: Router barrel now exposes feedback — consistent with existing pattern.LGTM. Keep this file as the single public surface for routers to avoid deep imports across the codebase.
apps/web/client/src/app/project/[id]/_components/left-panel/help-dropdown/index.tsx (1)
110-113: Good UX: closing dropdown before opening modalClosing the menu first avoids focus/scroll conflicts with the modal. Looks solid.
apps/web/client/.env.example (1)
54-57: Remove quotes around feedback email valuesAll feedback email vars are private (no
NEXT_PUBLIC_prefix), defined in your Zod schema, and only used in server-side code:
- Schema entries in
apps/web/client/src/env.tsFEEDBACK_FROM_EMAIL: z.string().email().optional(), FEEDBACK_TO_EMAIL: z.string().email().optional(),- Server consumption in
packages/email/src/feedback.tsconst fromEmail = process.env.FEEDBACK_FROM_EMAIL || '[email protected]'; const toEmail = process.env.FEEDBACK_TO_EMAIL || '[email protected]';Apply this minimal diff to satisfy dotenv-linter and avoid unnecessary quoting:
# Feedback - Email notifications for user feedback -FEEDBACK_FROM_EMAIL="[email protected]" -FEEDBACK_TO_EMAIL="[email protected]" +[email protected] +[email protected]packages/db/src/schema/user/user.ts (2)
35-36: Relation addition looks correct
usersRelations.feedbacks = many(feedbacks)matches thefeedbacks.userId → users.idFK; cardinality and naming are consistent with the rest of the schema.
4-4: Potential Circular Import Detected BetweenuserandfeedbackSchemasOur ripgrep scan confirms that:
packages/db/src/schema/feedback/feedback.tsimportsusersfrom../user(line 5)packages/db/src/schema/user/user.tsimportsfeedbacksfrom../feedback(line 4)Drizzle’s lazy resolver usually handles such back-references safely, but in certain build or runtime orders you can observe
undefinedexports. To avoid subtle bugs, please:
- Manually verify that module initialization order in your build pipeline never triggers an incomplete export.
- Run key integration tests that exercise both
usersandfeedbackstables together at startup.If issues arise, consider an optional refactor: co-locate these mutually referencing tables in a single module (e.g.
packages/db/src/schema/schema.ts) or split them into more granular modules to break the cycle.apps/web/client/src/components/feedback/index.ts (1)
1-3: Barrel export looks goodCentralizing feedback UI exports simplifies imports (as used by the Help dropdown). No issues.
apps/web/client/src/components/feedback/feedback-trigger.tsx (1)
6-19: LGTM: Simple, safe wrapper around FeedbackButtonReads the user from context and forwards email to prefill the modal. Clean composition.
packages/db/src/schema/feedback/feedback.ts (3)
18-23: Relations look correct and align with the FK (ON DELETE SET NULL, ON UPDATE CASCADE)The one-to-one from feedbacks → users via userId is appropriate given nullable FK.
32-38: Submit schema scope is right (no userId client input), confirm userAgent presence via createInsertSchemaLGTM. Excluding userId from client-submittable fields avoids spoofing. Since userAgent wasn’t overridden above initially, it relies on createInsertSchema to be present—ensure your drizzle-zod version includes it (it should), or keep the explicit override from the previous comment.
40-42: Types export set is complete and consistentSelect/Insert inferences and the submit input alias are all sensible.
| @@ -0,0 +1,28 @@ | |||
| CREATE SCHEMA "auth"; | |||
There was a problem hiding this comment.
Do not create Supabase auth schema unconditionally — this will fail in most environments.
Supabase provisions the "auth" schema by default. Use IF NOT EXISTS or remove the statement.
-CREATE SCHEMA "auth";
+-- Supabase manages the "auth" schema; if needed locally, guard it:
+-- CREATE SCHEMA IF NOT EXISTS "auth";📝 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.
| CREATE SCHEMA "auth"; | |
| -- Supabase manages the "auth" schema; if needed locally, guard it: | |
| -- CREATE SCHEMA IF NOT EXISTS "auth"; |
🤖 Prompt for AI Agents
In apps/backend/supabase/migrations/0017_unusual_black_panther.sql around lines
1 to 1, the migration unconditionally creates the "auth" schema which Supabase
typically provisions and will cause failures; update the migration to either
remove this CREATE SCHEMA statement entirely or change it to create the schema
only if it does not already exist (use an IF NOT EXISTS form) so the migration
is safe to run across environments.
| CREATE TABLE "feedbacks" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "user_id" uuid, | ||
| "email" text, | ||
| "message" text NOT NULL, | ||
| "page_url" text, | ||
| "user_agent" text, | ||
| "metadata" jsonb DEFAULT '{}'::jsonb NOT NULL, | ||
| "created_at" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fully-qualify feedbacks table in public schema for clarity and to match snapshot.
The unqualified name currently resolves to public, but being explicit avoids surprises and matches meta snapshot (public.feedbacks).
-CREATE TABLE "feedbacks" (
+CREATE TABLE "public"."feedbacks" (
"id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
"user_id" uuid,
"email" text,
"message" text NOT NULL,
"page_url" text,
"user_agent" text,
"metadata" jsonb DEFAULT '{}'::jsonb NOT NULL,
"created_at" timestamp with time zone DEFAULT now() NOT NULL
);📝 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.
| CREATE TABLE "feedbacks" ( | |
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | |
| "user_id" uuid, | |
| "email" text, | |
| "message" text NOT NULL, | |
| "page_url" text, | |
| "user_agent" text, | |
| "metadata" jsonb DEFAULT '{}'::jsonb NOT NULL, | |
| "created_at" timestamp with time zone DEFAULT now() NOT NULL | |
| ); | |
| CREATE TABLE "public"."feedbacks" ( | |
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | |
| "user_id" uuid, | |
| "email" text, | |
| "message" text NOT NULL, | |
| "page_url" text, | |
| "user_agent" text, | |
| "metadata" jsonb DEFAULT '{}'::jsonb NOT NULL, | |
| "created_at" timestamp with time zone DEFAULT now() NOT NULL | |
| ); |
🤖 Prompt for AI Agents
In apps/backend/supabase/migrations/0017_unusual_black_panther.sql around lines
3 to 12, the CREATE TABLE uses an unqualified "feedbacks" name; update the
statement to fully qualify the table in the public schema (e.g. CREATE TABLE
public."feedbacks" (...)) so the migration explicitly targets public and matches
the meta snapshot; ensure any matching DROP/ALTER or references in the same file
are similarly qualified.
| "created_at" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| ALTER TABLE "feedbacks" ENABLE ROW LEVEL SECURITY;--> statement-breakpoint |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Keep schema-qualified references and add minimal policies/indexes for RLS-enabled table.
RLS is enabled but no policies are defined here; without a service role, inserts/selects will fail. Also add indexes for common access paths.
-ALTER TABLE "feedbacks" ENABLE ROW LEVEL SECURITY;
+ALTER TABLE "public"."feedbacks" ENABLE ROW LEVEL SECURITY;
+
+-- Optional but recommended: performance
+CREATE INDEX IF NOT EXISTS "feedbacks_created_at_idx" ON "public"."feedbacks" ("created_at");
+CREATE INDEX IF NOT EXISTS "feedbacks_user_id_idx" ON "public"."feedbacks" ("user_id");
+
+-- Policies: choose one path depending on how feedback is submitted.
+-- A) If submitted from clients (including unauthenticated users):
+-- create policy "feedback_insert_clients"
+-- on public.feedbacks for insert to anon, authenticated with check (true);
+--
+-- B) If only the server inserts with service key: keep RLS and omit insert policies.
+-- Optionally restrict selects:
+-- create policy "feedback_select_none"
+-- on public.feedbacks for select to public using (false);📝 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.
| ALTER TABLE "feedbacks" ENABLE ROW LEVEL SECURITY;--> statement-breakpoint | |
| -- Enable row level security on feedbacks table | |
| ALTER TABLE "public"."feedbacks" ENABLE ROW LEVEL SECURITY; | |
| -- Optional but recommended: performance | |
| CREATE INDEX IF NOT EXISTS "feedbacks_created_at_idx" ON "public"."feedbacks" ("created_at"); | |
| CREATE INDEX IF NOT EXISTS "feedbacks_user_id_idx" ON "public"."feedbacks" ("user_id"); | |
| -- Policies: choose one path depending on how feedback is submitted. | |
| -- A) If submitted from clients (including unauthenticated users): | |
| -- create policy "feedback_insert_clients" | |
| -- on public.feedbacks for insert to anon, authenticated with check (true); | |
| -- | |
| -- B) If only the server inserts with service key: keep RLS and omit insert policies. | |
| -- Optionally restrict selects: | |
| -- create policy "feedback_select_none" | |
| -- on public.feedbacks for select to public using (false); |
🤖 Prompt for AI Agents
In apps/backend/supabase/migrations/0017_unusual_black_panther.sql around line
14, RLS is being enabled on "feedbacks" but the migration neither uses
schema-qualified object names consistently nor defines the minimal row-level
policies and helpful indexes required for the table to remain usable
(inserts/selects will fail for non-service roles). Update the migration to: 1)
reference the table with its schema (e.g., public.feedbacks) consistently; 2)
add minimal RLS policies to allow intended flows (e.g., a policy to allow
authenticated users to SELECT their own rows using auth.uid() or safe_role
checks, a policy to allow INSERTs for authenticated users, and optional
UPDATE/DELETE policies scoped to owner checks); and 3) create indexes for common
access paths (e.g., ON public.feedbacks(created_at) and ON
public.feedbacks(user_id) or whichever columns are used in WHERE clauses) so
queries remain efficient.
| "public.feedbacks": { | ||
| "name": "feedbacks", | ||
| "schema": "", | ||
| "columns": { | ||
| "id": { | ||
| "name": "id", | ||
| "type": "uuid", | ||
| "primaryKey": true, | ||
| "notNull": true, | ||
| "default": "gen_random_uuid()" | ||
| }, | ||
| "user_id": { | ||
| "name": "user_id", | ||
| "type": "uuid", | ||
| "primaryKey": false, | ||
| "notNull": false | ||
| }, | ||
| "email": { | ||
| "name": "email", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": false | ||
| }, | ||
| "message": { | ||
| "name": "message", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": true | ||
| }, | ||
| "page_url": { | ||
| "name": "page_url", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": false | ||
| }, | ||
| "user_agent": { | ||
| "name": "user_agent", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": false | ||
| }, | ||
| "metadata": { | ||
| "name": "metadata", | ||
| "type": "jsonb", | ||
| "primaryKey": false, | ||
| "notNull": true, | ||
| "default": "'{}'::jsonb" | ||
| }, | ||
| "created_at": { | ||
| "name": "created_at", | ||
| "type": "timestamp with time zone", | ||
| "primaryKey": false, | ||
| "notNull": true, | ||
| "default": "now()" | ||
| } | ||
| }, | ||
| "indexes": {}, | ||
| "foreignKeys": { | ||
| "feedbacks_user_id_users_id_fk": { | ||
| "name": "feedbacks_user_id_users_id_fk", | ||
| "tableFrom": "feedbacks", | ||
| "tableTo": "users", | ||
| "columnsFrom": [ | ||
| "user_id" | ||
| ], | ||
| "columnsTo": [ | ||
| "id" | ||
| ], | ||
| "onDelete": "set null", | ||
| "onUpdate": "cascade" | ||
| } | ||
| }, | ||
| "compositePrimaryKeys": {}, | ||
| "uniqueConstraints": {}, | ||
| "policies": {}, | ||
| "checkConstraints": {}, | ||
| "isRLSEnabled": true | ||
| }, |
There was a problem hiding this comment.
Feedback table lives in public schema, but migration also creates schema "auth" without using it — resolve schema intent.
The snapshot shows public.feedbacks. The migration creates schema "auth" but then creates an unqualified "feedbacks" table (which resolves to public). Creating "auth" here is unnecessary and on Supabase will likely fail if it already exists. Make the migration explicit and consistent:
- Either drop the schema creation entirely, or use IF NOT EXISTS.
- Fully-qualify the feedbacks table as public.feedbacks in all statements.
I’ve proposed the concrete SQL fix under the migration file comment.
🤖 Prompt for AI Agents
In apps/backend/supabase/migrations/meta/0017_snapshot.json around lines 322 to
399 the migration created the "auth" schema but the snapshot shows the feedbacks
table lives in the public schema (unqualified "feedbacks" resolves to public),
causing schema intent mismatch and potential failures if "auth" already exists;
fix by removing the unused "auth" schema creation (or change it to CREATE SCHEMA
IF NOT EXISTS auth) and make all feedback table statements fully qualified as
public.feedbacks throughout the migration so the schema is explicit and
consistent.
| const handleSubmit = async (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
|
|
||
| if (!message.trim()) { | ||
| toast.error('Please enter your feedback message'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Type mismatch: calling handleSubmit(KeyboardEvent) where signature expects FormEvent
Passing a React.KeyboardEvent to a function typed as React.FormEvent is a TS error and can mask real issues. Split into a no-arg submit function and call it from both the form onSubmit (preventDefault) and the keyboard handler.
- const handleSubmit = async (e: React.FormEvent) => {
- e.preventDefault();
+ const submitForm = async () => {
if (!message.trim()) {
toast.error('Please enter your feedback message');
return;
}
@@
- const feedbackData: FeedbackSubmitInput = {
+ const feedbackData: FeedbackSubmitInput = {
message: message.trim(),
- email: email.trim() || null,
+ // Don't send an email from the client if the user is authenticated.
+ email: userEmail ? null : (email.trim() || null),
pageUrl,
userAgent,
metadata: {
route: pathname,
timestamp: new Date().toISOString(),
screenWidth: window.innerWidth,
screenHeight: window.innerHeight,
},
};
@@
- const handleKeyDown = (e: React.KeyboardEvent) => {
+ const handleKeyDown = (e: React.KeyboardEvent) => {
if (e.key === 'Escape') {
onClose();
}
if (e.key === 'Enter' && (e.metaKey || e.ctrlKey)) {
- handleSubmit(e);
+ void submitForm();
}
};
@@
- <form onSubmit={handleSubmit} className="space-y-4">
+ <form
+ onSubmit={(e) => {
+ e.preventDefault();
+ void submitForm();
+ }}
+ className="space-y-4"
+ >Also applies to: 104-111, 148-149, 166-177
🤖 Prompt for AI Agents
In apps/web/client/src/components/feedback/feedback-modal.tsx around lines 50-56
(and also address similar issues at 104-111, 148-149, 166-177): the submit
handler is typed as (e: React.FormEvent) but is being called from keyboard
handlers with React.KeyboardEvent, causing a type mismatch; refactor by
extracting a no-arg async submit function (e.g., submitFeedback()) that contains
the validation and submission logic, update the form onSubmit to call a thin
wrapper that accepts the FormEvent, calls preventDefault() and then await
submitFeedback(), and update keyboard handlers to call submitFeedback() directly
(or return early) so no KeyboardEvent is passed into the FormEvent-typed
function; update types and any references accordingly and run TypeScript to
ensure errors are resolved.
| // Track feedback submission event | ||
| try { | ||
| await trackEvent({ | ||
| distinctId: userId || `anonymous-${feedback.email}`, | ||
| event: 'feedback_submitted', | ||
| properties: { | ||
| feedbackId: feedback.id, | ||
| hasEmail: !!feedback.email, | ||
| messageLength: feedback.message.length, | ||
| pageUrl: feedback.pageUrl, | ||
| isAuthenticated: !!userId, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Avoid sending raw email addresses to analytics (PII)
distinctId includes the user’s email for anonymous users. That’s PII. Use the userId when available and a hash of the email otherwise.
- await trackEvent({
- distinctId: userId || `anonymous-${feedback.email}`,
+ // Hash email to avoid leaking PII in analytics identifiers
+ const anonHash = feedback.email
+ ? require('node:crypto').createHash('sha256').update(feedback.email.toLowerCase()).digest('hex')
+ : 'none';
+ await trackEvent({
+ distinctId: userId || `anon-${anonHash}`,
event: 'feedback_submitted',
properties: {
feedbackId: feedback.id,
hasEmail: !!feedback.email,
messageLength: feedback.message.length,
pageUrl: feedback.pageUrl,
isAuthenticated: !!userId,
},
});If preferred, I can refactor this to a small helper to keep the router lean.
📝 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.
| // Track feedback submission event | |
| try { | |
| await trackEvent({ | |
| distinctId: userId || `anonymous-${feedback.email}`, | |
| event: 'feedback_submitted', | |
| properties: { | |
| feedbackId: feedback.id, | |
| hasEmail: !!feedback.email, | |
| messageLength: feedback.message.length, | |
| pageUrl: feedback.pageUrl, | |
| isAuthenticated: !!userId, | |
| }, | |
| }); | |
| // Track feedback submission event | |
| try { | |
| // Hash email to avoid leaking PII in analytics identifiers | |
| const anonHash = feedback.email | |
| ? require('node:crypto') | |
| .createHash('sha256') | |
| .update(feedback.email.toLowerCase()) | |
| .digest('hex') | |
| : 'none'; | |
| await trackEvent({ | |
| distinctId: userId || `anon-${anonHash}`, | |
| event: 'feedback_submitted', | |
| properties: { | |
| feedbackId: feedback.id, | |
| hasEmail: !!feedback.email, | |
| messageLength: feedback.message.length, | |
| pageUrl: feedback.pageUrl, | |
| isAuthenticated: !!userId, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/feedback.ts around lines 119 to 131,
the distinctId currently uses the raw email for anonymous users which leaks PII;
change it to use userId when present and otherwise a one-way hash of the email
(e.g., SHA-256 hex) prefixed with "anonymous-" so analytics get a non-reversible
identifier; implement the hash inline or extract a small helper
(hashEmail(email): string) that normalizes the email, computes a SHA-256 (or
other secure) digest, returns "anonymous-{digest}", and use that value as
distinctId.
| export const feedbacks = pgTable('feedbacks', { | ||
| id: uuid('id').primaryKey().defaultRandom().notNull(), | ||
| userId: uuid('user_id').references(() => users.id, { onDelete: 'set null', onUpdate: 'cascade' }), | ||
| email: text('email'), | ||
| message: text('message').notNull(), | ||
| pageUrl: text('page_url'), | ||
| userAgent: text('user_agent'), | ||
| metadata: jsonb('metadata').$type<Record<string, any>>().default({}).notNull(), | ||
| createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(), | ||
| }).enableRLS(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add practical indexes and verify RLS policies exist
- Add indexes on user_id and created_at; you’ll almost certainly filter or sort by these in list views and admin dashboards.
- You enabled RLS on the table, but the migration must also create explicit policies (e.g., allow only service-role inserts, and restrict selects to admins). Otherwise, client ops may fail or, worse, be too permissive if later granted broadly.
Apply this diff to add indexes:
-import { jsonb, pgTable, text, timestamp, uuid } from 'drizzle-orm/pg-core';
+import { jsonb, pgTable, text, timestamp, uuid, index } from 'drizzle-orm/pg-core';
@@
-export const feedbacks = pgTable('feedbacks', {
+export const feedbacks = pgTable('feedbacks', {
id: uuid('id').primaryKey().defaultRandom().notNull(),
userId: uuid('user_id').references(() => users.id, { onDelete: 'set null', onUpdate: 'cascade' }),
email: text('email'),
message: text('message').notNull(),
pageUrl: text('page_url'),
userAgent: text('user_agent'),
metadata: jsonb('metadata').$type<Record<string, any>>().default({}).notNull(),
createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(),
-}).enableRLS();
+}, (t) => ({
+ userIdIdx: index('feedbacks_user_id_idx').on(t.userId),
+ createdAtIdx: index('feedbacks_created_at_idx').on(t.createdAt),
+})).enableRLS();Run this script to confirm RLS policies and defaults in the migration:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show statements related to the feedbacks table
rg -nS -C3 'feedbacks' apps/backend/supabase/migrations
# Verify RLS policies exist for feedbacks
rg -nS -C2 'CREATE POLICY|ALTER POLICY|USING \(|WITH CHECK \(' apps/backend/supabase/migrations | rg -nS 'feedbacks' -n -C2
# Verify JSONB default is set at the DB level
rg -nS "DEFAULT '\\{\\}'::jsonb|DEFAULT '{}'::jsonb" apps/backend/supabase/migrations | rg -nS 'feedbacks' -n -C2Length of output: 4683
🏁 Script executed:
#!/bin/bash
# Inspect the feedbacks table definition for the metadata default
sed -n '1,30p' apps/backend/supabase/migrations/0017_unusual_black_panther.sql
# Search all migrations for any CREATE POLICY statements referencing feedbacks
rg -i -n 'CREATE POLICY' -C3 apps/backend/supabase/migrations | rg -i -n 'feedbacks' -C3Length of output: 2277
Add B-tree indexes on user_id/created_at and define missing RLS policies
The 0017_unusual_black_panther.sql migration currently:
- Defines
feedbacks.metadatawith a JSONB default ('{}'::jsonb NOT NULL) ✔ - Enables row-level security on
feedbacks…
• but noCREATE POLICYstatements follow RLS enablement → with RLS enabled and no policies, all access is denied for non-superusers. - Does not add any indexes on
"user_id"or"created_at"→ likely to cause slow list/sort operations.
To resolve these critical gaps, please add a new migration (or amend the existing one) to:
-
Create indexes for efficient queries:
--- apps/backend/supabase/migrations/0017_unusual_black_panther.sql +++ apps/backend/supabase/migrations/0017_add_feedbacks_indexes_and_policies.sql ALTER TABLE "feedbacks" ADD CONSTRAINT "feedbacks_user_id_users_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."users"("id") ON DELETE SET NULL ON UPDATE CASCADE; +CREATE INDEX CONCURRENTLY IF NOT EXISTS "feedbacks_user_id_idx" ON "public"."feedbacks" ("user_id"); +CREATE INDEX CONCURRENTLY IF NOT EXISTS "feedbacks_created_at_idx" ON "public"."feedbacks" ("created_at");
-
Define explicit RLS policies immediately after enabling RLS. For example (adjust roles/conditions to fit your auth model):
ALTER TABLE "public"."feedbacks" ENABLE ROW LEVEL SECURITY; -- Allow the service role to perform all operations CREATE POLICY "feedbacks_service_role_full_access" ON public.feedbacks FOR ALL TO service_role USING (true) WITH CHECK (true); -- Allow authenticated users to insert their own feedback CREATE POLICY "feedbacks_user_insert" ON public.feedbacks FOR INSERT TO authenticated WITH CHECK (auth.uid() = user_id); -- Allow admins to select all feedback CREATE POLICY "feedbacks_admin_select" ON public.feedbacks FOR SELECT TO admin USING (true);
-
Run
supabase db diffor inspectapps/backend/supabase/migrations/meta/0017_snapshot.jsonto confirm the new indexes and policies are reflected in the snapshot.
These changes are critical to prevent broken access (RLS will block all operations without policies) and to avoid severe performance degradation.
🤖 Prompt for AI Agents
packages/db/src/schema/feedback/feedback.ts lines 7-16: RLS was enabled for the
feedbacks table without any CREATE POLICY statements and there are no B-tree
indexes on user_id and created_at; amend or add a new migration that (1) creates
B-tree indexes on public.feedbacks(user_id) and public.feedbacks(created_at) for
efficient lookups/sorts, (2) immediately after ALTER TABLE ... ENABLE ROW LEVEL
SECURITY add explicit policies such as: a service_role policy granting FOR ALL
TO service_role USING (true) WITH CHECK (true); an authenticated insert policy
FOR INSERT TO authenticated WITH CHECK (auth.uid() = user_id); and an admin
select policy FOR SELECT TO admin USING (true) — adjust role names/conditions to
match your auth model, and (3) run supabase db diff or inspect
apps/backend/supabase/migrations/meta/0017_snapshot.json to confirm the indexes
and policies are included in the snapshot.
| try { | ||
| // Compress each image with smart options based on file size | ||
| const compressionResults: CompressionResult[] = []; | ||
| for (let i = 0; i < fileArray.length; i++) { |
There was a problem hiding this comment.
Consider batching compressible files into a single call to compressMultipleImages instead of iterating one by one for potential performance gains.
| reject(new Error('Failed to load image')); | ||
| }; | ||
|
|
||
| img.src = URL.createObjectURL(file); |
There was a problem hiding this comment.
After setting the image source with URL.createObjectURL, consider revoking the object URL (via URL.revokeObjectURL) once the image is loaded to prevent memory leaks.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
packages/db/src/schema/feedback/feedback.ts (3)
40-46: Relax pageUrl validation to allow relative paths (if clients send them)The UI currently passes
window.location.href(absolute), but other clients (or future refactors) may send relative routes like/project/123.z.string().url()will reject those.If you want to accept both absolute and app-relative URLs:
- pageUrl: z.string().url('Invalid URL format').optional(), + // Accept absolute (http/https) or app-relative paths + pageUrl: z.string().regex(/^(\/|https?:\/\/).+/, 'Invalid URL format').optional(),
23-23: RLS enablement without policies will block all accessEnabling RLS is great, but without explicit policies your non-superuser operations will fail. Ensure the migration adds policies for service-role, authenticated inserts, and admin reads.
If you want, I can generate a migration snippet for Supabase (SQL) that grants:
- FOR ALL to service_role
- FOR INSERT to authenticated with
auth.uid() = user_id- FOR SELECT to admin
7-23: Add indexes on frequently filtered columns (user_id, created_at)List views and admin dashboards will almost certainly filter by user and date. Lack of btree indexes will hurt as data grows.
Apply this diff:
-import { jsonb, pgTable, text, timestamp, uuid } from 'drizzle-orm/pg-core'; +import { jsonb, pgTable, text, timestamp, uuid, index } from 'drizzle-orm/pg-core'; @@ -export const feedbacks = pgTable('feedbacks', { +export const feedbacks = pgTable('feedbacks', { id: uuid('id').primaryKey().defaultRandom().notNull(), userId: uuid('user_id').references(() => users.id, { onDelete: 'set null', onUpdate: 'cascade' }), email: text('email'), message: text('message').notNull(), pageUrl: text('page_url'), userAgent: text('user_agent'), attachments: jsonb('attachments').$type<Array<{ name: string; size: number; type: string; url: string; uploadedAt: string; }>>().default([]).notNull(), metadata: jsonb('metadata').$type<Record<string, any>>().default({}).notNull(), createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(), -}).enableRLS(); +}, (t) => ({ + userIdIdx: index('feedbacks_user_id_idx').on(t.userId), + createdAtIdx: index('feedbacks_created_at_idx').on(t.createdAt), +})).enableRLS();
🧹 Nitpick comments (14)
apps/web/client/src/components/store/state/manager.ts (1)
7-7: Add explicit actions for opening/closing the feedback modal (and enable autoBind).Direct field mutation works but dedicated actions improve consistency with MobX best practices and make it easier to enforce action-only updates if strict actions are enabled later.
You can extend the class like this (outside the changed line range):
export class StateManager { isSubscriptionModalOpen = false; isSettingsModalOpen = false; isFeedbackModalOpen = false; settingsTab: SettingsTabValue | string = SettingsTabValue.SITE; constructor() { makeAutoObservable(this, {}, { autoBind: true }); } openFeedbackModal() { this.isFeedbackModalOpen = true; } closeFeedbackModal() { this.isFeedbackModalOpen = false; } toggleFeedbackModal() { this.isFeedbackModalOpen = !this.isFeedbackModalOpen; } }apps/web/client/src/utils/upload/image-compression.ts (1)
11-16: Clarify compressionRatio semantics (it’s a percentage).compressionRatio returns a percentage (0–100). Either rename to compressionPercent or document it to avoid confusion.
If you keep the name, tweak the comment:
export interface CompressionResult { file: File; originalSize: number; compressedSize: number; - compressionRatio: number; + compressionRatio: number; // percentage reduction (0–100) }apps/web/client/src/utils/upload/feedback-attachments.ts (4)
91-101: Clearer rejection for unknown/unsupported MIME typesToday the error only echoes the raw MIME string. Consider adding a hint for users (and telemetry) about which types are accepted to reduce confusion and support debugging.
Apply this diff to enrich the message:
- if (!ALLOWED_FILE_TYPES.includes(file.type)) { - errors.push(`File type '${file.type}' is not supported`); + if (!ALLOWED_FILE_TYPES.includes(file.type)) { + const allowed = ALLOWED_FILE_TYPES.join(', '); + errors.push(`File type '${file.type}' is not supported. Allowed types: ${allowed}`); return { isValid: false, errors, warnings, needsCompression }; }
110-121: Unused ‘compressed’ limit: either enforce or remove to avoid driftThe images limit includes a “compressed” target (2MB), but it isn’t used in validation logic. Either enforce a post-compression size guard or remove the constant to avoid misleading configuration.
Example enforcement right after compression (pseudocode – adapt to your compression utilities):
-// Replace original files with compressed versions -processedFiles = compressionResults.map(result => result.file); +// Replace originals; enforce compressed max if present +processedFiles = compressionResults.map(result => { + const f = result.file; + const limits = SIZE_LIMITS.images; + if (f.type.startsWith('image/') && limits?.compressed && f.size > limits.compressed) { + throw new Error(`${f.name} could not be reduced below ${formatFileSize(limits.compressed)} (actual ${formatFileSize(f.size)})`); + } + return f; +});
252-283: Sequential uploads may be noticeably slow on large batchesUploads run strictly sequentially. For up to 10 files this can be slow, especially for videos. A small concurrency pool (e.g., 2–3) keeps UI responsive without overloading the network.
Here is a light-concurrency refactor using a simple worker pool:
-export async function uploadFeedbackAttachments( +export async function uploadFeedbackAttachments( files: File[], userId?: string, onProgress?: (progress: number) => void ): Promise<AttachmentFile[]> { @@ - for (let i = 0; i < files.length; i++) { - const file = files[i]; - if (!file) continue; - - try { - const uploaded = await uploadFeedbackAttachment(file, userId); - uploadedFiles.push(uploaded); - - // Update progress - onProgress?.((i + 1) / files.length * 100); - } catch (error) { - // If one file fails, we should clean up successfully uploaded files - await cleanupFailedUploads(uploadedFiles); - throw new Error(`Failed to upload ${file.name}: ${error instanceof Error ? error.message : 'Unknown error'}`); - } - } + const concurrency = Math.min(3, files.length); + let completed = 0; + const queue = [...files]; + const runWorker = async () => { + while (queue.length) { + const file = queue.shift(); + if (!file) continue; + try { + const uploaded = await uploadFeedbackAttachment(file, userId); + uploadedFiles.push(uploaded); + completed += 1; + onProgress?.((completed / files.length) * 100); + } catch (error) { + await cleanupFailedUploads(uploadedFiles); + throw new Error(`Failed to upload ${file.name}: ${error instanceof Error ? error.message : 'Unknown error'}`); + } + } + }; + await Promise.all(Array.from({ length: concurrency }, runWorker));
141-201: Batch validation UX: distinguish hard errors from soft warningsGreat aggregation. For UX polish, sort errors before warnings and ensure duplicate messages are de-duplicated to avoid flooding toasts on many files.
apps/web/client/src/components/ui/feedback-modal.tsx (8)
79-106: Draft restore logic: avoid toast on empty restore and guard stale attachmentsNice UX on draft restoration. Two small tweaks:
- Skip the toast if only an email value was restored.
- If you ever change storage paths, consider validating that restored
attachmentsstill resolve (HEAD request) before rendering them.
116-121: Capture pageUrl/userAgent only when the modal opensYou already re-capture on open; the initial mount capture can be dropped to prevent stale values if users navigate before opening.
Apply this diff:
-useEffect(() => { - if (typeof window !== 'undefined') { - setPageUrl(window.location.href); - setUserAgent(navigator.userAgent); - } -}, []); +// capture happens in the open-effect below
123-143: Reset-then-restore order is good; also reset file input valueYou reset local state on open and then restore the draft—good. Add a file-input reset to ensure the hidden input doesn’t retain a previous FileList when the modal is reopened.
if (stateManager.isFeedbackModalOpen) { // Reset form state first setMessage(''); setEmail(user?.email || ''); setAttachments([]); + if (fileInputRef.current) fileInputRef.current.value = ''; setIsUploading(false);
156-215: Compression loop correctness and progress granularity
- The progress math is sensible; consider normalizing to integers for a steadier UI and use
Math.min(100, …)to avoid 101% rounding.- You’re invoking a “multiple images” helper per-image; if available, calling it once for the full array can reduce overhead and simplify progress math.
-const overallProgress = ((i + totalProgress / 100) / fileArray.length) * 100; -setCompressionProgress(overallProgress); +const overall = Math.min(100, ((i + totalProgress / 100) / fileArray.length) * 100); +setCompressionProgress(Math.round(overall));
240-265: Uploads: better error surface and preserve partial success optionCurrently a single failure triggers cleanup of all prior uploads. For feedback flows, partial success (keep the successfully uploaded attachments and report which ones failed) often beats all-or-nothing.
If acceptable, keep successes and only notify the failures; otherwise keep current behavior.
421-431: Accept list and server validator divergenceThe input accepts images/PDF/TXT/JSON/ZIP/MP4/WEBM/MOV, but your server-side validator also allows CSV, RTF, and a few archive/video types (7z, RAR, AVI). Decide intentionally: either allow those on the client or drop them from
FILE_TYPE_CONFIGto stay consistent.-accept="image/*,.pdf,.txt,.json,.zip,.mp4,.webm,.mov" +accept="image/*,.pdf,.txt,.json,.csv,.rtf,.zip,.7z,.rar,.mp4,.webm,.mov,.avi"If you don’t want to accept them, remove the types from
FILE_TYPE_CONFIG.
494-511: Disable submit when there are validation errors from attachmentsYou already disable submit during upload/compression; consider also disabling when
attachments.length > 10or total size > 100MB (if you surface those states in component state) to prevent a round-trip rejection.
522-527: Privacy note is great; add link to privacy policy if availableA small link (“Privacy Policy”) builds trust since users may include screenshots with sensitive content.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
CLAUDE.md(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/help-dropdown/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/main.tsx(2 hunks)apps/web/client/src/components/store/state/manager.ts(1 hunks)apps/web/client/src/components/ui/feedback-modal.tsx(1 hunks)apps/web/client/src/server/api/routers/feedback.ts(1 hunks)apps/web/client/src/utils/constants/index.ts(1 hunks)apps/web/client/src/utils/upload/feedback-attachments.ts(1 hunks)apps/web/client/src/utils/upload/image-compression.ts(1 hunks)package.json(1 hunks)packages/db/src/schema/feedback/feedback.ts(1 hunks)packages/email/src/templates/feedback-notification.tsx(1 hunks)packages/ui/src/components/icons/index.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/client/src/app/project/[id]/_components/left-panel/help-dropdown/index.tsx
- packages/email/src/templates/feedback-notification.tsx
- apps/web/client/src/server/api/routers/feedback.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-13T17:15:52.910Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-13T17:15:52.910Z
Learning: Use Bun for all workspace tasks; do not use npm
Applied to files:
CLAUDE.mdpackage.json
📚 Learning: 2025-08-13T17:15:52.910Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-13T17:15:52.910Z
Learning: Run type checking with: `cd apps/web/client && bun run typecheck`
Applied to files:
CLAUDE.mdpackage.json
📚 Learning: 2025-08-13T17:15:52.910Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-13T17:15:52.910Z
Learning: Run unit tests using `bun test`
Applied to files:
CLAUDE.md
📚 Learning: 2025-08-15T04:00:01.345Z
Learnt from: Raj-G07
PR: onlook-dev/onlook#2680
File: apps/web/preload/Dockerfile:20-20
Timestamp: 2025-08-15T04:00:01.345Z
Learning: When migrating Bun commands from docker-compose.yml to Dockerfile CMD, both "bun run server/index.ts" and "bun server/index.ts" work for direct file execution, but the latter is more explicit and efficient when no package.json scripts are involved.
Applied to files:
CLAUDE.md
🧬 Code graph analysis (1)
packages/db/src/schema/feedback/feedback.ts (1)
packages/db/src/schema/user/user.ts (1)
users(13-25)
🪛 LanguageTool
CLAUDE.md
[grammar] ~2-~2: There might be a mistake here.
Context: ...n, not npm - Unit tests can be ran with bun test - Run type checking with `bun run typechec...
(QB_NEW_EN)
[grammar] ~3-~3: There might be a mistake here.
Context: ...ith bun test - Run type checking with bun run typecheck - Database updates can be applied to local...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
🔇 Additional comments (5)
apps/web/client/src/utils/constants/index.ts (1)
44-45: FEEDBACK_DRAFT key addition—LGTM.Consistent naming with existing keys and scoped via as const. No issues spotted.
package.json (2)
33-34: Root typecheck delegation—LGTM.Delegating to apps/web/client keeps the scope tight and aligns with the workspace note to use Bun.
33-34: Client typecheck script and workspace package names verified
- The monorepo defines both
@onlook/web(in apps/web/package.json) and@onlook/web-client(in apps/web/client/package.json), matching the expected workspace package names.apps/web/client/package.jsonincludes atypecheckscript (tsc --noEmit), which will be correctly invoked bycd apps/web/client && bun run typecheck.No changes required here.
apps/web/client/src/app/project/[id]/_components/main.tsx (1)
137-137: Modal placement LGTMRendering the modal once at the app shell level avoids duplicate mounts and simplifies z-indexing relative to the editor. Nice.
packages/ui/src/components/icons/index.tsx (1)
120-123: Consistent icon surfacing and namingThe new MessageSquare icon is correctly imported and exposed with a consistent name. This keeps the API predictable for consumers.
Also applies to: 1656-1657
| const handleSubmit = async (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
|
|
||
| if (!message.trim()) { | ||
| toast.error('Please enter your feedback message'); | ||
| return; | ||
| } | ||
|
|
||
| if (!user?.email && !email.trim()) { | ||
| toast.error('Please enter your email address'); | ||
| return; | ||
| } | ||
|
|
||
| setIsSubmitting(true); | ||
|
|
||
| try { | ||
| const feedbackData: FeedbackSubmitInput = { | ||
| message: message.trim(), | ||
| email: email.trim(), | ||
| pageUrl, | ||
| userAgent, | ||
| attachments, | ||
| metadata: { | ||
| route: pathname, | ||
| timestamp: new Date().toISOString(), | ||
| screenWidth: window.innerWidth, | ||
| screenHeight: window.innerHeight, | ||
| }, | ||
| }; | ||
|
|
||
| await submitFeedback(feedbackData); | ||
|
|
||
| // Clear saved draft on successful submission | ||
| await clearFormState(); | ||
|
|
||
| toast.success('Thank you for your feedback!', { | ||
| description: 'We\'ll review it and get back to you if needed.', | ||
| }); | ||
|
|
||
| stateManager.isFeedbackModalOpen = false; | ||
| } catch (error) { | ||
| console.error('Error submitting feedback:', error); | ||
| const errorMessage = error instanceof Error ? error.message : 'Something went wrong'; | ||
|
|
||
| if (errorMessage.includes('TOO_MANY_REQUESTS')) { | ||
| toast.error('Too many feedback submissions', { | ||
| description: 'Please wait before submitting again.', | ||
| }); | ||
| } else { | ||
| toast.error('Failed to submit feedback', { | ||
| description: errorMessage, | ||
| }); | ||
| } | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use typed TRPC error handling instead of string matching
Matching error.message.includes('TOO_MANY_REQUESTS') is brittle. Prefer the TRPC error code, if available.
-} catch (error) {
+} catch (error: unknown) {
console.error('Error submitting feedback:', error);
- const errorMessage = error instanceof Error ? error.message : 'Something went wrong';
-
- if (errorMessage.includes('TOO_MANY_REQUESTS')) {
+ // @ts-ignore: narrow using TRPC client types if available
+ const code = (error as any)?.data?.code ?? (error as any)?.shape?.code;
+ const errorMessage = (error as Error)?.message ?? 'Something went wrong';
+ if (code === 'TOO_MANY_REQUESTS') {
toast.error('Too many feedback submissions', {
description: 'Please wait before submitting again.',
});
} else {
toast.error('Failed to submit feedback', {
description: errorMessage,
});
}📝 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.
| const handleSubmit = async (e: React.FormEvent) => { | |
| e.preventDefault(); | |
| if (!message.trim()) { | |
| toast.error('Please enter your feedback message'); | |
| return; | |
| } | |
| if (!user?.email && !email.trim()) { | |
| toast.error('Please enter your email address'); | |
| return; | |
| } | |
| setIsSubmitting(true); | |
| try { | |
| const feedbackData: FeedbackSubmitInput = { | |
| message: message.trim(), | |
| email: email.trim(), | |
| pageUrl, | |
| userAgent, | |
| attachments, | |
| metadata: { | |
| route: pathname, | |
| timestamp: new Date().toISOString(), | |
| screenWidth: window.innerWidth, | |
| screenHeight: window.innerHeight, | |
| }, | |
| }; | |
| await submitFeedback(feedbackData); | |
| // Clear saved draft on successful submission | |
| await clearFormState(); | |
| toast.success('Thank you for your feedback!', { | |
| description: 'We\'ll review it and get back to you if needed.', | |
| }); | |
| stateManager.isFeedbackModalOpen = false; | |
| } catch (error) { | |
| console.error('Error submitting feedback:', error); | |
| const errorMessage = error instanceof Error ? error.message : 'Something went wrong'; | |
| if (errorMessage.includes('TOO_MANY_REQUESTS')) { | |
| toast.error('Too many feedback submissions', { | |
| description: 'Please wait before submitting again.', | |
| }); | |
| } else { | |
| toast.error('Failed to submit feedback', { | |
| description: errorMessage, | |
| }); | |
| } | |
| } finally { | |
| setIsSubmitting(false); | |
| } | |
| }; | |
| } catch (error: unknown) { | |
| console.error('Error submitting feedback:', error); | |
| // @ts-ignore: narrow using TRPC client types if available | |
| const code = (error as any)?.data?.code ?? (error as any)?.shape?.code; | |
| const errorMessage = (error as Error)?.message ?? 'Something went wrong'; | |
| if (code === 'TOO_MANY_REQUESTS') { | |
| toast.error('Too many feedback submissions', { | |
| description: 'Please wait before submitting again.', | |
| }); | |
| } else { | |
| toast.error('Failed to submit feedback', { | |
| description: errorMessage, | |
| }); | |
| } | |
| } finally { | |
| setIsSubmitting(false); | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/components/ui/feedback-modal.tsx around lines 271–327,
the catch block currently detects rate-limit errors by string-matching
error.message.includes('TOO_MANY_REQUESTS'); replace that brittle check with
typed TRPC error handling: cast/guard the caught error as a TRPC client error
(or the TRPC error shape used in the project) and check error.data?.code (or the
library-specific error.code) for 'TOO_MANY_REQUESTS' instead, leaving a safe
fallback to the existing generic error toast if the typed fields are not
present; ensure any necessary TRPC types/imports are added and keep
console.error for unexpected shapes.
| return ( | ||
| <AnimatePresence> | ||
| {stateManager.isFeedbackModalOpen && ( | ||
| <motion.div | ||
| className="fixed inset-0 z-99 flex items-center justify-center p-4" | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| transition={{ duration: 0.2 }} | ||
| onClick={() => stateManager.isFeedbackModalOpen = false} | ||
| onKeyDown={handleKeyDown} | ||
| > | ||
| <div className="absolute inset-0 bg-black/50 backdrop-blur-sm" /> | ||
|
|
||
| <motion.div | ||
| className="relative bg-background border border-border rounded-2xl max-w-md w-full shadow-2xl max-h-[80vh] overflow-y-auto" | ||
| initial={{ scale: 0.95, opacity: 0 }} | ||
| animate={{ scale: 1, opacity: 1 }} | ||
| exit={{ scale: 0.95, opacity: 0 }} | ||
| transition={{ duration: 0.2, ease: [0.25, 0.46, 0.45, 0.94] }} | ||
| onClick={(e) => e.stopPropagation()} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Accessibility: add dialog semantics and initial focus
Add role="dialog", aria-modal="true", a labelled title, and initial focus on the first interactive element. This improves screen-reader support and keyboard navigation.
<motion.div
- className="fixed inset-0 z-99 flex items-center justify-center p-4"
+ className="fixed inset-0 z-99 flex items-center justify-center p-4"
+ role="dialog"
+ aria-modal="true"
+ aria-labelledby="feedback-modal-title"
...
>
@@
- <h2 className="text-xl font-semibold text-foreground flex items-center gap-2">
+ <h2 id="feedback-modal-title" className="text-xl font-semibold text-foreground flex items-center gap-2">Optionally set autoFocus on the textarea or close 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.
| return ( | |
| <AnimatePresence> | |
| {stateManager.isFeedbackModalOpen && ( | |
| <motion.div | |
| className="fixed inset-0 z-99 flex items-center justify-center p-4" | |
| initial={{ opacity: 0 }} | |
| animate={{ opacity: 1 }} | |
| exit={{ opacity: 0 }} | |
| transition={{ duration: 0.2 }} | |
| onClick={() => stateManager.isFeedbackModalOpen = false} | |
| onKeyDown={handleKeyDown} | |
| > | |
| <div className="absolute inset-0 bg-black/50 backdrop-blur-sm" /> | |
| <motion.div | |
| className="relative bg-background border border-border rounded-2xl max-w-md w-full shadow-2xl max-h-[80vh] overflow-y-auto" | |
| initial={{ scale: 0.95, opacity: 0 }} | |
| animate={{ scale: 1, opacity: 1 }} | |
| exit={{ scale: 0.95, opacity: 0 }} | |
| transition={{ duration: 0.2, ease: [0.25, 0.46, 0.45, 0.94] }} | |
| onClick={(e) => e.stopPropagation()} | |
| > | |
| return ( | |
| <AnimatePresence> | |
| {stateManager.isFeedbackModalOpen && ( | |
| <motion.div | |
| className="fixed inset-0 z-99 flex items-center justify-center p-4" | |
| role="dialog" | |
| aria-modal="true" | |
| aria-labelledby="feedback-modal-title" | |
| initial={{ opacity: 0 }} | |
| animate={{ opacity: 1 }} | |
| exit={{ opacity: 0 }} | |
| transition={{ duration: 0.2 }} | |
| onClick={() => stateManager.isFeedbackModalOpen = false} | |
| onKeyDown={handleKeyDown} | |
| > | |
| <div className="absolute inset-0 bg-black/50 backdrop-blur-sm" /> | |
| <motion.div | |
| className="relative bg-background border border-border rounded-2xl max-w-md w-full shadow-2xl max-h-[80vh] overflow-y-auto" | |
| initial={{ scale: 0.95, opacity: 0 }} | |
| animate={{ scale: 1, opacity: 1 }} | |
| exit={{ scale: 0.95, opacity: 0 }} | |
| transition={{ duration: 0.2, ease: [0.25, 0.46, 0.45, 0.94] }} | |
| onClick={(e) => e.stopPropagation()} | |
| > | |
| <h2 | |
| id="feedback-modal-title" | |
| className="text-xl font-semibold text-foreground flex items-center gap-2" | |
| > |
| // Upload to Supabase storage | ||
| const { data, error } = await supabase.storage | ||
| .from('file_transfer') | ||
| .upload(fileName, file, { | ||
| cacheControl: '3600', | ||
| upsert: false, | ||
| metadata: { | ||
| originalName: file.name, | ||
| uploadedBy: userId || 'anonymous', | ||
| purpose: 'feedback_attachment', | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Public URL derivation and cleanup path robustness
Cleanup derives the path from the public URL by splitting on “/”. This assumes flat paths and no querystrings. If you ever move to foldered keys (e.g., feedback/YYYY/MM/…), or signed URLs with query params, this will break.
Apply a more robust approach by returning the storage path from uploadFeedbackAttachment and storing it alongside the public URL:
-export interface AttachmentFile {
+export interface AttachmentFile {
name: string;
size: number;
type: string;
url: string;
+ path?: string; // storage path within the bucket
uploadedAt: string;
}
@@
- const { data: urlData } = supabase.storage
+ const { data: urlData } = supabase.storage
.from('file_transfer')
.getPublicUrl(data.path);
return {
name: file.name,
size: file.size,
type: file.type,
url: urlData.publicUrl,
+ path: data.path,
uploadedAt: new Date().toISOString(),
};And in cleanup:
-const fileName = file.url.split('/').pop();
-if (fileName) {
- await supabase.storage.from('file_transfer').remove([fileName]);
-}
+const key = file.path ?? file.url.split('/object/public/file_transfer/')[1]?.split('?')[0];
+if (key) {
+ await supabase.storage.from('file_transfer').remove([key]);
+}📝 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.
| // Upload to Supabase storage | |
| const { data, error } = await supabase.storage | |
| .from('file_transfer') | |
| .upload(fileName, file, { | |
| cacheControl: '3600', | |
| upsert: false, | |
| metadata: { | |
| originalName: file.name, | |
| uploadedBy: userId || 'anonymous', | |
| purpose: 'feedback_attachment', | |
| } | |
| }); | |
| // --- In the same file, update the AttachmentFile interface --- | |
| export interface AttachmentFile { | |
| name: string; | |
| size: number; | |
| type: string; | |
| url: string; | |
| path?: string; // storage path within the bucket | |
| uploadedAt: string; | |
| } | |
| // --- In uploadFeedbackAttachment, after the upload call --- | |
| const { data: uploadData, error: uploadError } = await supabase.storage | |
| .from('file_transfer') | |
| .upload(fileName, file, { | |
| cacheControl: '3600', | |
| upsert: false, | |
| metadata: { | |
| originalName: file.name, | |
| uploadedBy: userId || 'anonymous', | |
| purpose: 'feedback_attachment', | |
| } | |
| }); | |
| if (uploadError || !uploadData?.path) { | |
| throw new Error(`Upload failed: ${uploadError?.message}`); | |
| } | |
| // Derive public URL from the returned storage path | |
| const { data: urlData } = supabase.storage | |
| .from('file_transfer') | |
| .getPublicUrl(uploadData.path); | |
| return { | |
| name: file.name, | |
| size: file.size, | |
| type: file.type, | |
| url: urlData.publicUrl, | |
| path: uploadData.path, | |
| uploadedAt: new Date().toISOString(), | |
| }; | |
| // --- In the cleanup routine later on --- | |
| const key = file.path | |
| ?? file.url | |
| .split('/object/public/file_transfer/')[1] | |
| ?.split('?')[0]; | |
| if (key) { | |
| await supabase.storage | |
| .from('file_transfer') | |
| .remove([key]); | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/utils/upload/feedback-attachments.ts around lines 221 to
233, the code currently derives the storage path for cleanup by splitting the
public URL which breaks for foldered keys or signed URLs with querystrings;
change the uploadFeedbackAttachment function to return the actual storage path
(the key used in supabase.storage.from(...).upload) alongside the public URL and
persist that path where you store the attachment info, and update the cleanup
code to use this stored storage path directly when calling
supabase.storage.from('file_transfer').remove(...) (and ensure you
URL-decode/trim any accidental whitespace when reading the stored path) so you
no longer parse or split the public URL to derive the object key.
| format?: 'image/jpeg' | 'image/webp'; | ||
| maxSizeBytes?: number; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve transparency and tighten types for output format.
- Some PNG screenshots/assets have alpha; forcing JPEG for larger files will destroy transparency. Prefer WEBP for PNG/WEBP inputs across sizes.
- Align getExtensionForFormat’s param type with CompressionOptions to avoid drift.
Apply this diff:
@@
- format?: 'image/jpeg' | 'image/webp';
+ format?: 'image/jpeg' | 'image/webp' | 'image/png';
@@
-function getExtensionForFormat(format: string): string {
+function getExtensionForFormat(format: CompressionOptions['format']): string {
switch (format) {
case 'image/jpeg':
return '.jpg';
case 'image/webp':
return '.webp';
case 'image/png':
return '.png';
default:
return '.jpg';
}
}
@@
- if (size > 10 * MB) {
+ if (size > 10 * MB) {
// Very large files - aggressive compression
return {
maxWidth: 1280,
maxHeight: 720,
quality: 0.6,
- format: 'image/jpeg',
+ // Prefer WEBP for PNG/WEBP inputs to preserve alpha and often smaller sizes
+ format: file.type === 'image/png' || file.type === 'image/webp' ? 'image/webp' : 'image/jpeg',
maxSizeBytes: 1 * MB,
};
} else if (size > 5 * MB) {
// Large files - moderate compression
return {
maxWidth: 1600,
maxHeight: 900,
quality: 0.7,
- format: 'image/jpeg',
+ format: file.type === 'image/png' || file.type === 'image/webp' ? 'image/webp' : 'image/jpeg',
maxSizeBytes: 1.5 * MB,
};
} else if (size > 2 * MB) {
// Medium files - light compression
return {
maxWidth: 1920,
maxHeight: 1080,
quality: 0.8,
- format: 'image/jpeg',
+ format: file.type === 'image/png' || file.type === 'image/webp' ? 'image/webp' : 'image/jpeg',
maxSizeBytes: 2 * MB,
};
} else {
// Small files - minimal compression, keep quality
return {
maxWidth: 1920,
maxHeight: 1080,
quality: 0.85,
- format: file.type === 'image/png' ? 'image/webp' : 'image/jpeg',
+ format: file.type === 'image/png' || file.type === 'image/webp' ? 'image/webp' : 'image/jpeg',
maxSizeBytes: 2 * MB,
};
}Also applies to: 191-201, 209-246
🤖 Prompt for AI Agents
In apps/web/client/src/utils/upload/image-compression.ts around lines 7-9,
191-201 and 209-246, the current CompressionOptions and related logic force
'image/jpeg' output which will discard alpha channels and the
getExtensionForFormat function accepts a wider/different type than
CompressionOptions; change CompressionOptions.format to allow 'image/webp' and
preserve alpha by preferring webp when input has transparency (or when format is
'image/webp'), update any code paths that convert/choose formats to avoid
coercing PNG/WEBP to JPEG, and tighten getExtensionForFormat's parameter type to
accept the same CompressionOptions['format'] union so the types stay aligned.
| img.onload = () => { | ||
| try { | ||
| onProgress?.(30); | ||
|
|
||
| // Calculate new dimensions | ||
| let { width, height } = img; | ||
| const aspectRatio = width / height; | ||
|
|
||
| if (width > opts.maxWidth) { | ||
| width = opts.maxWidth; | ||
| height = width / aspectRatio; | ||
| } | ||
| if (height > opts.maxHeight) { | ||
| height = opts.maxHeight; | ||
| width = height * aspectRatio; | ||
| } |
There was a problem hiding this comment.
Revoke object URLs to prevent browser memory leaks.
The object URL created for img.src isn’t revoked. Over time this leaks memory, especially when compressing multiple large images.
Apply this diff:
@@
- const img = new Image();
+ const img = new Image();
+ const objectUrl = URL.createObjectURL(file);
@@
- img.onload = () => {
+ img.onload = () => {
+ // Release the object URL as soon as the image has loaded
+ URL.revokeObjectURL(objectUrl);
@@
- img.onerror = () => {
- reject(new Error('Failed to load image'));
- };
+ img.onerror = () => {
+ // Ensure we also release the object URL on error paths
+ URL.revokeObjectURL(objectUrl);
+ reject(new Error('Failed to load image'));
+ };
@@
- img.src = URL.createObjectURL(file);
- onProgress?.(20);
+ img.src = objectUrl;
+ onProgress?.(20);Also applies to: 137-143, 141-143
🤖 Prompt for AI Agents
In apps/web/client/src/utils/upload/image-compression.ts around lines 56-71
(also apply same fix to 137-143 and 141-143), the object URL assigned to img.src
is never revoked causing browser memory leaks; after the image finishes loading
or errors out (inside img.onload and img.onerror) and after you’ve
drawn/compressed it, call URL.revokeObjectURL(img.src) (or store the created
object URL in a variable and revoke that) to release the blob reference, and
ensure revocation runs in both success and error paths (and finally if you
return early).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* feat: add ResponsiveMockupSection to features page - Import ResponsiveMockupSection component from landing page - Add component after FeaturesHero in features page layout - Matches same positioning as on home page (after hero section) Co-Authored-By: [email protected] <[email protected]> * feat: reduce padding below ResponsiveMockupSection by 30% - Wrap ResponsiveMockupSection in div with pb-14 class - Reduces bottom padding from 5rem to 3.5rem (30% reduction) - Improves spacing between mockup section and benefits section Co-Authored-By: [email protected] <[email protected]> * fix: replace vh units with rem to reduce spacing by 30% - Replace h-screen (100vh) with h-[44rem] for desktop view - Reduce mobile section padding from py-20 to py-14 (5rem to 3.5rem) - Remove incorrect pb-14 wrapper from features page - Provides better spacing control and consistent behavior across viewports Co-Authored-By: [email protected] <[email protected]> * feat: double BenefitsSection padding and reorder mobile layout - Double padding from py-32 to py-64 for desktop only (lg:py-64) - Reorder mobile layout: creative elements first, then heading/title/text - Use CSS flexbox order classes (order-1/order-2) with responsive breakpoints - Desktop layout unchanged (lg:grid-cols-2 with original order) Co-Authored-By: [email protected] <[email protected]> * feat: implement responsive typography hierarchy across features page - Update hero title (h1) from text-6xl to text-4xl md:text-6xl - Update main section titles (h2) to text-2xl md:text-4xl for consistency: - FeaturesIntroSection, CTASection, FeaturesFAQSection - Update feature grid titles (h3) from text-title2 to text-lg md:text-xl - Follow mobile-first responsive design with proper breakpoints - Maintain existing styling properties (font-weight, line-height, margins) Co-Authored-By: [email protected] <[email protected]> * feat: update mobile-only titles in ResponsiveMockupSection to use responsive typography Co-Authored-By: [email protected] <[email protected]> * Update cta-section.tsx * merged FAQs as components, improved typography, styling --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: [email protected] <[email protected]> Co-authored-by: Daniel R Farrell <[email protected]>
…2720) * feat: Add pricing table to pricing page with dynamic authentication - Create new PricingTable component that reuses existing FreeCard and ProCard - Modify FreeCard and ProCard to support unauthenticated users with signup links - Replace 'coming soon' placeholder on pricing page with functional pricing table - Implement dynamic authentication behavior: signup for unauthenticated users, Stripe checkout for authenticated users - Maintain backward compatibility with existing SubscriptionModal usage Co-Authored-By: [email protected] <[email protected]> * feat: Add enterprise card to pricing table with responsive layout - Created EnterpriseCard component with Contact Us mailto button - Updated PricingTable to display 3 cards in responsive layout - Desktop: 3 columns side by side - Mobile: stacked single column - Maintains consistent gap spacing between cards Co-Authored-By: [email protected] <[email protected]> * improved styling and copy * bun.lockb fix --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: [email protected] <[email protected]> Co-authored-by: Daniel R Farrell <[email protected]>
* feat: add ResponsiveMockupSection to features page - Import ResponsiveMockupSection component from landing page - Add component after FeaturesHero in features page layout - Matches same positioning as on home page (after hero section) Co-Authored-By: [email protected] <[email protected]> * feat: reduce padding below ResponsiveMockupSection by 30% - Wrap ResponsiveMockupSection in div with pb-14 class - Reduces bottom padding from 5rem to 3.5rem (30% reduction) - Improves spacing between mockup section and benefits section Co-Authored-By: [email protected] <[email protected]> * fix: replace vh units with rem to reduce spacing by 30% - Replace h-screen (100vh) with h-[44rem] for desktop view - Reduce mobile section padding from py-20 to py-14 (5rem to 3.5rem) - Remove incorrect pb-14 wrapper from features page - Provides better spacing control and consistent behavior across viewports Co-Authored-By: [email protected] <[email protected]> * feat: double BenefitsSection padding and reorder mobile layout - Double padding from py-32 to py-64 for desktop only (lg:py-64) - Reorder mobile layout: creative elements first, then heading/title/text - Use CSS flexbox order classes (order-1/order-2) with responsive breakpoints - Desktop layout unchanged (lg:grid-cols-2 with original order) Co-Authored-By: [email protected] <[email protected]> * feat: implement responsive typography hierarchy across features page - Update hero title (h1) from text-6xl to text-4xl md:text-6xl - Update main section titles (h2) to text-2xl md:text-4xl for consistency: - FeaturesIntroSection, CTASection, FeaturesFAQSection - Update feature grid titles (h3) from text-title2 to text-lg md:text-xl - Follow mobile-first responsive design with proper breakpoints - Maintain existing styling properties (font-weight, line-height, margins) Co-Authored-By: [email protected] <[email protected]> * feat: update mobile-only titles in ResponsiveMockupSection to use responsive typography Co-Authored-By: [email protected] <[email protected]> * Update cta-section.tsx * merged FAQs as components, improved typography, styling * fix FAQs issue --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: [email protected] <[email protected]>
* feat: add RB2B script to web client and docs layouts - Add RB2B tracking script to both main web client and docs layouts - Follow same pattern as Zaraz: production environment gating and lazyOnload strategy - Script loads only in production environments for performance - Maintains consistency with existing marketing script implementations Co-Authored-By: [email protected] <[email protected]> * feat: update RB2B implementation to follow Next.js 13+ best practices - Add NEXT_PUBLIC_RB2B_ID environment variable to env.ts and .env.example - Create dedicated RB2B client components following documentation patterns - Use CloudFront URL format and proper navigation handling with usePathname - Replace hardcoded script implementation with reusable components - Maintain production gating and environment-driven configuration Co-Authored-By: [email protected] <[email protected]> --------- Co-authored-by: [email protected] <[email protected]>
…#2742) * feat: add AI prototype generator features page at /features/prototype - Add new FEATURES_PROTOTYPE route constant - Create prototype-focused features page with AI-generated content - Update hero section with 'AI Prototype Generator' messaging - Add 3 prototype-specific benefit sections - Include 6 prototype features grid (AI generation, interactive components, etc.) - Add 6 prototype-focused FAQs covering tool differences and capabilities - Maintain existing component structure and styling - Update meta tags for prototype generator focus Co-Authored-By: [email protected] <[email protected]> * fix: remove metadata export from client component to fix Vercel deployment - Remove metadata export that was causing Next.js build failure - Remove unused Metadata type import - Client components cannot export metadata in Next.js App Router - Follows pattern from original features page which doesn't export metadata Co-Authored-By: [email protected] <[email protected]> * fix: add missing human titles to features grid section for font consistency - Add larger descriptive titles with text-lg md:text-xl font-light styling - Match exact structure and font classes from original features page - Ensures consistent typography across all sections Co-Authored-By: [email protected] <[email protected]> * fix: restructure HTML semantic tags while preserving visual styling - Swap hero section styling between H1 and subtitle, reorder elements - Change benefits section H3 to H2, H2 to p tags (keep styling) - Replace FeaturesIntroSection with inline H2 + text elements - Remove features grid intro section, restructure items as H2-Text-Text - Remove unused FeaturesIntroSection import Co-Authored-By: [email protected] <[email protected]> * fix: adjust HTML structure in features intro and cards sections - Features intro: give 'Complete Rapid Prototyping Solution' small caps styling (keep h2) - Features intro: give 'All the Features you need to Build and Scale' larger h2 styling - Feature cards: convert all h2s to text, convert small caps text to h2s - Feature cards: move new h2s (formerly small caps) to first line of each card - Maintain exact visual styling while fixing semantic HTML structure Co-Authored-By: [email protected] <[email protected]> * fix: change FAQ grid heading from h2 to h3 while maintaining styling - Change 'Frequently asked questions' from h2 to h3 tag - Preserve exact visual styling with same CSS classes - Maintain semantic HTML structure consistency Co-Authored-By: [email protected] <[email protected]> --------- Co-authored-by: [email protected] <[email protected]>
…2743) * feat: add AI features page at /features/ai with AI-focused content - Create new /features/ai page cloning existing /features structure - Add AI-focused hero component with updated messaging - Add AI benefits section with 3 AI-specific benefits - Add AI features grid with 6 AI design tool features - Add AI features intro section - Update metadata with AI-focused SEO content - All components follow existing patterns and styling Co-Authored-By: [email protected] <[email protected]> * fix: remove metadata export from client component in AI features page Co-Authored-By: [email protected] <[email protected]> * fix: restructure HTML semantic tags while preserving visual styling - Swap hero section styling between H1 and subtitle, reorder elements - Change benefits section H3 to H2, H2 to p tags (keep styling) - Update features intro section H3 to H2, H2 to text elements - Restructure features grid items as H2-Text-Text structure - Maintain all visual styling and animations Co-Authored-By: [email protected] <[email protected]> * fix: change FAQ section heading from H2 to H3 while preserving styling - Update FAQ section title to use H3 tag instead of H2 - Maintain exact visual styling with same CSS classes - Complete HTML semantic tag restructuring for AI features page Co-Authored-By: [email protected] <[email protected]> * fix: correct JSX closing tag mismatch in FAQ section - Fix H3 opening tag with H2 closing tag syntax error - Ensure proper JSX tag matching for FAQ section title - Resolve Vercel deployment failure caused by syntax error Co-Authored-By: [email protected] <[email protected]> * fix: invert styling between H2 and p elements in AI features grid section Co-Authored-By: [email protected] <[email protected]> --------- Co-authored-by: [email protected] <[email protected]>
* fix: apply HTML semantic tag fixes to original features page - Hero: swap H1/H3 tags and styling, reorder elements - Benefits: change H3 to H2 for titles, H2 to p for descriptions - Features intro: update H3 to H2, split description text - Features grid: change H3 to H2, H2 to text, reorder to H2-Text-Text structure - FAQ: change H2 to H3 for main title Maintains exact visual styling while fixing semantic HTML structure Co-Authored-By: [email protected] <[email protected]> * fix: correct HTML semantic tags in features grid and FAQ sections - Features grid: change H2 back to H3 for feature titles - FAQ section: change H3 back to H2 for main title Addresses user feedback on semantic tag hierarchy Co-Authored-By: [email protected] <[email protected]> --------- Co-authored-by: [email protected] <[email protected]>
…2745) * fix: apply HTML semantic tag fixes to original features page - Hero: swap H1/H3 tags and styling, reorder elements - Benefits: change H3 to H2 for titles, H2 to p for descriptions - Features intro: update H3 to H2, split description text - Features grid: change H3 to H2, H2 to text, reorder to H2-Text-Text structure - FAQ: change H2 to H3 for main title Maintains exact visual styling while fixing semantic HTML structure Co-Authored-By: [email protected] <[email protected]> * fix: correct HTML semantic tags in features grid and FAQ sections - Features grid: change H2 back to H3 for feature titles - FAQ section: change H3 back to H2 for main title Addresses user feedback on semantic tag hierarchy Co-Authored-By: [email protected] <[email protected]> * feat: add /features/builder page with React visual builder content Co-Authored-By: [email protected] <[email protected]> --------- Co-authored-by: [email protected] <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
CLAUDE.md (1)
2-4: Make the bullets consistent, imperative, and grammatically tight.Shift to imperative mood and expand “local dev” for clarity. This mirrors prior feedback on similar wording.
-- Unit tests can be run with `bun test` - - Run type checking with `bun run typecheck` -- Apply database updates to local dev with `bun run db:push` +- Run unit tests with `bun test` +- Run type checks with `bun run typecheck` +- Apply database updates to the local development environment with `bun run db:push`
🧹 Nitpick comments (1)
CLAUDE.md (1)
1-1: AI summary is inconsistent with file contents; keep and tighten the Bun policy line.The AI summary claims this policy line was removed, but it still exists in the file. Also, prefer clearer wording and correct capitalization for product names.
Apply this diff to improve clarity:
-- This is a Bun workspace, only use bun, not npm +- Use Bun for all workspace tasks; do not use npm.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
CLAUDE.md(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-13T17:15:52.910Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-13T17:15:52.910Z
Learning: Use Bun for all workspace tasks; do not use npm
Applied to files:
CLAUDE.md
📚 Learning: 2025-08-13T17:15:52.910Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-13T17:15:52.910Z
Learning: Run type checking with: `cd apps/web/client && bun run typecheck`
Applied to files:
CLAUDE.md
📚 Learning: 2025-08-13T17:15:52.910Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-13T17:15:52.910Z
Learning: Run unit tests using `bun test`
Applied to files:
CLAUDE.md
📚 Learning: 2025-08-15T04:00:01.345Z
Learnt from: Raj-G07
PR: onlook-dev/onlook#2680
File: apps/web/preload/Dockerfile:20-20
Timestamp: 2025-08-15T04:00:01.345Z
Learning: When migrating Bun commands from docker-compose.yml to Dockerfile CMD, both "bun run server/index.ts" and "bun server/index.ts" work for direct file execution, but the latter is more explicit and efficient when no package.json scripts are involved.
Applied to files:
CLAUDE.md
🪛 LanguageTool
CLAUDE.md
[grammar] ~2-~2: There might be a mistake here.
Context: ...n, not npm - Unit tests can be run with bun test - Run type checking with `bun run typechec...
(QB_NEW_EN)
[grammar] ~3-~3: There might be a mistake here.
Context: ...ith bun test - Run type checking with bun run typecheck - Apply database updates to local dev with...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
🔇 Additional comments (1)
CLAUDE.md (1)
3-3: Resolved: Root ‘typecheck’ script is defined
The repository’s top-level package.json includes atypecheckscript that runscd apps/web/client && bun run typecheck, so invokingbun run typecheckfrom the project root correctly performs the intended client-side type check. No updates to CLAUDE.md are needed.
apps/web/client/src/app/project/[id]/_components/left-panel/help-dropdown/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/components/ui/pricing-modal/free-card.tsx (1)
53-56: Open new windows safelyUse
noopener,noreferrerto prevent reverse tabnabbing when opening external checkout.- window.open(session.url, '_blank'); + window.open(session.url, '_blank', 'noopener,noreferrer');
🧹 Nitpick comments (46)
apps/web/client/src/app/_components/landing-page/cta-section.tsx (1)
49-49: Responsive heading size change looks goodSwitching to text-5xl md:text-6xl improves readability on small screens without affecting layout.
Minor micro-optimization: avoid splitting ctaText multiple times within render.
Example refactor (outside the selected lines):
const lines = ctaText.split('\n'); ... {lines.map((line, index) => ( <span key={index}> {line} {index < lines.length - 1 && <br />} </span> ))}apps/web/client/src/app/_components/landing-page/page-footer.tsx (1)
38-38: Use Routes.PRICING and Next.js Link for internal navigationKeeps route definitions DRY and enables client-side navigation without full reload.
Apply this diff to the changed line:
- <li><a href="/pricing" className="hover:underline" title="View Onlook pricing">Pricing</a></li> + <li><Link href={Routes.PRICING} className="hover:underline" title="View Onlook pricing">Pricing</Link></li>Add the missing import at the top of this file (outside the selected line):
import Link from 'next/link';apps/web/client/src/app/_components/landing-page/features-grid-section.tsx (1)
9-9: Better heading hierarchy and readabilityDemoting the secondary line from a heading to a paragraph under each h3 improves semantic structure and scanability. Typography remains consistent with text-lg/md:text-xl.
To reduce repetition, consider extracting a small component (e.g., FeatureTagline) or a class utility for "text-foreground-primary text-lg md:text-xl font-light mb-6 text-balance" used in all six places.
Example:
function FeatureTagline({ children }: { children: React.ReactNode }) { return <p className="text-foreground-primary text-lg md:text-xl font-light mb-6 text-balance">{children}</p>; }Then replace the six occurrences with:
<FeatureTagline>Unified components for design and code</FeatureTagline>Also applies to: 17-17, 25-25, 33-33, 41-41, 49-49
docs/src/components/rb2b-loader.tsx (3)
13-21: Avoid re-injecting the RB2B script on every route changeCurrently, the effect removes and re-adds the script on each pathname change, which can cause redundant network work and side effects in the third‑party SDK. Inject once per ID; only replace if the ID changes.
Apply this diff to dedupe by ID and persist a data attribute:
- const existing = document.getElementById('rb2b-script'); - if (existing) existing.remove(); - - const script = document.createElement('script'); - script.id = 'rb2b-script'; - script.src = `https://ddwl4m2hdecbv.cloudfront.net/b/${rb2bId}/${rb2bId}.js.gz`; - script.async = true; - document.body.appendChild(script); + const existing = document.getElementById('rb2b-script'); + if (existing && existing.getAttribute('data-rb2b-id') === rb2bId) { + // Already loaded for this ID — nothing to do. + return; + } + if (existing) existing.remove(); + + const script = document.createElement('script'); + script.id = 'rb2b-script'; + script.setAttribute('data-rb2b-id', rb2bId); + script.src = `https://ddwl4m2hdecbv.cloudfront.net/b/${rb2bId}/${rb2bId}.js.gz`; + script.async = true; + // Optionally: script.crossOrigin = 'anonymous'; + document.body.appendChild(script);
10-11: Align env access with the client env helper used in apps/webThe web client’s RB2B loader uses a typed client env (env.ts). For consistency and to catch typos at build time, consider mirroring that here instead of raw process.env access.
Example:
- const rb2bId = process.env.NEXT_PUBLIC_RB2B_ID; + // import { env } from '@/env' (or docs-specific env helper) + const rb2bId = env.NEXT_PUBLIC_RB2B_ID;If a docs-side env helper doesn’t exist, feel free to keep process.env for now.
16-21: CSP nonce support (optional refactor)Next.js App Router does not expose a built-in client hook (e.g.
useCSPNonce) for retrieving a per-request CSP nonce. The recommended pattern is to generate the nonce on the server, emit it into the HTML (via a<meta name="csp-nonce" …/>or a global variable), then read and assign it to any dynamically created<script>elements in your client components.Locations to update:
docs/src/components/rb2b-loader.tsx(inside the effect that appends the script)Suggested diff:
useEffect(() => { - const script = document.createElement('script'); + const script = document.createElement('script'); script.id = 'rb2b-script'; script.src = `https://ddwl4m2hdecbv.cloudfront.net/b/${rb2bId}/${rb2bId}.js.gz`; - script.async = true; + script.async = true; + // Attach CSP nonce if one was server-rendered into <meta name="csp-nonce"> + const nonce = document + .querySelector('meta[name="csp-nonce"]') + ?.getAttribute('content'); + if (nonce) { + // TS may not know about the nonce property on HTMLScriptElement + (script as unknown as { nonce: string }).nonce = nonce; + } document.body.appendChild(script); }, [pathname]);This change follows Next.js Security/CSP guidance:
- Generate a per-request nonce on the server.
- Emit it into your root HTML via a meta tag (or
window.__CSP_NONCE__).- In client code, read that value and assign it to
script.nonce.No built-in
useCSPNoncehook exists in Next.js App Router, so this approach is the officially recommended way.docs/src/app/layout.tsx (1)
52-57: Gate the RB2B loader on the env var to avoid adding a client boundary unnecessarilyOnly render the loader when an ID is configured. This saves a client component boundary in prod builds where RB2B is disabled.
- {isProduction && ( + {isProduction && process.env.NEXT_PUBLIC_RB2B_ID && ( <> <Script src="https://z.onlook.com/cdn-cgi/zaraz/i.js" strategy="lazyOnload" /> <RB2BLoader /> </> )}apps/web/client/src/app/_components/landing-page/ai-features-intro-section.tsx (1)
5-17: Optional: wrap in a semantic section and expose an aria-labelledbyThis improves document outline and navigation without changing visuals.
- <div className="w-full max-w-6xl mx-auto py-32 px-8 text-center"> + <section className="w-full max-w-6xl mx-auto py-32 px-8 text-center" aria-labelledby="ai-intro-title"> <div className="max-w-3xl mx-auto"> - <h2 className="text-foreground-secondary text-sm font-medium uppercase tracking-wider mb-6"> + <h2 className="text-foreground-secondary text-sm font-medium uppercase tracking-wider mb-6"> AI Design Tools with Visual Control </h2> - <p className="text-foreground-primary text-2xl md:text-5xl leading-[1.1] font-light mb-8 text-balance"> + <p id="ai-intro-title" className="text-foreground-primary text-2xl md:text-5xl leading-[1.1] font-light mb-8 text-balance"> Visual Design Control Supercharged with AI </p> <p className="text-foreground-secondary text-lg max-w-xl mx-auto text-balance"> Get the precision of visual editing with the speed of AI generation. Design with complete creative control while AI handles the heavy lifting - from maintaining brand consistency to generating responsive layouts that match your exact vision. </p> </div> - </div> + </section>apps/web/client/src/app/project/[id]/_components/left-panel/help-dropdown/index.tsx (1)
139-147: Use i18n for “Send Feedback” to stay consistent with the rest of the menuAll other items use next-intl via t(transKeys...). Localizing this label keeps UX consistent.
Apply this minimal change:
- <DropdownMenuItem + <DropdownMenuItem className="text-sm" onClick={() => { setIsDropdownOpen(false); stateManager.isFeedbackModalOpen = true; }} > <Icons.MessageSquare className="w-4 h-4 mr-2" /> - Send Feedback + {t(transKeys.help.menu.sendFeedback)} </DropdownMenuItem>If transKeys.help.menu.sendFeedback doesn’t exist yet, I can add it (and default translations) in a follow-up.
apps/web/client/src/app/_components/hero/features-hero.tsx (2)
40-48: Prefer a paragraph for the descriptive line rather than an h2The “Code as you design…” line is body text, not a section heading. Using a heading may confuse screen-reader users and bloats the outline.
- <motion.h2 + <motion.p className="text-lg text-foreground-secondary mx-auto max-w-xl text-center" initial={{ opacity: 0, filter: "blur(4px)" }} animate={{ opacity: 1, filter: "blur(0px)" }} transition={{ duration: 0.6, delay: 0.15, ease: "easeOut" }} style={{ willChange: "opacity, filter", transform: "translateZ(0)" }} - > + > Code as you design. Build React applications visually while Onlook writes reliable code you can trust, exactly where it needs to go. - </motion.h2> + </motion.p>
32-33: Nit: remove redundant leading classesYou’re setting both leading-tight and !leading-[1]; the latter already overrides the former. Keep only one.
- className="text-4xl md:text-6xl font-light leading-tight text-center !leading-[1] text-balance" + className="text-4xl md:text-6xl font-light text-center !leading-[1] text-balance"apps/web/client/src/app/_components/landing-page/features-intro-section.tsx (1)
10-12: Promote the subheading back to a semantic heading (or add ARIA).“All the Features you need to Build and Scale” reads like a section heading but is rendered as a paragraph. Prefer an h3 (or h2 depending on page outline), or keep
with role="heading" and an appropriate aria-level to preserve accessibility and document outline.
Option A — switch to h3:
- <p className="text-foreground-primary text-2xl md:text-5xl leading-[1.1] font-light mb-8 text-balance"> + <h3 className="text-foreground-primary text-2xl md:text-5xl leading-[1.1] font-light mb-8 text-balance"> All the Features you need to Build and Scale - </p> + </h3>Option B — keep
but add ARIA:
- <p className="text-foreground-primary text-2xl md:text-5xl leading-[1.1] font-light mb-8 text-balance"> + <p role="heading" aria-level={3} className="text-foreground-primary text-2xl md:text-5xl leading-[1.1] font-light mb-8 text-balance"> All the Features you need to Build and Scale </p>apps/web/client/.env.example (1)
57-60: Remove quotes and avoid hard-coded onlook.com emails in example env.dotenv-linter flags the quotes; also, shipping real-looking addresses in an example can mislead users and cause failed email sending without domain verification. Use placeholders and drop quotes.
-# Feedback - Email notifications for user feedback -FEEDBACK_FROM_EMAIL="[email protected]" -FEEDBACK_TO_EMAIL="[email protected]" +# Feedback - Email notifications for user feedback +# Use a verified sender and a monitored inbox (must be verified with your email provider) +FEEDBACK_FROM_EMAIL=<[email protected]> +FEEDBACK_TO_EMAIL=<[email protected]>apps/web/client/src/app/_components/landing-page/ai-features-grid-section.tsx (1)
8-53: Use appropriate heading levels for card titles and trim duplicate copy.Each card uses h2; depending on page outline, h3/h4 may be more correct. Also, the lead line and body copy repeat the same sentence in a couple of cards—tightening the lead makes the section scan better.
Example for one card:
- <h2 className="text-foreground-secondary text-small uppercase tracking-wider mb-4">Instant Visual Feedback</h2> - <p className="text-foreground-primary text-lg md:text-xl font-light mb-6 text-balance">See AI-generated components appear in real-time</p> - <p className="text-foreground-secondary text-regular text-balance leading-relaxed"> - See AI-generated components appear in real-time as you describe them, with immediate visual updates for every change you make - </p> + <h3 className="text-foreground-secondary text-small uppercase tracking-wider mb-4">Instant Visual Feedback</h3> + <p className="text-foreground-primary text-lg md:text-xl font-light mb-6 text-balance">Real-time component updates as you describe changes</p> + <p className="text-foreground-secondary text-regular text-balance leading-relaxed"> + Watch components generate and update instantly with every instruction—no rebuilds or context switching. + </p>If keeping h2 for visual style, add role="heading" aria-level={3} for semantic correctness.
apps/web/client/src/app/_components/hero/ai-features-hero.tsx (1)
19-21: Ensure full-viewport hero height.The container uses h-full, which depends on parent height. Use min-h-screen to guarantee the hero fills the viewport.
- <div className="w-full h-full flex flex-col items-center justify-center gap-12 p-8 text-lg text-center relative"> + <div className="w-full min-h-screen flex flex-col items-center justify-center gap-12 p-8 text-lg text-center relative">apps/web/client/src/components/ui/pricing-modal/pro-card.tsx (2)
161-164: i18n: hardcoded CTA"Get Started with Pro" bypasses next-intl. Please localize for consistency.
- if (isUnauthenticated) { - return "Get Started with Pro"; - } + if (isUnauthenticated) { + return t('pricing.cta.getStartedPro'); // add key to your locale files + }
100-110: i18n key usage is inconsistent with earlier toastsElsewhere you use t(transKeys.pricing.toasts.error.title). Here you use a string key. Standardize for consistency.
- toast.error(t('pricing.toasts.error.title'), { + toast.error(t(transKeys.pricing.toasts.error.title), { description: error instanceof Error ? error.message : 'Unknown error', });apps/web/client/src/app/projects/page.tsx (1)
3-3: Multiple FeedbackModal instances detected; consider hoisting to RootLayoutFeedbackModal is currently imported and rendered in six different locations, leading to duplicate mounts across your pages:
• apps/web/client/src/app/page.tsx
• apps/web/client/src/app/projects/page.tsx
• apps/web/client/src/app/project/[id]/_components/main.tsx
• apps/web/client/src/app/features/page.tsx
• apps/web/client/src/app/features/prototype/page.tsx
• apps/web/client/src/app/features/ai/page.tsxRather than importing and rendering the modal in each page component, you can move a single
<FeedbackModal />into your RootLayout (e.g. apps/web/client/src/app/layout.tsx). This will:
- Ensure only one instance of the modal is ever mounted
- Simplify page components by removing repetitive imports and JSX
- Centralize modal state and handlers in one place
Optional refactor example (in layout.tsx):
--- a/apps/web/client/src/app/layout.tsx +++ b/apps/web/client/src/app/layout.tsx @@ return ( <html lang="en"> <body> + {/* Global feedback modal */} + <FeedbackModal /> {children} </body> </html> ) }Please review this refactor to streamline your code and avoid duplicate mounts.
apps/web/client/src/app/layout.tsx (2)
17-17: Render RB2BLoader independent of NODE_ENV; gate on env var insteadYou already no-op when NEXT_PUBLIC_RB2B_ID is unset inside RB2BLoader. Keeping the loader behind isProduction blocks dev/preview testing. Suggest: always render RB2BLoader; keep Zaraz gated by production.
- {isProduction && ( - <> - <Script src="https://z.onlook.com/cdn-cgi/zaraz/i.js" strategy="lazyOnload" /> - <RB2BLoader /> - </> - )} + <> + {isProduction && ( + <Script src="https://z.onlook.com/cdn-cgi/zaraz/i.js" strategy="lazyOnload" /> + )} + <RB2BLoader /> + </>Also applies to: 76-79
76-79: RB2BLoader is already a client component; consider injecting its script only onceVerified that
apps/web/client/src/components/rb2b-loader.tsxbegins with"use client"and usesuseEffect+usePathnameto remove and re-append the script on every route change. Unless the RB2B snippet must reload per-page, this is extra work and can be simplified.• File to update:
apps/web/client/src/components/rb2b-loader.tsx• Current behavior:
- Imports and calls
usePathname()useEffect(..., [pathname])removes any existing<script id="rb2b-script">and appends a new one on every navigation• Optional refactor: only inject once
- Drop
usePathname(remove the import and its call)- Change the effect to an empty dependency array (
[])- Skip reinjection if the script already exists
-'use client'; +’use client’; -import { usePathname } from 'next/navigation'; import { useEffect } from 'react'; import { env } from '@/env'; export default function RB2BLoader() { - const pathname = usePathname(); - - useEffect(() => { - if (!env.NEXT_PUBLIC_RB2B_ID) return; - - const existing = document.getElementById('rb2b-script'); - if (existing) existing.remove(); - - const script = document.createElement('script'); - script.id = 'rb2b-script'; - script.src = `https://ddwl4m2hdecbv.cloudfront.net/b/${env.NEXT_PUBLIC_RB2B_ID}/${env.NEXT_PUBLIC_RB2B_ID}.js.gz`; - script.async = true; - document.body.appendChild(script); - }, [pathname]); + useEffect(() => { + if (!env.NEXT_PUBLIC_RB2B_ID) return; + if (document.getElementById('rb2b-script')) return; + + const script = document.createElement('script'); + script.id = 'rb2b-script'; + script.src = `https://ddwl4m2hdecbv.cloudfront.net/b/${env.NEXT_PUBLIC_RB2B_ID}/${env.NEXT_PUBLIC_RB2B_ID}.js.gz`; + script.async = true; + document.body.appendChild(script); + }, []); return null; }apps/web/client/src/app/_components/landing-page/responsive-mockup-section.tsx (1)
15-16: Mobile spacing/typography tweaks look reasonable; check heading hierarchy and CLSThe reduced padding and smaller h2 sizes improve compactness. Ensure heading levels align with page hierarchy and that absolute-positioned mockups plus mt-[700px] don’t cause layout shifts on slow devices.
Also applies to: 23-24, 33-34, 41-42
apps/web/client/src/app/features/page.tsx (2)
16-41: Hardcoded FAQs bypass translationsThese strings won’t be localized via next-intl. Prefer pulling from translations or at least centralizing content for reuse.
Example: source from i18n keys (pseudo-code):
const featuresFaqs = [ { question: t('features.faq.q1'), answer: t('features.faq.a1') }, // ... ];If you want, I can generate a draft locale JSON and wire-up keys.
61-62: Hoist FeedbackModal to RootLayoutYou currently have
<FeedbackModal />mounted in six separate pages, which leads to duplication and potential divergence in behavior:
- apps/web/client/src/app/page.tsx (line 35)
- apps/web/client/src/app/projects/page.tsx (line 20)
- apps/web/client/src/app/project/[id]/_components/main.tsx (line 137)
- apps/web/client/src/app/features/page.tsx (line 61)
- apps/web/client/src/app/features/prototype/page.tsx (line 281)
- apps/web/client/src/app/features/ai/page.tsx (line 61)
Refactor steps:
- Import and render
FeedbackModalonce in your RootLayout (e.g.apps/web/client/src/app/layout.tsx), ideally right inside<html>or directly under your top-level layout wrapper.- Remove all individual
<FeedbackModal />mounts from the pages listed above.This change ensures the modal is mounted a single time, simplifies its visibility control via your global
stateManager, and eliminates redundant imports and JSX across multiple files.apps/web/client/src/components/ui/avatar-dropdown/index.tsx (1)
61-66: Nice UX: switch to in-app “Send Feedback” modal and close popover on clickClean integration with the global state flag; closing the popover avoids focus traps and feels polished. No functional concerns here.
If you track product analytics, consider emitting an event when opening the feedback modal to measure adoption.
apps/web/client/src/components/rb2b-loader.tsx (2)
10-21: Avoid reloading the RB2B script on every route change unless vendor requires itRe-adding the script on each pathname change can waste bandwidth, reset vendor state, and introduce race conditions. Load once on mount; if pageview tracking is needed for SPA navigations, prefer calling the vendor’s pageview API instead of re-inserting the script.
Apply this diff to load once and add basic hardening:
- useEffect(() => { - if (!env.NEXT_PUBLIC_RB2B_ID) return; - - const existing = document.getElementById('rb2b-script'); - if (existing) existing.remove(); - - const script = document.createElement('script'); - script.id = 'rb2b-script'; - script.src = `https://ddwl4m2hdecbv.cloudfront.net/b/${env.NEXT_PUBLIC_RB2B_ID}/${env.NEXT_PUBLIC_RB2B_ID}.js.gz`; - script.async = true; - document.body.appendChild(script); - }, [pathname]); + useEffect(() => { + if (!env.NEXT_PUBLIC_RB2B_ID) return; + if (document.getElementById('rb2b-script')) return; // load once + + const script = document.createElement('script'); + script.id = 'rb2b-script'; + script.src = `https://ddwl4m2hdecbv.cloudfront.net/b/${env.NEXT_PUBLIC_RB2B_ID}/${env.NEXT_PUBLIC_RB2B_ID}.js.gz`; + script.async = true; + script.referrerPolicy = 'no-referrer'; + script.onerror = () => console.warn('RB2B script failed to load'); + document.body.appendChild(script); + }, []); // mount only
18-20: CSP and integrity considerations for third-party scriptsIf you enforce a CSP with nonces, consider accepting a nonce via props/context and applying it to the script element. Subresource Integrity (SRI) is unlikely feasible here due to dynamic paths, but a nonce plus restrictive
script-srcis a good baseline.Would you like me to propose a pattern for injecting CSP nonces into client-only loaders (compatible with Next.js)?
apps/web/client/src/app/page.tsx (1)
4-4: Lazy-load FeedbackModal to trim the landing page’s initial JSThe modal pulls in compression, upload, and persistence utilities. Prefer dynamic import with
ssr: falseso the landing page doesn’t ship that code until the modal opens.Apply this diff to convert to a dynamic import:
-import { FeedbackModal } from '@/components/ui/feedback-modal'; +import dynamic from 'next/dynamic'; +const FeedbackModal = dynamic( + () => import('@/components/ui/feedback-modal').then(m => m.FeedbackModal), + { ssr: false } +);apps/web/client/src/app/features/ai/page.tsx (2)
16-41: Externalize FAQ strings for consistency and i18nHard-coded copy here diverges from other pages using shared FAQ content and translation keys. Move these strings to your i18n layer or a shared content module to keep marketing pages consistent and localizable.
4-4: Defer FeedbackModal via dynamic importSame reasoning as the homepage: reduce initial payload for this marketing page.
Apply this diff:
-import { FeedbackModal } from '@/components/ui/feedback-modal'; +import dynamic from 'next/dynamic'; +const FeedbackModal = dynamic( + () => import('@/components/ui/feedback-modal').then(m => m.FeedbackModal), + { ssr: false } +);apps/web/client/src/app/features/builder/page.tsx (3)
193-204: Use WebsiteLayout (and global modals) for parity with other feature pagesThis page omits the shared layout/top bar/footer and the global modals, making UX inconsistent with pages like features/ai. Wrap content with WebsiteLayout and include FeedbackModal/Settings/Subscription modals.
Apply this diff to the render tree:
-export default function BuilderFeaturesPage() { - return ( - <div className="min-h-screen bg-background-primary"> +export default function BuilderFeaturesPage() { + return ( + <div className="min-h-screen bg-background"> <BuilderFeaturesHero /> <BuilderBenefitsSection /> <BuilderFeaturesIntroSection /> <BuilderFeaturesGridSection /> <BuilderCTASection /> <BuilderFAQSection /> - </div> + </div> ); }And add these imports and usage (outside the selected range):
// imports to add at top import { WebsiteLayout } from '../../_components/website-layout'; import { NonProjectSettingsModal } from '@/components/ui/settings-modal/non-project'; import { SubscriptionModal } from '@/components/ui/pricing-modal'; import dynamic from 'next/dynamic'; // after imports const FeedbackModal = dynamic( () => import('@/components/ui/feedback-modal').then(m => m.FeedbackModal), { ssr: false } ); // then wrap the page content: return ( <WebsiteLayout showFooter> {/* existing sections */} <NonProjectSettingsModal /> <SubscriptionModal /> <FeedbackModal /> </WebsiteLayout> );
19-22: CTA buttons lack behavior; prefer the shared CTASection for consistent routing/UXThese plain buttons don’t navigate anywhere. Reuse the shared CTASection (like other pages) or wire onClick to a route/anchor.
Example replacement for the bottom CTA:
import { CTASection } from '../../_components/landing-page/cta-section'; // ... <CTASection ctaText="Start Building React Apps Visually Today" buttonText="Get Started for Free" href="/" />Also applies to: 133-136
142-191: Deduplicate FAQ UI by reusing the shared FAQSectionThis hand-rolled FAQ markup duplicates styling/behavior already provided by FAQSection and makes future updates harder.
Apply this diff to simplify the section:
-function BuilderFAQSection() { - const builderFAQs = [ - { question: "What is Onlook?", answer: "Onlook is an open-source, visual editor for websites. It allows anyone to create and style their own websites without any coding knowledge." }, - { question: "What can I use Onlook to do?", answer: "Onlook is great for creating websites, prototypes, user interfaces, and designs. Whether you need a quick mockup or a full-fledged website, ask Onlook to craft it for you." }, - { question: "How do I get started?", answer: "Getting started with Onlook is easy. Simply sign up for an account, create a new project, and follow our step-by-step guide to deploy your first application." }, - { question: "Is Onlook free to use?", answer: "Onlook is free for your first prompt, but you're limited by the number of messages you can send. Please see our Pricing page for more details." }, - { question: "What is the difference between Onlook and other design tools?", answer: "Onlook is a visual editor for code. It allows you to create and style your own creations with code as the source of truth. While it is best suited for creating websites, it can be used for anything visual – presentations, mockups, and more. Because Onlook uses code as the source of truth, the types of designs you can create are unconstrained by Onlook's interface." }, - { question: "Why is Onlook open-source?", answer: "Developers have historically been second-rate citizens in the design process. Onlook was founded to bridge the divide between design and development, and we wanted to make developers first-class citizens alongside designers. We chose to be open-source to give developers transparency into how we are building Onlook and how the work created through Onlook will complement the work of developers." } - ]; - - return ( - <div className="w-full max-w-4xl mx-auto py-32 px-8"> - <div className="text-center mb-16"> - <h2 className="text-foreground-primary text-3xl md:text-5xl leading-[1.1] font-light mb-8 text-balance"> - Frequently Asked Questions - </h2> - </div> - <div className="space-y-4"> - {builderFAQs.map((faq, index) => ( - <div key={index} className="border border-border-primary rounded-lg p-6"> - <h3 className="text-foreground-primary text-lg font-medium mb-3"> - {faq.question} - </h3> - <p className="text-foreground-secondary text-regular leading-relaxed"> - {faq.answer} - </p> - </div> - ))} - </div> - </div> - ); -} +import { FAQSection } from '../../_components/landing-page/faq-section'; +function BuilderFAQSection() { + const builderFAQs = [ + { question: "What is Onlook?", answer: "Onlook is an open-source, visual editor for websites. It allows anyone to create and style their own websites without any coding knowledge." }, + { question: "What can I use Onlook to do?", answer: "Onlook is great for creating websites, prototypes, user interfaces, and designs. Whether you need a quick mockup or a full-fledged website, ask Onlook to craft it for you." }, + { question: "How do I get started?", answer: "Getting started with Onlook is easy. Simply sign up for an account, create a new project, and follow our step-by-step guide to deploy your first application." }, + { question: "Is Onlook free to use?", answer: "Onlook is free for your first prompt, but you're limited by the number of messages you can send. Please see our Pricing page for more details." }, + { question: "What is the difference between Onlook and other design tools?", answer: "Onlook is a visual editor for code..." }, + { question: "Why is Onlook open-source?", answer: "Developers have historically been second-rate citizens..." } + ]; + return <FAQSection faqs={builderFAQs} className="!py-32" />; +}apps/web/client/src/components/ui/pricing-table/index.tsx (1)
13-15: Prevent auth “flash” from treating loading state as unauthenticated
isUnauthenticated={!user}evaluates to true while the user query is still loading, which can surface signup CTAs and allow clicks that open the auth modal prematurely. Derive unauthenticated from the query status instead and provide a stable onSignupClick callback.Apply:
import { api } from '@/trpc/react'; import { useTranslations } from 'next-intl'; +import { useCallback } from 'react'; @@ export const PricingTable = () => { const t = useTranslations(); - const { data: user } = api.user.get.useQuery(); - const { setIsAuthModalOpen } = useAuthContext(); + const { data: user, status: userStatus } = api.user.get.useQuery(); + const { setIsAuthModalOpen } = useAuthContext(); + const isUnauthenticated = userStatus === 'success' && !user; + const handleSignupClick = useCallback(() => setIsAuthModalOpen(true), [setIsAuthModalOpen]); @@ - <FreeCard - delay={0.1} - isUnauthenticated={!user} - onSignupClick={() => setIsAuthModalOpen(true)} - /> + <FreeCard + delay={0.1} + isUnauthenticated={isUnauthenticated} + onSignupClick={handleSignupClick} + /> <ProCard - delay={0.2} - isUnauthenticated={!user} - onSignupClick={() => setIsAuthModalOpen(true)} + delay={0.2} + isUnauthenticated={isUnauthenticated} + onSignupClick={handleSignupClick} />Also applies to: 19-31
apps/web/client/src/app/_components/landing-page/ai-benefits-section.tsx (2)
6-8: Code-split heavy mockups to reduce initial bundle and hydrate costThese interactive mockups are sizable. Load them client-only and on demand to keep the landing page snappy.
-import { AiChatInteractive } from '../shared/mockups/ai-chat-interactive'; -import { DirectEditingInteractive } from '../shared/mockups/direct-editing-interactive'; -import { TailwindColorEditorMockup } from '../shared/mockups/tailwind-color-editor'; +import dynamic from 'next/dynamic'; +const AiChatInteractive = dynamic( + () => import('../shared/mockups/ai-chat-interactive').then(m => ({ default: m.AiChatInteractive })), + { ssr: false } +); +const DirectEditingInteractive = dynamic( + () => import('../shared/mockups/direct-editing-interactive').then(m => ({ default: m.DirectEditingInteractive })), + { ssr: false } +); +const TailwindColorEditorMockup = dynamic( + () => import('../shared/mockups/tailwind-color-editor').then(m => ({ default: m.TailwindColorEditorMockup })), + { ssr: false } +);
16-21: Consider i18n for headings and copyHeadings and descriptions are hard-coded in English. If this page participates in next-intl, migrating these strings to translations will keep UX consistent.
Happy to generate a quick pass extracting these into
transKeys.features.ai.*and wiringuseTranslations().Also applies to: 28-36, 42-56
apps/web/client/src/components/ui/pricing-modal/free-card.tsx (3)
118-126: Tailwind z-index utility likely invalid
z-99isn’t a default Tailwind class. Use an arbitrary valuez-[99]or a stock token likez-50.- <SelectContent className="z-99"> + <SelectContent className="z-[99]">If you’ve extended the theme with
z-99, ignore this.
135-141: Button UX: guard unauthenticated flows and add accessibility hints
- If
isUnauthenticatedis true butonSignupClickis not provided, falling back to downgrade will likely fail. Disable in that case.- Expose
aria-busyduring checkout.- <Button + <Button className="w-full" variant="outline" - onClick={handleButtonClick} - disabled={isCheckingOut || (!isUnauthenticated && (isFree || isScheduledCancellation))} + onClick={handleButtonClick} + aria-busy={isCheckingOut} + disabled={ + isCheckingOut || + (isUnauthenticated && !onSignupClick) || + (!isUnauthenticated && (isFree || isScheduledCancellation)) + } >- const handleButtonClick = () => { - if (isUnauthenticated && onSignupClick) { + const handleButtonClick = () => { + if (isUnauthenticated) { + if (onSignupClick) { onSignupClick(); - } else { + } + return; + } else { handleDowngradeToFree(); } };Also applies to: 94-100
46-47: Optional: precompute scheduled date string onceMinor readability improvement: compute a formatted string once rather than in render branches.
const isScheduledCancellation = subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION; +const scheduledChangeDateStr = subscription?.scheduledChange?.scheduledChangeAt + ? new Date(subscription.scheduledChange.scheduledChangeAt as unknown as string | number | Date) + .toLocaleDateString(undefined, { month: 'long', day: 'numeric', year: 'numeric' }) + : null;Then use
scheduledChangeDateStrinsidebuttonContent().apps/web/client/src/components/ui/pricing-modal/enterprise-card.tsx (2)
29-41: Avoid hardcoding contact email; allow configurationHardcoding
[email protected]couples code to a personal address. Prefer a public contact (sales/support) or an env-backed value. Next.js can inlineNEXT_PUBLIC_*at build time.export const EnterpriseCard = ({ delay, }: { delay: number; }) => { - const t = useTranslations(); + const t = useTranslations(); @@ - const handleContactUs = () => { + const handleContactUs = () => { + const contact = process.env.NEXT_PUBLIC_ENTERPRISE_CONTACT_EMAIL ?? '[email protected]'; const subject = encodeURIComponent('[Enterprise]: Onlook Enterprise Inquiry'); const body = encodeURIComponent(`Hi Daniel, @@ - window.location.href = `mailto:[email protected]?subject=${subject}&body=${body}`; + window.location.href = `mailto:${contact}?subject=${subject}&body=${body}`; };Follow-up: add
NEXT_PUBLIC_ENTERPRISE_CONTACT_EMAILto env validation and.env.example.
59-64: Optional: translate button labelIf the rest of pricing uses next-intl, consider translating “Contact Us”.
- <Button + <Button className="w-full" onClick={handleContactUs} > - Contact Us + {t(transKeys.pricing.buttons.contactUs)} </Button>apps/web/client/src/app/_components/landing-page/faq-section.tsx (2)
55-61: Avoid repeated string splitting in render pathYou split the title on every iteration and compare against a freshly split array. Use the map’s third arg to avoid the second split.
Apply this diff to simplify and shave some work per render:
- {title.split('\n').map((line, index) => ( + {title.split('\n').map((line, index, arr) => ( <React.Fragment key={index}> {line} - {index < title.split('\n').length - 1 && <br />} + {index < arr.length - 1 && <br />} </React.Fragment> ))}
20-41: Type the default FAQ listGive defaultFaqs an explicit type to preserve safety if this array is edited later.
-const defaultFaqs = [ +const defaultFaqs: FAQ[] = [apps/web/client/src/app/_components/landing-page/benefits-section.tsx (1)
6-9: Defer heavy interactive mockups with dynamic imports (reduce JS on first load)AiChatInteractive, DirectEditingInteractive, and TailwindColorEditorMockup are sizable. Loading them dynamically (client-only) will improve TTI on this marketing section.
import React from 'react'; import { Icons } from '@onlook/ui/icons'; -import { ButtonLink } from '../button-link'; -import { AiChatInteractive } from '../shared/mockups/ai-chat-interactive'; -import { DirectEditingInteractive } from '../shared/mockups/direct-editing-interactive'; -import { TailwindColorEditorMockup } from '../shared/mockups/tailwind-color-editor'; +import { ButtonLink } from '../button-link'; +import dynamic from 'next/dynamic'; + +const AiChatInteractive = dynamic( + () => import('../shared/mockups/ai-chat-interactive').then(m => m.AiChatInteractive), + { ssr: false } +); +const DirectEditingInteractive = dynamic( + () => import('../shared/mockups/direct-editing-interactive').then(m => m.DirectEditingInteractive), + { ssr: false } +); +const TailwindColorEditorMockup = dynamic( + () => import('../shared/mockups/tailwind-color-editor').then(m => m.TailwindColorEditorMockup), + { ssr: false } +);Optional: add a lightweight skeleton/fallback for each while loading.
apps/web/client/src/app/features/prototype/page.tsx (3)
20-23: Dynamically import heavy mockups on this page tooSame rationale as the benefits section: defer large interactive components to reduce main bundle and improve responsiveness.
-import { AiChatInteractive } from '../../_components/shared/mockups/ai-chat-interactive'; -import { DirectEditingInteractive } from '../../_components/shared/mockups/direct-editing-interactive'; -import { TailwindColorEditorMockup } from '../../_components/shared/mockups/tailwind-color-editor'; +import dynamic from 'next/dynamic'; + +const AiChatInteractive = dynamic( + () => import('../../_components/shared/mockups/ai-chat-interactive').then(m => m.AiChatInteractive), + { ssr: false } +); +const DirectEditingInteractive = dynamic( + () => import('../../_components/shared/mockups/direct-editing-interactive').then(m => m.DirectEditingInteractive), + { ssr: false } +); +const TailwindColorEditorMockup = dynamic( + () => import('../../_components/shared/mockups/tailwind-color-editor').then(m => m.TailwindColorEditorMockup), + { ssr: false } +);
34-99: Consider motion preferences for reduced-animation usersHero uses several framer-motion animations. Consider useReducedMotion to respect prefers-reduced-motion and adjust durations/opacity transitions accordingly.
Example:
import { useReducedMotion } from 'framer-motion'; function PrototypeFeaturesHero() { const shouldReduceMotion = useReducedMotion(); // then gate animation/transition props or shorten durations when true }
16-18: Refactor to reuse existing FAQSection – PrototypeFAQSection still presentThe bespoke
PrototypeFAQSectionis still defined and rendered, duplicating functionality provided byFAQSection. Please apply the following optional refactor inapps/web/client/src/app/features/prototype/page.tsx:• Locations to update:
– Imports (lines 16–18)
– Definition ofPrototypeFAQSection(lines 233–249)
– Render call (lines 278–279)
- Replace the import for
FAQDropdownwithFAQSection:- import { FAQDropdown } from '../../_components/landing-page/faq-dropdown'; + import { FAQSection } from '../../_components/landing-page/faq-section';
- Remove the entire
PrototypeFAQSectiondefinition (lines 233–249):- function PrototypeFAQSection() { - return ( - <div className="w-full py-48 px-8 bg-background-onlook/80" id="faq"> - <div className="max-w-6xl mx-auto flex flex-col md:flex-row items-start gap-24 md:gap-12"> - <div className="flex-1 flex flex-col items-start"> - <h3 className="text-foreground-primary text-5xl md:text-6xl leading-[1.1] font-light mb-12 mt-4 max-w-3xl text-balance"> - Frequently<br />asked questions - </h3> - <ButtonLink href={Routes.FAQ} rightIcon={<Icons.ArrowRight className="w-5 h-5" />}>Read our FAQs</ButtonLink> - </div> - <div className="flex-1 flex flex-col gap-6"> - <FAQDropdown faqs={prototypeFaqs} /> - </div> - </div> - </div> - ); - }
- Swap out the bespoke component with the reusable
FAQSectionat render time (lines 278–279):- <PrototypeFAQSection /> + <FAQSection + faqs={prototypeFaqs} + title={"Frequently\nasked questions"} + buttonText="Read our FAQs" + buttonHref={Routes.FAQ} + />This will eliminate duplication, ensure consistent styling, and make future maintenance easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (37)
apps/web/client/.env.example(2 hunks)apps/web/client/src/app/_components/hero/ai-features-hero.tsx(1 hunks)apps/web/client/src/app/_components/hero/features-hero.tsx(1 hunks)apps/web/client/src/app/_components/landing-page/ai-benefits-section.tsx(1 hunks)apps/web/client/src/app/_components/landing-page/ai-features-grid-section.tsx(1 hunks)apps/web/client/src/app/_components/landing-page/ai-features-intro-section.tsx(1 hunks)apps/web/client/src/app/_components/landing-page/benefits-section.tsx(2 hunks)apps/web/client/src/app/_components/landing-page/cta-section.tsx(2 hunks)apps/web/client/src/app/_components/landing-page/faq-section.tsx(2 hunks)apps/web/client/src/app/_components/landing-page/features-faq-section.tsx(0 hunks)apps/web/client/src/app/_components/landing-page/features-grid-section.tsx(1 hunks)apps/web/client/src/app/_components/landing-page/features-intro-section.tsx(1 hunks)apps/web/client/src/app/_components/landing-page/features-section.tsx(0 hunks)apps/web/client/src/app/_components/landing-page/page-footer.tsx(1 hunks)apps/web/client/src/app/_components/landing-page/responsive-mockup-section.tsx(2 hunks)apps/web/client/src/app/_components/landing-page/what-can-onlook-do-section.tsx(1 hunks)apps/web/client/src/app/features/ai/page.tsx(1 hunks)apps/web/client/src/app/features/builder/page.tsx(1 hunks)apps/web/client/src/app/features/page.tsx(1 hunks)apps/web/client/src/app/features/prototype/page.tsx(1 hunks)apps/web/client/src/app/layout.tsx(2 hunks)apps/web/client/src/app/page.tsx(2 hunks)apps/web/client/src/app/pricing/page.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/help-dropdown/index.tsx(1 hunks)apps/web/client/src/app/projects/page.tsx(2 hunks)apps/web/client/src/components/rb2b-loader.tsx(1 hunks)apps/web/client/src/components/ui/avatar-dropdown/index.tsx(1 hunks)apps/web/client/src/components/ui/feedback-modal.tsx(1 hunks)apps/web/client/src/components/ui/pricing-modal/enterprise-card.tsx(1 hunks)apps/web/client/src/components/ui/pricing-modal/free-card.tsx(5 hunks)apps/web/client/src/components/ui/pricing-modal/pro-card.tsx(4 hunks)apps/web/client/src/components/ui/pricing-table/index.tsx(1 hunks)apps/web/client/src/env.ts(4 hunks)apps/web/client/src/utils/constants/index.ts(2 hunks)docs/src/app/layout.tsx(2 hunks)docs/src/components/rb2b-loader.tsx(1 hunks)packages/constants/src/contact.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/web/client/src/app/_components/landing-page/features-faq-section.tsx
- apps/web/client/src/app/_components/landing-page/features-section.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/client/src/env.ts
- apps/web/client/src/components/ui/feedback-modal.tsx
🧰 Additional context used
🧬 Code graph analysis (17)
docs/src/components/rb2b-loader.tsx (1)
apps/web/client/src/components/rb2b-loader.tsx (1)
RB2BLoader(7-24)
apps/web/client/src/app/layout.tsx (1)
apps/web/client/src/components/rb2b-loader.tsx (1)
RB2BLoader(7-24)
apps/web/client/src/components/rb2b-loader.tsx (2)
docs/src/components/rb2b-loader.tsx (1)
RB2BLoader(6-24)apps/web/client/src/env.ts (1)
env(4-155)
apps/web/client/src/components/ui/pricing-table/index.tsx (6)
apps/web/client/src/trpc/react.tsx (1)
api(23-23)apps/web/client/src/app/auth/auth-context.tsx (1)
useAuthContext(66-72)apps/web/client/src/components/ui/pricing-modal/free-card.tsx (1)
FreeCard(32-158)apps/web/client/src/components/ui/pricing-modal/pro-card.tsx (1)
ProCard(25-252)apps/web/client/src/components/ui/pricing-modal/enterprise-card.tsx (1)
EnterpriseCard(22-80)apps/web/client/src/i18n/keys.ts (1)
transKeys(5-5)
apps/web/client/src/app/_components/landing-page/responsive-mockup-section.tsx (1)
apps/web/client/src/app/_components/landing-page/onlook-interface-mockup.tsx (1)
OnlookInterfaceMockup(80-454)
apps/web/client/src/app/_components/hero/ai-features-hero.tsx (3)
apps/web/client/src/app/_components/top-bar/github.tsx (1)
useGitHubStats(17-55)apps/web/client/src/utils/constants/index.ts (1)
Routes(1-26)apps/web/client/src/app/_components/hero/unicorn-background.tsx (1)
UnicornBackground(73-145)
apps/web/client/src/app/projects/page.tsx (1)
apps/web/client/src/components/ui/feedback-modal.tsx (1)
FeedbackModal(40-548)
docs/src/app/layout.tsx (1)
docs/src/components/rb2b-loader.tsx (1)
RB2BLoader(6-24)
apps/web/client/src/components/ui/pricing-modal/enterprise-card.tsx (2)
packages/ui/src/components/motion-card.tsx (1)
MotionCard(75-75)packages/ui/src/components/button.tsx (1)
Button(57-57)
apps/web/client/src/app/features/ai/page.tsx (7)
apps/web/client/src/components/store/create/index.tsx (1)
CreateManagerProvider(12-22)apps/web/client/src/app/_components/website-layout.tsx (1)
WebsiteLayout(11-28)apps/web/client/src/app/_components/landing-page/cta-section.tsx (1)
CTASection(15-75)apps/web/client/src/app/_components/landing-page/faq-section.tsx (1)
FAQSection(43-70)apps/web/client/src/components/ui/settings-modal/non-project.tsx (1)
NonProjectSettingsModal(12-105)apps/web/client/src/components/ui/pricing-modal/index.tsx (1)
SubscriptionModal(16-110)apps/web/client/src/components/ui/feedback-modal.tsx (1)
FeedbackModal(40-548)
apps/web/client/src/app/_components/landing-page/benefits-section.tsx (2)
apps/web/client/src/app/_components/shared/mockups/ai-chat-interactive.tsx (1)
AiChatInteractive(50-162)apps/web/client/src/app/_components/shared/mockups/direct-editing-interactive.tsx (1)
DirectEditingInteractive(226-476)
apps/web/client/src/app/pricing/page.tsx (5)
apps/web/client/src/app/_components/website-layout.tsx (1)
WebsiteLayout(11-28)apps/web/client/src/components/ui/pricing-table/index.tsx (1)
PricingTable(11-40)apps/web/client/src/app/_components/landing-page/faq-section.tsx (1)
FAQSection(43-70)apps/web/client/src/app/_components/landing-page/cta-section.tsx (1)
CTASection(15-75)apps/web/client/src/app/_components/auth-modal.tsx (1)
AuthModal(15-42)
apps/web/client/src/app/features/prototype/page.tsx (14)
apps/web/client/src/app/_components/top-bar/github.tsx (1)
useGitHubStats(17-55)apps/web/client/src/utils/constants/index.ts (1)
Routes(1-26)apps/web/client/src/app/_components/hero/unicorn-background.tsx (1)
UnicornBackground(73-145)apps/web/client/src/app/_components/shared/mockups/ai-chat-interactive.tsx (1)
AiChatInteractive(50-162)apps/web/client/src/app/_components/shared/mockups/direct-editing-interactive.tsx (1)
DirectEditingInteractive(226-476)apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx (1)
TailwindColorEditorMockup(12-264)apps/web/client/src/app/_components/button-link.tsx (1)
ButtonLink(2-24)apps/web/client/src/app/_components/landing-page/faq-dropdown.tsx (1)
FAQDropdown(13-47)apps/web/client/src/components/store/create/index.tsx (1)
CreateManagerProvider(12-22)apps/web/client/src/app/_components/landing-page/responsive-mockup-section.tsx (1)
ResponsiveMockupSection(4-52)apps/web/client/src/app/_components/landing-page/cta-section.tsx (1)
CTASection(15-75)apps/web/client/src/components/ui/settings-modal/non-project.tsx (1)
NonProjectSettingsModal(12-105)apps/web/client/src/components/ui/pricing-modal/index.tsx (1)
SubscriptionModal(16-110)apps/web/client/src/components/ui/feedback-modal.tsx (1)
FeedbackModal(40-548)
apps/web/client/src/app/features/page.tsx (3)
apps/web/client/src/app/_components/landing-page/responsive-mockup-section.tsx (1)
ResponsiveMockupSection(4-52)apps/web/client/src/app/_components/landing-page/faq-section.tsx (1)
FAQSection(43-70)apps/web/client/src/components/ui/feedback-modal.tsx (1)
FeedbackModal(40-548)
apps/web/client/src/app/_components/landing-page/ai-benefits-section.tsx (4)
apps/web/client/src/app/_components/shared/mockups/ai-chat-interactive.tsx (1)
AiChatInteractive(50-162)apps/web/client/src/app/_components/shared/mockups/direct-editing-interactive.tsx (1)
DirectEditingInteractive(226-476)packages/ui/src/components/icons/index.tsx (1)
Icons(137-3592)apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx (1)
TailwindColorEditorMockup(12-264)
apps/web/client/src/app/page.tsx (1)
apps/web/client/src/components/ui/feedback-modal.tsx (1)
FeedbackModal(40-548)
apps/web/client/src/app/_components/landing-page/faq-section.tsx (3)
apps/web/client/src/utils/constants/index.ts (1)
Routes(1-26)apps/web/client/src/app/_components/button-link.tsx (1)
ButtonLink(2-24)apps/web/client/src/app/_components/landing-page/faq-dropdown.tsx (1)
FAQDropdown(13-47)
🪛 dotenv-linter (3.3.0)
apps/web/client/.env.example
[warning] 58-58: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 59-59: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Supabase Preview
| { | ||
| question: 'What is the difference between Onlook and other design tools?', | ||
| answer: 'Onlook is a visual editor for code. It allows you to create and style your own creations with code as the source of truth. While it is best suited for creating websites, it can be used for anything visual – presentations, mockups, and more. Because Onlook uses code as the source of truth, the types of designs you can create are unconstrained by Onlook interface.', | ||
| }, | ||
| { | ||
| question: 'Why is Onlook open-source?', | ||
| answer: 'Developers have historically been second-rate citizens in the design process. Onlook was founded to bridge the divide between design and development, and we wanted to make developers first-class citizens alongside designers. We chose to be open-source to give developers transparency into how we are building Onlook and how the work created through Onlook will complement the work of developers.', | ||
| }, |
There was a problem hiding this comment.
Fix user-facing copy (“Onlook interface” → “the Onlook interface”)
Minor grammatical issue in the default FAQs answer. “Unconstrained by Onlook interface” should be “unconstrained by the Onlook interface.”
- answer: 'Onlook is a visual editor for code. It allows you to create and style your own creations with code as the source of truth. While it is best suited for creating websites, it can be used for anything visual – presentations, mockups, and more. Because Onlook uses code as the source of truth, the types of designs you can create are unconstrained by Onlook interface.',
+ answer: 'Onlook is a visual editor for code. It allows you to create and style your own creations with code as the source of truth. While it is best suited for creating websites, it can be used for anything visual – presentations, mockups, and more. Because Onlook uses code as the source of truth, the types of designs you can create are unconstrained by the Onlook interface.',📝 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.
| { | |
| question: 'What is the difference between Onlook and other design tools?', | |
| answer: 'Onlook is a visual editor for code. It allows you to create and style your own creations with code as the source of truth. While it is best suited for creating websites, it can be used for anything visual – presentations, mockups, and more. Because Onlook uses code as the source of truth, the types of designs you can create are unconstrained by Onlook interface.', | |
| }, | |
| { | |
| question: 'Why is Onlook open-source?', | |
| answer: 'Developers have historically been second-rate citizens in the design process. Onlook was founded to bridge the divide between design and development, and we wanted to make developers first-class citizens alongside designers. We chose to be open-source to give developers transparency into how we are building Onlook and how the work created through Onlook will complement the work of developers.', | |
| }, | |
| { | |
| question: 'What is the difference between Onlook and other design tools?', | |
| answer: 'Onlook is a visual editor for code. It allows you to create and style your own creations with code as the source of truth. While it is best suited for creating websites, it can be used for anything visual – presentations, mockups, and more. Because Onlook uses code as the source of truth, the types of designs you can create are unconstrained by the Onlook interface.', | |
| }, |
🤖 Prompt for AI Agents
apps/web/client/src/app/_components/landing-page/faq-section.tsx around lines 33
to 40: fix the user-facing copy by updating the sentence that currently reads
“unconstrained by Onlook interface” to include the definite article; change it
to “unconstrained by the Onlook interface” so the phrasing is grammatically
correct and reads naturally.
| <div className="flex-1"> | ||
| <h2 className="text-4xl lg:text-5xl font-light text-foreground-primary leading-tight"> | ||
| <span className="bg-gradient-to-l from-white/20 via-white/90 to-white/20 bg-[length:200%_100%] bg-clip-text text-transparent animate-shimmer filter drop-shadow-[0_0_14px_rgba(255,255,255,1)]">AI</span> <span className="text-foreground-tertiary">•</span> <span className="font-mono">Code</span> <span className="text-foreground-tertiary">•</span> <span className="font-['Vujahday_Script'] not-italic text-5xl large:text-6xl">Design</span><br /> Side-by-side-by-side | ||
| </h2> | ||
| </div> |
There was a problem hiding this comment.
Fix invalid Tailwind breakpoint class: use lg: instead of large:.
large:text-6xl isn’t a standard Tailwind breakpoint and will be ignored. Replace with lg:text-6xl.
- <h2 className="text-4xl lg:text-5xl font-light text-foreground-primary leading-tight">
- <span className="bg-gradient-to-l from-white/20 via-white/90 to-white/20 bg-[length:200%_100%] bg-clip-text text-transparent animate-shimmer filter drop-shadow-[0_0_14px_rgba(255,255,255,1)]">AI</span> <span className="text-foreground-tertiary">•</span> <span className="font-mono">Code</span> <span className="text-foreground-tertiary">•</span> <span className="font-['Vujahday_Script'] not-italic text-5xl large:text-6xl">Design</span><br /> Side-by-side-by-side
+ <h2 className="text-4xl lg:text-5xl font-light text-foreground-primary leading-tight">
+ <span className="bg-gradient-to-l from-white/20 via-white/90 to-white/20 bg-[length:200%_100%] bg-clip-text text-transparent animate-shimmer filter drop-shadow-[0_0_14px_rgba(255,255,255,1)]">AI</span> <span className="text-foreground-tertiary">•</span> <span className="font-mono">Code</span> <span className="text-foreground-tertiary">•</span> <span className="font-['Vujahday_Script'] not-italic text-5xl lg:text-6xl">Design</span><br /> Side-by-side-by-side📝 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.
| <div className="flex-1"> | |
| <h2 className="text-4xl lg:text-5xl font-light text-foreground-primary leading-tight"> | |
| <span className="bg-gradient-to-l from-white/20 via-white/90 to-white/20 bg-[length:200%_100%] bg-clip-text text-transparent animate-shimmer filter drop-shadow-[0_0_14px_rgba(255,255,255,1)]">AI</span> <span className="text-foreground-tertiary">•</span> <span className="font-mono">Code</span> <span className="text-foreground-tertiary">•</span> <span className="font-['Vujahday_Script'] not-italic text-5xl large:text-6xl">Design</span><br /> Side-by-side-by-side | |
| </h2> | |
| </div> | |
| <div className="flex-1"> | |
| <h2 className="text-4xl lg:text-5xl font-light text-foreground-primary leading-tight"> | |
| <span className="bg-gradient-to-l from-white/20 via-white/90 to-white/20 bg-[length:200%_100%] bg-clip-text text-transparent animate-shimmer filter drop-shadow-[0_0_14px_rgba(255,255,255,1)]"> | |
| AI | |
| </span> | |
| <span className="text-foreground-tertiary">•</span> | |
| <span className="font-mono">Code</span> | |
| <span className="text-foreground-tertiary">•</span> | |
| <span className="font-['Vujahday_Script'] not-italic text-5xl lg:text-6xl"> | |
| Design | |
| </span> | |
| <br /> Side-by-side-by-side | |
| </h2> | |
| </div> |
🤖 Prompt for AI Agents
In
apps/web/client/src/app/_components/landing-page/what-can-onlook-do-section.tsx
around lines 289 to 293, the Tailwind class "large:text-6xl" is invalid and will
be ignored; replace it with the correct breakpoint class "lg:text-6xl" on the
same element (and search nearby for any other occurrences of "large:" to update
them as well).
| 'Visual code editor access', | ||
| '5 projects', | ||
| '5 AI chat messages a day', | ||
| '50 AI messages a month', | ||
| '15 AI messages a month', | ||
| 'Unlimited styling and code editing', | ||
| 'Limited to 1 screenshot per chat' | ||
| ], |
There was a problem hiding this comment.
Conflicting allowances: “5 AI chat messages a day” vs “15 AI messages a month”
These two bullets contradict each other (5/day implies ~150/month). Clarify the policy to avoid misleading users.
If the intent is “up to 5/day capped at 15/month”, apply:
- '5 AI chat messages a day',
- '15 AI messages a month',
+ 'Up to 5 AI messages/day (15 per month cap)',
'Unlimited styling and code editing',📝 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.
| 'Visual code editor access', | |
| '5 projects', | |
| '5 AI chat messages a day', | |
| '50 AI messages a month', | |
| '15 AI messages a month', | |
| 'Unlimited styling and code editing', | |
| 'Limited to 1 screenshot per chat' | |
| ], | |
| 'Visual code editor access', | |
| '5 projects', | |
| 'Up to 5 AI messages/day (15 per month cap)', | |
| 'Unlimited styling and code editing', | |
| 'Limited to 1 screenshot per chat' |
🤖 Prompt for AI Agents
In apps/web/client/src/components/ui/pricing-modal/free-card.tsx around lines 19
to 25, the two bullets “5 AI chat messages a day” and “15 AI messages a month”
conflict; consolidate into one clear allowance statement such as “Up to 5 AI
chat messages per day, capped at 15 per month” (or remove one of the bullets if
the intent is different), updating the copy accordingly to avoid ambiguity and
keeping the list punctuation/spacing consistent.
| if (isScheduledCancellation) { | ||
| return `Pro plan ends on ${subscription?.scheduledChange?.scheduledChangeAt.toLocaleDateString('en-US', { month: 'long', day: 'numeric', year: 'numeric' })}` | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden scheduled date formatting against serialized values and locale
TRPC often serializes Dates to strings. Calling toLocaleDateString on a string will throw. Coerce to Date and let the browser choose locale (or use next-intl).
- return `Pro plan ends on ${subscription?.scheduledChange?.scheduledChangeAt.toLocaleDateString('en-US', { month: 'long', day: 'numeric', year: 'numeric' })}`
+ const d = subscription?.scheduledChange?.scheduledChangeAt;
+ const when = d ? new Date(d as unknown as string | number | Date) : null;
+ return when
+ ? `Pro plan ends on ${when.toLocaleDateString(undefined, { month: 'long', day: 'numeric', year: 'numeric' })}`
+ : 'Pro plan scheduled to end';📝 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.
| if (isScheduledCancellation) { | |
| return `Pro plan ends on ${subscription?.scheduledChange?.scheduledChangeAt.toLocaleDateString('en-US', { month: 'long', day: 'numeric', year: 'numeric' })}` | |
| } | |
| if (isScheduledCancellation) { | |
| const d = subscription?.scheduledChange?.scheduledChangeAt; | |
| const when = d ? new Date(d as unknown as string | number | Date) : null; | |
| return when | |
| ? `Pro plan ends on ${when.toLocaleDateString(undefined, { month: 'long', day: 'numeric', year: 'numeric' })}` | |
| : 'Pro plan scheduled to end'; | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/components/ui/pricing-modal/free-card.tsx around lines 83
to 85, the code calls toLocaleDateString on
subscription?.scheduledChange?.scheduledChangeAt which may be a serialized
string; coerce the value to a Date before formatting, e.g. create a Date from
subscription?.scheduledChange?.scheduledChangeAt, verify it is a valid date (not
NaN) and then call toLocaleDateString without forcing 'en-US' so the browser
locale is used; also handle null/invalid by returning a safe fallback string (or
the original UI path) to avoid runtime exceptions.
| export const ProCard = ({ | ||
| delay, | ||
| isUnauthenticated = false, | ||
| onSignupClick, | ||
| }: { | ||
| delay: number; | ||
| isUnauthenticated?: boolean; | ||
| onSignupClick?: () => void; | ||
| }) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Public API: couple the unauthenticated flow so it can’t fall through to checkout
Right now, isUnauthenticated can be true without onSignupClick, which will fall back to handleCheckout (likely not intended). Either enforce this at the type level, or guard at runtime.
Apply one (or both) of the following:
Type-level enforcement (prevents misuse at compile time):
-export const ProCard = ({
- delay,
- isUnauthenticated = false,
- onSignupClick,
-}: {
- delay: number;
- isUnauthenticated?: boolean;
- onSignupClick?: () => void;
-}) => {
+type ProCardProps =
+ | { delay: number; isUnauthenticated: true; onSignupClick: () => void }
+ | { delay: number; isUnauthenticated?: false; onSignupClick?: never };
+
+export const ProCard = ({
+ delay,
+ isUnauthenticated = false,
+ onSignupClick,
+}: ProCardProps) => {And add a runtime guard (see comments at Lines 180-187 and 227-229 for the corresponding changes).
📝 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.
| export const ProCard = ({ | |
| delay, | |
| isUnauthenticated = false, | |
| onSignupClick, | |
| }: { | |
| delay: number; | |
| isUnauthenticated?: boolean; | |
| onSignupClick?: () => void; | |
| }) => { | |
| // apps/web/client/src/components/ui/pricing-modal/pro-card.tsx | |
| // — add this above the component — | |
| type ProCardProps = | |
| | { delay: number; isUnauthenticated: true; onSignupClick: () => void } | |
| | { delay: number; isUnauthenticated?: false; onSignupClick?: never }; | |
| // — replace the existing signature (lines 25–33) with — | |
| export const ProCard = ({ | |
| delay, | |
| isUnauthenticated = false, | |
| onSignupClick, | |
| }: ProCardProps) => { | |
| // … |
| const handleButtonClick = () => { | ||
| if (isUnauthenticated && onSignupClick) { | ||
| onSignupClick(); | ||
| } else { | ||
| handleCheckout(); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard unauthenticated fallback; don’t attempt checkout without a signup handler
If onSignupClick is missing, we should not attempt checkout. Provide a toast (localized if you prefer) and bail early.
- const handleButtonClick = () => {
- if (isUnauthenticated && onSignupClick) {
- onSignupClick();
- } else {
- handleCheckout();
- }
- };
+ const handleButtonClick = () => {
+ if (isUnauthenticated) {
+ if (onSignupClick) return onSignupClick();
+ toast.error('Please sign up to continue');
+ return;
+ }
+ handleCheckout();
+ };📝 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.
| const handleButtonClick = () => { | |
| if (isUnauthenticated && onSignupClick) { | |
| onSignupClick(); | |
| } else { | |
| handleCheckout(); | |
| } | |
| }; | |
| const handleButtonClick = () => { | |
| if (isUnauthenticated) { | |
| if (onSignupClick) return onSignupClick(); | |
| toast.error('Please sign up to continue'); | |
| return; | |
| } | |
| handleCheckout(); | |
| }; |
🤖 Prompt for AI Agents
In apps/web/client/src/components/ui/pricing-modal/pro-card.tsx around lines 180
to 187, the current handler may call handleCheckout even when the user is
unauthenticated and no onSignupClick handler exists; change the function to
early-return in that case: if isUnauthenticated and onSignupClick is present,
call it; if isUnauthenticated and onSignupClick is missing, show a user-facing
toast/error message (use the existing toast utility and localization strings if
available) and return without calling handleCheckout; otherwise proceed to call
handleCheckout.
| onClick={handleButtonClick} | ||
| disabled={isCheckingOut || (!isUnauthenticated && !isNewTierSelected)} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Disable button if unauthenticated and no signup handler
Prevents clicking a CTA that can’t proceed.
- disabled={isCheckingOut || (!isUnauthenticated && !isNewTierSelected)}
+ disabled={
+ isCheckingOut ||
+ (isUnauthenticated && !onSignupClick) ||
+ (!isUnauthenticated && !isNewTierSelected)
+ }📝 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.
| onClick={handleButtonClick} | |
| disabled={isCheckingOut || (!isUnauthenticated && !isNewTierSelected)} | |
| > | |
| onClick={handleButtonClick} | |
| disabled={ | |
| isCheckingOut || | |
| (isUnauthenticated && !onSignupClick) || | |
| (!isUnauthenticated && !isNewTierSelected) | |
| } | |
| > |
🤖 Prompt for AI Agents
In apps/web/client/src/components/ui/pricing-modal/pro-card.tsx around lines 227
to 229, the CTA button is currently disabled only when checking out or when an
authenticated user hasn't selected a new tier; update the disabled logic to also
disable the button when the user is unauthenticated and there is no signup
handler available. Change the disabled prop to include a condition like:
isCheckingOut || ((!isUnauthenticated && !isNewTierSelected) ||
(isUnauthenticated && !onSignup)), so the button cannot be clicked if the user
is unauthenticated with no signup flow provided.
| FEATURES_BUILDER: '/features/builder', | ||
| FEATURES_PROTOTYPE: '/features/prototype', |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New feature routes added—validate presence and usage
Routes.FEATURES_BUILDER and Routes.FEATURES_PROTOTYPE look consistent with the features namespace. Verify corresponding pages exist and that in-app links use these constants (not raw strings).
If these routes are public, I can draft a follow-up to add them to the sitemap and any nav surfaces.
🏁 Script executed:
#!/bin/bash
# Confirm pages exist
fd -a 'page.tsx' | rg -nP '/features/(builder|prototype)/page\.tsx$' || true
# Check for consumers of the new route constants
rg -nP '\bRoutes\.(FEATURES_BUILDER|FEATURES_PROTOTYPE)\b' -C2Length of output: 277
Validate feature routes and update in-app links
- Verified that the pages exist at
•apps/web/client/src/app/features/builder/page.tsx
•apps/web/client/src/app/features/prototype/page.tsx - No references to
Routes.FEATURES_BUILDERorRoutes.FEATURES_PROTOTYPEwere found in the codebase. - Please update all in-app links, navigation items, or
<Link>usages to import and use these constants instead of hard-coded strings. - If these routes are intended to be public, let me know—happy to help draft updates for the sitemap and any global navigation surfaces.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
apps/web/client/src/server/api/routers/feedback.ts (4)
110-133: Check webhook response; consider centralizing via utility.
- Log non-2xx responses to avoid silent failures.
- Prior reviews suggested reusing a
callUserWebhookutility for consistency.- await fetch(env.N8N_WEBHOOK_URL, { + const resp = await fetch(env.N8N_WEBHOOK_URL, { method: 'POST', headers: { 'n8n-api-key': env.N8N_API_KEY, 'Content-Type': 'application/json' }, body: JSON.stringify({ type: 'feedback', message: feedback.message, userEmail: feedback.email, userName: userId ? ctx.user?.user_metadata?.name || ctx.user?.user_metadata?.display_name : 'Anonymous', pageUrl: feedback.pageUrl, submittedAt: feedback.createdAt.toISOString(), feedbackId: feedback.id, }), }); + if (!resp.ok) { + console.error('N8N webhook non-OK response:', resp.status, await resp.text().catch(() => '')); + }
158-166: Hardcoding admin email is brittle; move to roles or env-based allowlist.Previous reviews flagged this. At minimum, use an env allowlist (e.g.,
ADMIN_EMAILS) and normalize email. Long-term: proper RBAC.Example change (requires adding
ADMIN_EMAILStoenv.ts):- if (!ctx.user || ctx.user.email !== '[email protected]') { + const admins = (env.ADMIN_EMAILS ?? '').split(',').map(s => s.trim().toLowerCase()).filter(Boolean); + if (!ctx.user || !admins.includes(ctx.user.email?.toLowerCase() ?? '')) { return []; }
9-11: Introduce a server-side message length cap (align with UI).Prevent abuse and oversized DB rows by enforcing a cap (e.g., 5000 chars).
const RATE_LIMIT_WINDOW_HOURS = 1; const RATE_LIMIT_MAX_SUBMISSIONS = 3; +const MAX_MESSAGE_LENGTH = 5000;
41-41: Add rate limiting for anonymous users (by email).Unauthenticated submissions can currently bypass rate limits. Add an email-keyed limiter in the same time window.
} + // Rate limiting for anonymous users (by email) + if (!userId && userEmail) { + const oneHourAgo = new Date(Date.now() - RATE_LIMIT_WINDOW_HOURS * 60 * 60 * 1000); + const recentAnon = await ctx.db + .select({ count: count() }) + .from(feedbacks) + .where(and(eq(feedbacks.email, userEmail), gte(feedbacks.createdAt, oneHourAgo))); + const anonCount = recentAnon[0]?.count || 0; + if (anonCount >= RATE_LIMIT_MAX_SUBMISSIONS) { + throw new TRPCError({ + code: 'TOO_MANY_REQUESTS', + message: 'Too many feedback submissions. Please wait before submitting again.', + }); + } + }
🧹 Nitpick comments (4)
packages/email/src/feedback.ts (2)
2-2: Avoid cross-package relative imports; depend on the published/aliased package.
../../constants/src/contactcouples internal file layout across packages and breaks after builds. Prefer the package entry (e.g.,@onlook/constants) or a local re-export from this package.-import { CONTACT_EMAIL, SUPPORT_EMAIL } from '../../constants/src/contact'; +import { CONTACT_EMAIL, SUPPORT_EMAIL } from '@onlook/constants';If
@onlook/constantsisn't exposed, re-export frompackages/emailor add a path alias in tsconfig.
12-16: Dry-run logs full HTML (PII) to console; consider truncation.Even in non-prod, this can leak sensitive content into logs. Log metadata and first ~1–2KB only.
- if (dryRun) { - const rendered = await render(FeedbackNotificationEmail(feedbackParams)); - console.log(rendered); - return; - } + if (dryRun) { + const rendered = await render(FeedbackNotificationEmail(feedbackParams)); + const preview = rendered.slice(0, 2000); + console.log('DRY RUN: feedback email preview (truncated to 2KB)\n', preview); + return; + }apps/web/client/src/server/api/routers/feedback.ts (2)
59-68: Normalize/cap auxiliary fields; optional attachment limits.
- Cap
pageUrlanduserAgentlengths (e.g., 2048/1024).- Optionally cap attachments count and reject oversized entries to avoid bloating rows/emails.
- pageUrl: input.pageUrl || null, - userAgent: input.userAgent || null, - attachments: input.attachments || [], + pageUrl: input.pageUrl?.slice(0, 2048) || null, + userAgent: input.userAgent?.slice(0, 1024) || null, + attachments: (input.attachments ?? []).slice(0, 5),If you want, I can add server-side validation (size/type whitelist) mirroring the client rules.
82-104: Pass FEEDBACK_FROM_EMAIL/FEEDBACK_TO_EMAIL and set Reply-To.Hook up the configurable emails and make it easy to respond directly to the reporter.
- await sendFeedbackNotificationEmail(resendClient, { + await sendFeedbackNotificationEmail(resendClient, { message: feedback.message, userEmail: feedback.email, userName: userId ? ctx.user?.user_metadata?.name || ctx.user?.user_metadata?.display_name : null, pageUrl: feedback.pageUrl, userAgent: feedback.userAgent, attachments: feedback.attachments as Array<{ name: string; size: number; type: string; url: string; uploadedAt: string; }>, metadata: feedback.metadata as Record<string, any>, submittedAt: feedback.createdAt, }, { - dryRun: env.NODE_ENV !== 'production', + dryRun: env.NODE_ENV !== 'production', + from: env.FEEDBACK_FROM_EMAIL, + to: env.FEEDBACK_TO_EMAIL, + replyTo: feedback.email ?? undefined, });This assumes the email helper accepts these overrides (see my comment on
packages/email/src/feedback.ts).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/client/src/server/api/routers/feedback.ts(1 hunks)packages/email/src/feedback.ts(1 hunks)packages/email/src/templates/feedback-notification.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/email/src/templates/feedback-notification.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/client/src/server/api/routers/feedback.ts (6)
apps/web/client/src/server/api/trpc.ts (1)
createTRPCRouter(88-88)packages/db/src/schema/feedback/feedback.ts (3)
feedbackSubmitSchema(48-55)feedbacks(7-23)NewFeedback(58-58)apps/web/client/src/env.ts (1)
env(4-155)packages/email/src/client.ts (1)
getResendClient(3-5)packages/email/src/feedback.ts (1)
sendFeedbackNotificationEmail(6-27)apps/web/client/src/utils/analytics/server.ts (1)
trackEvent(26-32)
packages/email/src/feedback.ts (3)
packages/email/src/types/send-email.ts (1)
SendEmailParams(7-7)packages/email/src/templates/feedback-notification.tsx (2)
FeedbackNotificationEmailProps(15-30)FeedbackNotificationEmail(32-167)packages/constants/src/contact.ts (2)
CONTACT_EMAIL(2-2)SUPPORT_EMAIL(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Supabase Preview
🔇 Additional comments (1)
apps/web/client/src/server/api/routers/feedback.ts (1)
152-156: LGTM on non-blocking side effects and safe success path.The mutation isolates failures (email/webhook/analytics) so user submissions aren’t blocked. Good UX choice.
| const [client, feedbackParams, { dryRun = false } = {}] = params; | ||
| const { userEmail, userName } = feedbackParams; | ||
|
|
||
| if (dryRun) { | ||
| const rendered = await render(FeedbackNotificationEmail(feedbackParams)); | ||
| console.log(rendered); | ||
| return; | ||
| } | ||
|
|
||
| const fromEmail = CONTACT_EMAIL; | ||
| const toEmail = SUPPORT_EMAIL; | ||
|
|
||
| return await client.emails.send({ | ||
| from: `Onlook Feedback <${fromEmail}>`, | ||
| to: toEmail, | ||
| subject: `New Feedback from ${userName || userEmail || 'Anonymous User'}`, | ||
| react: FeedbackNotificationEmail(feedbackParams), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sanitize header values, add reply-to, and make from/to configurable (env- or caller-provided).
- Prevent header injection by stripping CR/LF from user-controlled parts in the Subject.
- Let callers override From/To (to align with FEEDBACK_FROM_EMAIL/FEEDBACK_TO_EMAIL) and set Reply-To to the submitter when present.
export const sendFeedbackNotificationEmail = async (
...params: SendEmailParams<FeedbackNotificationEmailProps>
) => {
- const [client, feedbackParams, { dryRun = false } = {}] = params;
- const { userEmail, userName } = feedbackParams;
+ const [client, feedbackParams, { dryRun = false, from, to, replyTo } = {}] = params;
+ const { userEmail, userName } = feedbackParams;
+ // Strip CR/LF to avoid header injection in Subject
+ const display = (userName || userEmail || 'Anonymous User')?.replace(/[\r\n]/g, ' ').trim();
if (dryRun) {
const rendered = await render(FeedbackNotificationEmail(feedbackParams));
console.log(rendered);
return;
}
- const fromEmail = CONTACT_EMAIL;
- const toEmail = SUPPORT_EMAIL;
+ const fromEmail = from || CONTACT_EMAIL;
+ const toEmail = to || SUPPORT_EMAIL;
return await client.emails.send({
- from: `Onlook Feedback <${fromEmail}>`,
- to: toEmail,
- subject: `New Feedback from ${userName || userEmail || 'Anonymous User'}`,
- react: FeedbackNotificationEmail(feedbackParams),
+ from: `Onlook Feedback <${fromEmail}>`,
+ to: toEmail,
+ // Reply to the reporter when possible
+ reply_to: replyTo ?? userEmail ?? undefined,
+ subject: `New Feedback from ${display}`,
+ react: FeedbackNotificationEmail(feedbackParams),
});
};Outside this file, extend the SendEmailOptions type accordingly:
// packages/email/src/types/send-email.ts
export interface SendEmailOptions {
dryRun?: boolean;
from?: string;
to?: string;
replyTo?: string;
subject?: string;
}
export type SendEmailParams<P> = [client: Resend, props: P, options?: Partial<SendEmailOptions>];🤖 Prompt for AI Agents
In packages/email/src/feedback.ts around lines 9 to 26, the subject and headers
are currently built from user-controlled values and From/To are hardcoded;
update the function to: 1) accept optional from/to/replyTo/subject via the
options object (falling back to FEEDBACK_FROM_EMAIL/FEEDBACK_TO_EMAIL
CONTACT_EMAIL/SUPPORT_EMAIL env constants), 2) sanitize any user-provided
strings used in headers (strip CR and LF characters from userName and userEmail
before using them in the Subject or Reply-To to prevent header injection), 3)
set a Reply-To header to the submitter email when present, and 4) use the
caller/env-provided subject if given, otherwise build a sanitized subject
string; ensure dryRun behavior remains unchanged and the final
client.emails.send call uses the resolved from, to, replyTo and subject values.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Introduces a feedback feature with a modal for submissions, email notifications, and backend support for storing feedback data.
FeedbackModalcomponent for in-app feedback submission with message, email, and file attachments.feedbackRouterinrouters/feedback.tsfor handling feedback submissions and rate limiting.FeedbackModalto various pages includingfeatures/page.tsx,main.tsx, andprojects/page.tsx.feedbackstable schema infeedback.tsfor storing feedback data.feedback.tsandfeedback-notification.tsx.FEEDBACK_FROM_EMAILandFEEDBACK_TO_EMAILto.env.exampleandenv.ts.This description was created by
for 0a04420. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
UI
Backend / Email
Chores