Skip to content

Chore/add preview modal#313

Merged
FloB95 merged 3 commits intomainfrom
chore/add-preview-modal
Jan 8, 2026
Merged

Chore/add preview modal#313
FloB95 merged 3 commits intomainfrom
chore/add-preview-modal

Conversation

@FloB95
Copy link
Copy Markdown
Owner

@FloB95 FloB95 commented Jan 8, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Added QR code preview functionality with customizable backgrounds.
    • Enabled website screenshot previews, camera capture, and custom image uploads for QR backgrounds.
    • Introduced predefined background gallery with multiple categories.
    • Dynamic content type preferences now persist across sessions.
  • Improvements

    • Enhanced preview modal with draggable, resizable, and rotatable QR code overlays.
    • Added rate limiting for screenshot creation based on user tier.
    • Extended multi-language support with new preview UI translations.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions github-actions Bot added the chore Simple change label Jan 8, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 8, 2026

Walkthrough

Introduces a comprehensive website screenshot preview feature for QR codes. Adds backend screenshot capture service via Thum.io integration with rate limiting, a new GET endpoint, and frontend modal system supporting website screenshots, live camera capture, and image uploads alongside predefined backgrounds. Includes QR code overlay positioning/rotation controls and extensive internationalization across seven languages.

Changes

Cohort / File(s) Summary
Backend Build & Dependencies
.github/workflows/backend-test.yml
Added step to build @shared/schemas package before running checks; adjusted pnpm install to run from repository root.
Backend Core Interfaces
apps/backend/src/core/interface/request.interface.ts
Renamed generic type parameters B, P, QBody, Params, Query on IHttpRequest interface for improved readability.
Backend Rate Limiting
apps/backend/src/core/rate-limit/rate-limit.policy.ts
Added SCREENSHOT_CREATE policy with tier-specific limits: ANONYMOUS=0, AUTHENTICATED=3, PRO_PLAN=10.
Backend Screenshot Service
apps/backend/src/core/services/screenshot.service.ts
New injectable service capturing website screenshots via Thum.io with 5MB size validation and error handling.
Backend QR Code Endpoint
apps/backend/src/modules/qr-code/http/controller/qr-code.controller.ts
Added GET /screenshot endpoint with WebsiteScreenshotDto validation, returns JPEG with 24-hour cache header; injects ScreenshotService.
Shared DTOs
packages/shared/src/dtos/WebsiteScreenshotDto.ts, packages/shared/src/index.ts
New DTO for website screenshot requests with HTTP URL validation via Zod schema; added re-export to shared index.
Frontend QR Code Components
apps/frontend/src/components/qr-generator/QrCodeWithDownloadBtn.tsx, QrCodeWithUpdateBtn.tsx, templates/QrCodeWithTemplateUpdateBtn.tsx
Added QrPreview component rendering to display preview button alongside existing generator controls.
Frontend Preview Modal & Root
apps/frontend/src/components/qr-generator/preview/QrPreview.tsx, QrPreviewModal.tsx, index.ts, types.ts
New preview system: QrPreview button with login guard, QrPreviewModal with two-step workflow (background selection → QR overlay positioning), comprehensive type definitions for hooks and state.
Frontend Background Selection
apps/frontend/src/components/qr-generator/preview/BackgroundSelector.tsx, WebsitePreview.tsx, CameraCapture.tsx, PredefinedBackgrounds.tsx
Four background source components: website URL input with screenshot capture, live camera feed capture, file upload with validation, predefined category grid with selection state.
Frontend Preview Hooks
apps/frontend/src/components/qr-generator/preview/hooks/useCamera.ts, useQrPosition.ts
useCamera: manages device camera access, frame capture, permission states; useQrPosition: handles QR code position/size/rotation with boundary constraints and pointer event handling.
Frontend Content Generator
apps/frontend/src/components/qr-generator/content/ContentSwitch.tsx
Added localStorage persistence for dynamic content preferences (isEditable for URL, isDynamic for vCard) with initialization guards to prevent redundant updates.
Frontend UI Components
apps/frontend/src/components/ui/dialog.tsx, style/ColorPicker.tsx
DialogContent now accepts optional showOverlay prop to hide backdrop; updated close button positioning and shadow styling.
Frontend Other
apps/frontend/src/components/provider/PostHogProvider.tsx, qr-generator/content/EditUrlSection.tsx
Added guard to skip PostHog init if env vars undefined; removed p-6 padding from URL input.
Frontend API & Utilities
apps/frontend/src/lib/api/screenshot.ts, lib/screenshot.utils.ts, lib/color-picker.utils.ts
New screenshot mutation hook calling backend endpoint with auth token; utility functions for URL validation, image↔data-URL conversion, blob↔data-URL conversion; improved gradient validation and offset sanitization.
Frontend Environment Config
apps/frontend/src/env.js
Made NEXT_PUBLIC_POSTHOG_KEY and NEXT_PUBLIC_POSTHOG_HOST optional instead of required.
Frontend Localization
apps/frontend/src/dictionaries/en.json, de.json, es.json, fr.json, it.json, nl.json, pl.json, ru.json
Added comprehensive preview section with UI strings for all preview features: button, modal titles, background selection, website/camera/upload forms, contrast levels, positioning controls, and error messages across all supported languages.
Configuration & Documentation
.gitignore, CLAUDE.md, apps/frontend/content/docs/changelogs.mdx
Removed CLAUDE.md from ignore list (now tracked); added 420-line developer guide for monorepo context; updated changelog from v1.6.0 to v1.6.1 with preview feature highlights.

Sequence Diagram

sequenceDiagram
    actor User
    participant Frontend as Frontend App
    participant Backend as Backend API
    participant Thum as Thum.io Service

    User->>Frontend: Opens QR Preview Modal
    Frontend->>Frontend: Renders Background Selector
    
    alt Website Background
        User->>Frontend: Enters URL
        Frontend->>Frontend: Validates URL format
        Frontend->>Backend: GET /qr-code/screenshot?url=...
        Backend->>Thum: Request screenshot image
        Thum-->>Backend: Return image buffer
        Backend->>Backend: Validate size (<5MB)
        Backend-->>Frontend: Return JPEG (24h cache)
        Frontend->>Frontend: Convert to data URL
        Frontend->>Frontend: Display preview
    else Camera Background
        User->>Frontend: Triggers camera capture
        Frontend->>Frontend: Request camera permission
        Frontend->>Frontend: Start video stream
        User->>Frontend: Clicks capture frame
        Frontend->>Frontend: Draw frame to canvas
        Frontend->>Frontend: Convert to data URL
        Frontend->>Frontend: Display preview
    else Upload Background
        User->>Frontend: Selects/drags image file
        Frontend->>Frontend: Validate file (type, <5MB)
        Frontend->>Frontend: Read file as data URL
        Frontend->>Frontend: Display preview
    end
    
    Frontend->>Frontend: Render QR overlay on background
    User->>Frontend: Drag/resize/rotate QR code
    Frontend->>Frontend: Update position constraints
    Frontend->>Frontend: Re-render with new position
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested labels

enhancement

Poem

🐰 A screenshot's here, the camera's alive,
Backgrounds await, let previews thrive!
Drag and rotate, position just right,
QR codes dancing in spotlight!

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request has no description provided by the author. The template requires summary, type of change, checklist items, and related issues, all of which are missing. Add a comprehensive description explaining the preview modal feature, type of change (appears to be 'New feature'), checklist confirmations, and link any related issues. Include what problem this solves and why it's necessary.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Chore/add preview modal' is partially related to the changeset—it accurately describes a major component addition (preview modal), but undersells the scope of the PR which includes backend screenshot service, frontend preview feature with multiple sub-components (camera, background selector, website preview), and new localization strings across multiple languages.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/add-preview-modal

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 19

🤖 Fix all issues with AI agents
In @apps/backend/src/core/services/screenshot.service.ts:
- Around line 14-21: captureWebsite is currently passing the raw url into the
external service (screenshotUrl) creating SSRF and protocol issues; before
building screenshotUrl, validate and parse the incoming url in captureWebsite:
use URL parsing to reject malformed URLs, enforce only http: and https:
protocols, and block localhost or private/internal IP hostnames (e.g., patterns
for localhost, 127.*, 10.*, 172.16-31.*, 192.168.*, ::1, fe80:, fc00:, etc.); if
validation fails throw a BadRequestError (or existing equivalent) and only then
construct screenshotUrl and proceed with the request while keeping
this.logger.info('screenshot.take', { url }) as-is.
- Around line 57-64: The catch block in the screenshot capture flow currently
wraps every error as BadRequestError; update the catch in the method containing
the shown catch (the screenshot service method that calls this.logger.error) to
map different failure conditions to appropriate HTTP error types instead of
always throwing BadRequestError: keep the this.logger.error call but inspect the
caught error (e.g., error.name, error.message, or any response/status fields)
and throw ServiceUnavailableError (503) for network failures/timeouts,
GatewayTimeoutError (504) for request timeouts/AbortError, BadGatewayError (502)
for upstream non-200 responses, and only throw BadRequestError for genuine
input/URL validation problems; ensure the thrown errors use clear messages and
preserve/log the original error details.
- Around line 24-28: The fetch in screenshot.service.ts that calls
fetch(screenshotUrl, { headers: { 'User-Agent': 'QRcodly/1.0' } }) needs an
AbortController timeout to avoid hanging: create an AbortController, pass its
signal to fetch, start a setTimeout that calls controller.abort() after a
sensible timeout (e.g., 5–10s) and clear the timeout on success; update the
fetch invocation to include signal and ensure the response variable is used as
before; and update the catch block to specifically handle an AbortError (e.name
=== 'AbortError') so aborts are handled distinctly from other errors.

In @apps/backend/src/modules/qr-code/http/controller/qr-code.controller.ts:
- Around line 304-312: The /screenshot route decorator block (the
@Get('/screenshot', { querySchema: WebsiteScreenshotDtoSchema, ... }) method)
must be moved so it is registered before any parameterized routes like
@Get('/:id', ...) and @Get('/:id/download', ...) to avoid path collision where
"screenshot" is treated as an :id; relocate the entire screenshot route
(decorator and its handler method) to a position above the first @Get('/:id'...)
route declaration (i.e., before the controller methods handling :id paths) so
the static /screenshot route takes precedence.

In @apps/frontend/content/docs/changelogs.mdx:
- Around line 41-46: The changelog entry "Removed unused contrast detection
hook" is incorrect; update the docs by either removing that list item or
replacing it with an accurate change: edit
apps/frontend/content/docs/changelogs.mdx and remove or modify the <li> that
contains "Removed unused contrast detection hook" (the list under "🛠 Technical
Improvements"), and if you keep a replacement claim, ensure you reference the
actual file/commit or include the deleted symbol name; also add a brief note in
the PR description referencing the search/commit that verifies the removal so
reviewers can validate the claim.

In @apps/frontend/src/components/qr-generator/content/ContentSwitch.tsx:
- Around line 85-123: The useEffect is at risk of an infinite render loop
because it depends on content.data (a new object each render) and calls
updateContent; change the dependencies to only the specific fields used (e.g.,
content.type plus content.data.isEditable and content.data.isDynamic or their
memoized equivalents) or create memoized values like
currentIsEditable/currentIsDynamic and depend on those instead of content.data,
and move the early bail-out check (lastInitializedTypeRef.current ===
contentTypeKey) before the isEditMode check so the effect short-circuits
earlier; ensure updateContent is only called when the specific primitive field
actually differs to avoid unnecessary state updates.
- Around line 55-68: The localStorage calls in getDynamicPreference and
setDynamicPreference can throw (e.g., private mode or quota); wrap the
localStorage.getItem and localStorage.setItem calls in try-catch blocks: in
getDynamicPreference catch and return the default true if any error occurs
(optionally log the error), and in setDynamicPreference catch and ignore/set
no-op on errors (optionally log). Keep the existing key logic (qr-url-isEditable
/ qr-vcard-isDynamic) and preserve the typeof window check; ensure the functions
still return boolean for getDynamicPreference and void for setDynamicPreference
even when an exception is caught.

In @apps/frontend/src/components/qr-generator/preview/BackgroundSelector.tsx:
- Around line 32-57: handleFileUpload currently silently returns on invalid
input; update it to give user feedback for validation failures and read errors.
Inside BackgroundSelector's handleFileUpload, when rejecting a file for
non-image type or size > 5MB, call the app's notification/feedback mechanism
(e.g., toast.error, an existing notify() util, or a new onError prop) with clear
messages like "Please upload an image file" and "Image must be under 5MB". Also
surface FileReader errors by handling reader.onerror and reporting those to the
same feedback channel, then return early as before.

In @apps/frontend/src/components/qr-generator/preview/CameraCapture.tsx:
- Around line 47-57: The handler handleCapture calls
t('error.captureFailedMessage') which doesn't exist; either add a new
translation key "captureFailedMessage" under the same namespace used by the
camera translations (e.g., generator.preview.camera.error.captureFailedMessage)
in all locale files, or change the t(...) call in handleCapture to an existing
key (for example reuse generator.preview.camera.error.title or a generic error
key) so the lookup succeeds; update the onError invocation accordingly to pass
the chosen existing key or the newly added key.

In @apps/frontend/src/components/qr-generator/preview/hooks/useCamera.ts:
- Around line 42-48: The code sets streamRef.current and then uses
videoRef.current after an await, which can be null if the component unmounted;
capture videoRef.current into a local (e.g., const video = videoRef.current) and
check if video is non-null before assigning video.srcObject and awaiting
video.play(); if null, cleanly return (or stop the stream) to avoid runtime
errors when accessing videoRef.current after the async getUserMedia/play steps.

In @apps/frontend/src/components/qr-generator/preview/QrPreviewModal.tsx:
- Around line 293-300: The animate prop currently sets opacity to 0
unconditionally due to `opacity: isResizing || isRotating ? 0 : 0`; update that
ternary in the QrPreviewModal component so handles are hidden while
resizing/rotating and visible otherwise by changing it to `opacity: isResizing
|| isRotating ? 0 : 1`; locate the animate block where `initial={{ opacity: 0
}}` and `animate={{ opacity: isResizing || isRotating ? 0 : 0, scale: ... }}`
are defined and replace the second 0 with 1 to restore normal visibility when
not interacting.
- Around line 332-339: The animate prop for the resize/rotate handle sets
opacity using a ternary that returns 0 in both branches (opacity: isResizing ||
isRotating ? 0 : 0), so update the animate object in QrPreviewModal (the animate
prop for the resize handle) to return 0 when resizing/rotating and 1 otherwise
(e.g., opacity: isResizing || isRotating ? 0 : 1); ensure any other identical
animate blocks in QrPreviewModal use the same fix so the handle becomes visible
when not interacting.
- Around line 96-141: The resize effect currently re-attaches on every position
change and may use a stale `position` closure; change it so `position` is not in
the dependency array (keep only `backgroundImage` and `setPosition`) and update
`handleResize` to avoid reading `position` from the outer scope—either read the
latest position from a ref (e.g., `positionRef.current`) that you keep in sync
where `setPosition` is called, or use the functional updater form inside
`setPosition` (e.g., setPosition(prev => ({ x: prev.x * widthRatio, y: prev.y *
heightRatio, ... }))) so the resize handler always applies to the latest state;
ensure you still add/remove the `resize` listener in `useEffect` and update any
refs when position changes elsewhere.

In @apps/frontend/src/components/ui/dialog.tsx:
- Line 51: The conditional render of DialogOverlay when using showOverlay breaks
Radix Dialog's focus trapping; either always render DialogOverlay but make it
visually transparent when showOverlay is false (so FocusScope/pointer-event
behavior is preserved) or switch the Dialog to non-modal (Dialog modal={false})
and add an explicit FocusScope/focus-trap to the dialog content; update the
usage around the DialogOverlay and Dialog components (references: DialogOverlay,
showOverlay, Dialog modal prop) to implement one of these approaches so keyboard
focus cannot escape the dialog.

In @apps/frontend/src/lib/api/screenshot.ts:
- Around line 18-31: The mutationFn currently sends the screenshot URL as a GET
query which risks URL length/caching and is semantically a mutation; change the
fetch to use POST to `${env.NEXT_PUBLIC_API_URL}/qr-code/screenshot`, include
Authorization and Content-Type: application/json headers, send JSON.stringify({
url: dto.url }) in the request body (keeping use of getToken() and
ScreenshotRequestDto), and update the backend /qr-code/screenshot handler to
accept POST with a JSON body and return the blob as before.
- Around line 7-9: Replace the local ScreenshotRequestDto with the shared
WebsiteScreenshotDto from packages/shared: remove the local interface
declaration for ScreenshotRequestDto, import WebsiteScreenshotDto and use it
everywhere the local type was referenced (e.g., in the mutation function
signature that currently accepts ScreenshotRequestDto and any handlers/call
sites in this file), and update the mutation/function parameter types to
WebsiteScreenshotDto to ensure consistent types across the codebase.

In @apps/frontend/src/lib/screenshot.utils.ts:
- Around line 44-85: The convertImageUrlToDataUrl function currently loads
arbitrary-sized images; add explicit size checks and safe downscaling to avoid
memory exhaustion: after img.onload read img.naturalWidth/naturalHeight, compare
them against a defined MAX_DIMENSION (e.g., 4096 or configurable) and a
MAX_PIXELS cap (e.g., MAX_DIMENSION*MAX_DIMENSION); if either limit is exceeded
either reject with a clear error or compute scaledWidth/scaledHeight preserving
aspect ratio and set canvas.width/canvas.height to the scaled values before
drawing; ensure the error paths use the existing reject flow and keep
crossOrigin/ctx handling unchanged so oversized images are handled safely.

In @CLAUDE.md:
- Around line 172-173: Replace hard tabs with spaces in the markdown around the
entries containing "free: { url: 10, text: 5, event: 0 }" and "pro: { url: null,
text: null, event: 100 }" and the other block with similar tab indentation so
all indentation uses spaces; add a fenced code block language identifier (e.g.,
"```typescript") to the unnamed fenced block that begins near the sample code
snippet shown in the file; and correct the phrase "rate limiting policies" to
the hyphenated form "rate-limiting policies" wherever it appears.
🧹 Nitpick comments (18)
apps/frontend/src/lib/color-picker.utils.ts (1)

65-74: Consider logging when gradient fallback occurs.

The validation correctly ensures at least 2 color stops for a valid CSS gradient. However, silently falling back to a solid color might mask data quality issues or bugs upstream. Consider adding a console warning in development mode to help debug scenarios where invalid gradient data is passed.

💡 Optional: Add development logging
 		const validStops = color.colorStops.filter(
 			(stop) => stop.color && typeof stop.color === 'string',
 		);
 
 		// Ensure we have at least 2 color stops for a valid gradient
 		if (validStops.length < 2) {
+			if (process.env.NODE_ENV === 'development') {
+				console.warn('Invalid gradient: less than 2 valid color stops', color);
+			}
 			// Fallback to first valid color or black
 			const fallbackColor = validStops[0]?.color || '#000000';
 			return fallbackColor;
 		}
apps/frontend/src/components/provider/PostHogProvider.tsx (1)

13-22: Consider conditionally rendering the entire provider.

The guard correctly prevents PostHog initialization when environment variables are missing. However, the PHProvider wrapper (line 34) still renders with a potentially uninitialized PostHog client, and downstream effects (user identification on lines 24-31 and pageview tracking) will attempt operations on the uninitialized client.

While PostHog handles this gracefully with no-ops, it's cleaner to conditionally render the provider entirely when PostHog is not configured.

♻️ Proposed refactor to conditionally render provider
 export function PostHogProvider({ children }: { children: React.ReactNode }) {
 	const { isSignedIn, user } = useUser();
+	const isPostHogEnabled = env.NEXT_PUBLIC_POSTHOG_KEY && env.NEXT_PUBLIC_POSTHOG_HOST;
 
 	useEffect(() => {
-		if (!env.NEXT_PUBLIC_POSTHOG_KEY || !env.NEXT_PUBLIC_POSTHOG_HOST) return;
+		if (!isPostHogEnabled) return;
 
 		posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY, {
 			api_host: 'https://ph.qrcodly.de',
 			ui_host: env.NEXT_PUBLIC_POSTHOG_HOST,
 			capture_pageview: false, // We capture pageviews manually
 			capture_pageleave: true, // Enable pageleave capture
 		});
 	}, []);
 
 	useEffect(() => {
-		if (isSignedIn) {
+		if (isSignedIn && isPostHogEnabled) {
 			posthog.identify(user.id, {
 				email: user?.primaryEmailAddress?.emailAddress,
 				fullName: user?.fullName,
 			});
 		}
-	}, [isSignedIn, user?.id, user?.fullName, user?.primaryEmailAddress?.emailAddress]);
+	}, [isSignedIn, isPostHogEnabled, user?.id, user?.fullName, user?.primaryEmailAddress?.emailAddress]);
+
+	// Don't wrap with PostHog provider if not enabled
+	if (!isPostHogEnabled) {
+		return <>{children}</>;
+	}
 
 	return (
 		<PHProvider client={posthog}>
 			<SuspendedPostHogPageView />
 			{children}
 		</PHProvider>
 	);
 }
apps/frontend/src/components/ui/dialog.tsx (1)

62-62: Minor: Unrelated layout adjustment.

The close button position change from top-4 right-4 to top-3 right-3 appears unrelated to the overlay feature. While the 4px adjustment is minimal, consider documenting such layout refinements separately or explaining the motivation in the commit message for clarity.

apps/frontend/src/lib/api/screenshot.ts (1)

23-31: Add timeout to prevent hanging requests.

The fetch request has no timeout, which could cause the UI to hang indefinitely if the backend is slow or unresponsive.

⏱️ Proposed timeout implementation
+const controller = new AbortController();
+const timeoutId = setTimeout(() => controller.abort(), 30000); // 30 second timeout
+
+try {
 	const response = await fetch(
 		`${env.NEXT_PUBLIC_API_URL}/qr-code/screenshot?url=${encodeURIComponent(dto.url)}`,
 		{
 			method: 'GET',
 			headers: {
 				Authorization: `Bearer ${token}`,
 			},
+			signal: controller.signal,
 		},
 	);
+} catch (error) {
+	clearTimeout(timeoutId);
+	if (error instanceof Error && error.name === 'AbortError') {
+		throw new ApiError('Screenshot request timed out', 408);
+	}
+	throw error;
+} finally {
+	clearTimeout(timeoutId);
+}
apps/frontend/src/components/qr-generator/preview/hooks/useQrPosition.ts (1)

220-232: Consider optimizing event listener management.

Currently, both handleResizeMove and handleRotateMove are attached to the pointermove event, and both handlers execute on every pointer move (though they early-return if not active). Similarly for pointerup with handleResizeEnd and handleRotateEnd.

While the early returns prevent unnecessary work, you could optimize by conditionally adding only the active handler:

♻️ Optional optimization
 useEffect(() => {
-  window.addEventListener('pointermove', handleResizeMove);
-  window.addEventListener('pointermove', handleRotateMove);
-  window.addEventListener('pointerup', handleResizeEnd);
-  window.addEventListener('pointerup', handleRotateEnd);
+  if (isResizing) {
+    window.addEventListener('pointermove', handleResizeMove);
+    window.addEventListener('pointerup', handleResizeEnd);
+  }
+  if (isRotating) {
+    window.addEventListener('pointermove', handleRotateMove);
+    window.addEventListener('pointerup', handleRotateEnd);
+  }

   return () => {
-    window.removeEventListener('pointermove', handleResizeMove);
-    window.removeEventListener('pointermove', handleRotateMove);
-    window.removeEventListener('pointerup', handleResizeEnd);
-    window.removeEventListener('pointerup', handleRotateEnd);
+    if (isResizing) {
+      window.removeEventListener('pointermove', handleResizeMove);
+      window.removeEventListener('pointerup', handleResizeEnd);
+    }
+    if (isRotating) {
+      window.removeEventListener('pointermove', handleRotateMove);
+      window.removeEventListener('pointerup', handleRotateEnd);
+    }
   };
- }, [handleResizeMove, handleRotateMove, handleResizeEnd, handleRotateEnd]);
+ }, [isResizing, isRotating, handleResizeMove, handleRotateMove, handleResizeEnd, handleRotateEnd]);

This reduces the number of event listener invocations when only one operation is active.

apps/frontend/src/components/qr-generator/preview/WebsitePreview.tsx (3)

38-38: Remove unused iframe element.

The component comment mentions "iframe-first approach with screenshot API fallback," but the iframe is never used for capture—only the screenshot API is called. This dead code should be removed to avoid confusion.

🧹 Proposed cleanup
 export function WebsitePreview({ onSelect, className = '' }: WebsitePreviewProps) {
 	const t = useTranslations('generator.preview.website');
 	const [isLoading, setIsLoading] = useState(false);
 	const [error, setError] = useState<string | null>(null);
-	const iframeRef = useRef<HTMLIFrameElement>(null);

And remove lines 181-187:

-		{/* Hidden iframe for capture attempts */}
-		<iframe
-			ref={iframeRef}
-			title="Website preview capture frame"
-			className="hidden"
-			aria-hidden="true"
-		/>

Also applies to: 181-187


73-91: Redundant error handling—outer catch is unreachable.

The captureWithScreenshotApi function already catches all errors internally (lines 60-68), so the catch block in onSubmit (lines 81-88) will never execute. This can be simplified.

♻️ Proposed simplification

Either remove the inner catch and let errors propagate:

 const captureWithScreenshotApi = useCallback(
 	async (websiteUrl: string) => {
-		try {
-			const dataUrl = await screenshotMutation.mutateAsync({ url: websiteUrl });
-			onSelect(dataUrl);
-			setIsLoading(false);
-		} catch (err) {
-			setError(
-				t('errors.screenshotFailed', {
-					default:
-						'Failed to capture screenshot. The screenshot service may be unavailable or the URL is invalid.',
-				}),
-			);
-			setIsLoading(false);
-		}
+		const dataUrl = await screenshotMutation.mutateAsync({ url: websiteUrl });
+		onSelect(dataUrl);
 	},
-	[onSelect, t, screenshotMutation],
+	[onSelect, screenshotMutation],
 );

 const onSubmit = useCallback(
 	async (data: WebsiteUrlFormData) => {
 		setIsLoading(true);
 		setError(null);

 		try {
 			await captureWithScreenshotApi(data.url);
 		} catch (err) {
 			setError(
-				t('errors.loadFailed', {
-					default: 'Failed to load website. Please try again.',
+				t('errors.screenshotFailed', {
+					default:
+						'Failed to capture screenshot. The screenshot service may be unavailable or the URL is invalid.',
 				}),
 			);
+		} finally {
 			setIsLoading(false);
 		}
 	},
 	[t, captureWithScreenshotApi],
 );

Or remove the outer try/catch since the inner one handles all cases.


54-71: Avoid mutation object in dependency array.

Including screenshotMutation in the dependency array can cause unnecessary re-renders since the mutation object reference changes on each render. Extract the stable mutateAsync function instead.

♻️ Proposed fix
+	const { mutateAsync: captureScreenshot } = useScreenshotMutation();

 	const captureWithScreenshotApi = useCallback(
 		async (websiteUrl: string) => {
 			try {
-				const dataUrl = await screenshotMutation.mutateAsync({ url: websiteUrl });
+				const dataUrl = await captureScreenshot({ url: websiteUrl });
 				onSelect(dataUrl);
 				setIsLoading(false);
 			} catch (err) {
 				setError(
 					t('errors.screenshotFailed', {
 						default:
 							'Failed to capture screenshot. The screenshot service may be unavailable or the URL is invalid.',
 					}),
 				);
 				setIsLoading(false);
 			}
 		},
-		[onSelect, t, screenshotMutation],
+		[onSelect, t, captureScreenshot],
 	);
apps/frontend/src/components/qr-generator/preview/QrPreview.tsx (1)

30-37: Consider tracking login prompt events.

The PostHog event only captures successful preview opens for signed-in users. Consider also tracking when users are prompted to sign in, which would provide insights into conversion friction.

📊 Enhanced analytics tracking
 const handleOpen = () => {
   if (!isSignedIn) {
+    posthog.capture('qr-code.preview.login-required');
     setLoginDialogOpen(true);
     return;
   }
   setOpen(true);
   posthog.capture('qr-code.preview.open');
 };
apps/frontend/src/components/qr-generator/preview/CameraCapture.tsx (2)

87-94: Consider handling initial null permission state.

The loading indicator only displays when permissionState === 'prompt', but according to the type definition, the initial state is null. This means there's a brief moment before camera initialization where no loading state is shown. Consider also checking for null:

🔄 Improved loading state check
-{!isActive && permissionState === 'prompt' && (
+{!isActive && (permissionState === 'prompt' || permissionState === null) && (
   <div className="absolute inset-0 flex items-center justify-center bg-gray-900 bg-opacity-75">

27-38: Remove the unnecessary 100ms delay in the useEffect.

The startCamera and stopCamera functions are already properly memoized with useCallback in the useCamera hook (with empty dependencies), so they remain stable across renders and won't cause effect loops. The 100ms delay, however, is unnecessary since React guarantees that effects run after the component is mounted.

⚡ Remove unnecessary delay
 useEffect(() => {
-  // Small delay to ensure component is mounted
-  const timer = setTimeout(() => {
-    startCamera();
-  }, 100);
+  startCamera();
 
   return () => {
-    clearTimeout(timer);
     stopCamera();
   };
 }, [startCamera, stopCamera]);
apps/frontend/src/components/qr-generator/preview/BackgroundSelector.tsx (1)

13-13: Track commented import as technical debt.

The PredefinedBackgrounds component is commented out with a "future use" note. Consider either:

  1. Creating a TODO/issue to track the planned feature
  2. Removing the commented code if there's no immediate plan to implement it

Commented code can become stale and confusing over time.

apps/backend/src/core/services/screenshot.service.ts (1)

14-65: Consider adding rate limiting documentation.

The comment mentions Thum.io's 1000/month free tier, but there's no rate limiting implemented in this service. Based on the PR context, rate limiting is configured at the controller level (RateLimitPolicy.SCREENSHOT_CREATE), but it would be helpful to document this dependency.

Add a comment referencing the rate limit policy:

/**
 * Captures a screenshot of a website using Thum.io
 * @param url - The website URL to screenshot
 * @returns Image buffer
 * @note Rate limiting is enforced at the controller level via RateLimitPolicy.SCREENSHOT_CREATE
 */
apps/frontend/src/components/qr-generator/preview/hooks/useCamera.ts (1)

97-122: Consider validating video dimensions before capture.

The captureFrame function should verify that the video has valid dimensions (videoWidth and videoHeight > 0) before attempting to draw to canvas. Some browsers may report 0x0 dimensions if the video hasn't fully loaded.

♻️ Proposed enhancement
 	const captureFrame = useCallback((): string | null => {
 		if (!videoRef.current || !isActive) {
 			return null;
 		}

 		try {
 			const video = videoRef.current;
+			
+			// Ensure video has valid dimensions
+			if (video.videoWidth === 0 || video.videoHeight === 0) {
+				console.warn('Video dimensions not ready');
+				return null;
+			}
+			
 			const canvas = document.createElement('canvas');

 			// Match video dimensions
 			canvas.width = video.videoWidth;
 			canvas.height = video.videoHeight;
apps/frontend/src/components/qr-generator/preview/PredefinedBackgrounds.tsx (2)

150-156: Add error handling for failed image loads.

If an Unsplash URL becomes unavailable or the CDN is down, the image will fail to load silently. Consider adding an onError handler to provide visual feedback or fallback behavior.

♻️ Proposed enhancement

Add state for failed images and an error handler:

const [failedImages, setFailedImages] = useState<Set<string>>(new Set());

const handleImageError = (imageUrl: string) => {
	setFailedImages(prev => new Set(prev).add(imageUrl));
};

Then in the image element:

 					<img
 						src={image.url}
 						alt={image.alt}
 						className="h-full w-full object-cover transition-transform group-hover:scale-105"
 						loading="lazy"
+						onError={() => handleImageError(image.url)}
 					/>
+					{failedImages.has(image.url) && (
+						<div className="absolute inset-0 flex items-center justify-center bg-gray-100">
+							<p className="text-sm text-gray-500">Failed to load image</p>
+						</div>
+					)}

19-90: Consider externalizing image URLs to configuration.

Hardcoded Unsplash URLs in the component make it harder to update or swap image sources. Consider moving these to a separate configuration file or environment variables for easier maintenance.

This would allow you to:

  • Easily update image sources without code changes
  • Support different image sets for different environments
  • Potentially load from a CMS in the future

Example structure:

// config/predefined-backgrounds.ts
export const BACKGROUND_CATEGORIES = [...];
apps/frontend/src/components/qr-generator/preview/QrPreviewModal.tsx (2)

258-265: Use functional update to avoid stale closure in onDragEnd.

The onDragEnd handler references position from the closure, which may be stale if the state changes during the drag operation.

Suggested fix
 onDragEnd={(_, info) => {
-	setPosition({
-		...position,
-		x: position.x + info.offset.x,
-		y: position.y + info.offset.y,
-		rotation: position.rotation,
-	});
+	setPosition((prev) => ({
+		...prev,
+		x: prev.x + info.offset.x,
+		y: prev.y + info.offset.y,
+	}));
 }}

215-219: Redundant null check.

Line 219 checks backgroundImage ? ... : null, but this block is already inside the condition step === 'preview' && backgroundImage on line 215, so backgroundImage is guaranteed to be truthy.

Suggested simplification
 {step === 'preview' && backgroundImage && (
 	<div className="flex flex-col gap-4">
 		{/* Preview Area - Full Width */}
 		<div className="flex-1">
-			{backgroundImage ? (
 				<div
 					className="relative flex w-full justify-center overflow-y-auto overflow-x-hidden rounded-lg bg-gray-100 dark:bg-gray-900"
 					style={{ height: 'calc(95vh - 200px)', maxHeight: '800px' }}
 				>
 					{/* ... content ... */}
 				</div>
-			) : null}
 		</div>
 	</div>
 )}
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a55714 and 4272bba.

📒 Files selected for processing (40)
  • .github/workflows/backend-test.yml
  • .gitignore
  • CLAUDE.md
  • apps/backend/src/core/interface/request.interface.ts
  • apps/backend/src/core/rate-limit/rate-limit.policy.ts
  • apps/backend/src/core/services/screenshot.service.ts
  • apps/backend/src/modules/qr-code/http/controller/qr-code.controller.ts
  • apps/frontend/content/docs/changelogs.mdx
  • apps/frontend/src/components/provider/PostHogProvider.tsx
  • apps/frontend/src/components/qr-generator/QrCodeWithDownloadBtn.tsx
  • apps/frontend/src/components/qr-generator/QrCodeWithUpdateBtn.tsx
  • apps/frontend/src/components/qr-generator/content/ContentSwitch.tsx
  • apps/frontend/src/components/qr-generator/content/EditUrlSection.tsx
  • apps/frontend/src/components/qr-generator/preview/BackgroundSelector.tsx
  • apps/frontend/src/components/qr-generator/preview/CameraCapture.tsx
  • apps/frontend/src/components/qr-generator/preview/PredefinedBackgrounds.tsx
  • apps/frontend/src/components/qr-generator/preview/QrPreview.tsx
  • apps/frontend/src/components/qr-generator/preview/QrPreviewModal.tsx
  • apps/frontend/src/components/qr-generator/preview/WebsitePreview.tsx
  • apps/frontend/src/components/qr-generator/preview/hooks/useCamera.ts
  • apps/frontend/src/components/qr-generator/preview/hooks/useQrPosition.ts
  • apps/frontend/src/components/qr-generator/preview/index.ts
  • apps/frontend/src/components/qr-generator/preview/types.ts
  • apps/frontend/src/components/qr-generator/style/ColorPicker.tsx
  • apps/frontend/src/components/qr-generator/templates/QrCodeWithTemplateUpdateBtn.tsx
  • apps/frontend/src/components/ui/dialog.tsx
  • apps/frontend/src/dictionaries/de.json
  • apps/frontend/src/dictionaries/en.json
  • apps/frontend/src/dictionaries/es.json
  • apps/frontend/src/dictionaries/fr.json
  • apps/frontend/src/dictionaries/it.json
  • apps/frontend/src/dictionaries/nl.json
  • apps/frontend/src/dictionaries/pl.json
  • apps/frontend/src/dictionaries/ru.json
  • apps/frontend/src/env.js
  • apps/frontend/src/lib/api/screenshot.ts
  • apps/frontend/src/lib/color-picker.utils.ts
  • apps/frontend/src/lib/screenshot.utils.ts
  • packages/shared/src/dtos/WebsiteScreenshotDto.ts
  • packages/shared/src/index.ts
💤 Files with no reviewable changes (1)
  • apps/frontend/src/components/qr-generator/content/EditUrlSection.tsx
🧰 Additional context used
🧬 Code graph analysis (13)
apps/frontend/src/components/qr-generator/preview/BackgroundSelector.tsx (3)
apps/frontend/src/components/qr-generator/preview/types.ts (1)
  • BackgroundSource (10-10)
apps/frontend/src/components/qr-generator/preview/index.ts (1)
  • BackgroundSource (7-7)
apps/frontend/src/components/ui/tabs.tsx (4)
  • Tabs (52-52)
  • TabsList (52-52)
  • TabsTrigger (52-52)
  • TabsContent (52-52)
apps/frontend/src/components/qr-generator/preview/CameraCapture.tsx (1)
apps/frontend/src/components/qr-generator/preview/hooks/useCamera.ts (1)
  • useCamera (11-142)
apps/frontend/src/lib/api/screenshot.ts (2)
apps/frontend/src/lib/api/ApiError.ts (1)
  • ApiError (3-13)
apps/frontend/src/lib/screenshot.utils.ts (1)
  • convertBlobToDataUrl (92-108)
apps/frontend/src/components/qr-generator/QrCodeWithUpdateBtn.tsx (3)
apps/frontend/src/components/provider/QrCodeConfigStoreProvider.tsx (1)
  • useQrCodeGeneratorStore (55-63)
apps/frontend/src/components/qr-generator/preview/QrPreview.tsx (1)
  • QrPreview (24-55)
apps/frontend/src/components/qr-generator/preview/index.ts (1)
  • QrPreview (5-5)
apps/frontend/src/components/qr-generator/templates/QrCodeWithTemplateUpdateBtn.tsx (3)
apps/frontend/src/components/provider/QrCodeConfigStoreProvider.tsx (1)
  • useQrCodeGeneratorStore (55-63)
apps/frontend/src/components/qr-generator/preview/QrPreview.tsx (1)
  • QrPreview (24-55)
apps/frontend/src/components/qr-generator/preview/index.ts (1)
  • QrPreview (5-5)
apps/frontend/src/components/qr-generator/preview/WebsitePreview.tsx (1)
apps/frontend/src/lib/api/screenshot.ts (1)
  • useScreenshotMutation (14-46)
apps/frontend/src/components/qr-generator/preview/hooks/useCamera.ts (1)
apps/frontend/src/components/qr-generator/preview/types.ts (2)
  • UseCameraReturn (31-39)
  • CameraPermissionState (15-15)
apps/frontend/src/components/provider/PostHogProvider.tsx (1)
apps/frontend/src/env.js (2)
  • env (4-67)
  • env (4-67)
apps/frontend/src/components/qr-generator/style/ColorPicker.tsx (2)
apps/frontend/src/components/ui/dialog.tsx (1)
  • DialogContent (117-117)
apps/frontend/src/components/qr-generator/style/ColorPicker.constants.ts (1)
  • COLOR_PICKER_CONFIG (4-9)
apps/frontend/src/lib/color-picker.utils.ts (1)
apps/frontend/src/lib/utils.ts (1)
  • rgbaToHex (77-123)
apps/frontend/src/components/qr-generator/preview/hooks/useQrPosition.ts (1)
apps/frontend/src/components/qr-generator/preview/types.ts (2)
  • UseQrPositionReturn (44-53)
  • QrPosition (20-26)
apps/frontend/src/components/qr-generator/preview/QrPreviewModal.tsx (5)
apps/frontend/src/components/provider/QrCodeConfigStoreProvider.tsx (1)
  • useQrCodeGeneratorStore (55-63)
apps/frontend/src/components/qr-generator/preview/types.ts (1)
  • BackgroundSource (10-10)
apps/frontend/src/components/qr-generator/preview/hooks/useQrPosition.ts (1)
  • useQrPosition (16-244)
apps/frontend/src/components/qr-generator/preview/BackgroundSelector.tsx (1)
  • BackgroundSelector (24-164)
apps/frontend/src/components/qr-generator/DynamicQrCode.tsx (1)
  • DynamicQrCode (6-9)
apps/frontend/src/components/qr-generator/preview/types.ts (1)
apps/frontend/src/components/qr-generator/preview/index.ts (1)
  • BackgroundSource (7-7)
🪛 LanguageTool
CLAUDE.md

[grammar] ~419-~419: Use a hyphen to join words.
Context: ...it tests 6. Respect plan limits and rate limiting policies 7. For dynamic content...

(QB_NEW_EN_HYPHEN)

🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md

172-172: Hard tabs
Column: 1

(MD010, no-hard-tabs)


173-173: Hard tabs
Column: 1

(MD010, no-hard-tabs)


241-241: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


376-376: Hard tabs
Column: 1

(MD010, no-hard-tabs)


377-377: Hard tabs
Column: 1

(MD010, no-hard-tabs)


378-378: Hard tabs
Column: 1

(MD010, no-hard-tabs)


379-379: Hard tabs
Column: 1

(MD010, no-hard-tabs)


380-380: Hard tabs
Column: 1

(MD010, no-hard-tabs)


381-381: Hard tabs
Column: 1

(MD010, no-hard-tabs)

🔇 Additional comments (43)
.gitignore (1)

18-18: Appropriate change to track CLAUDE.md documentation.

Removing CLAUDE.md from the ignore list enables the project guidance document to be tracked in the repository, which is appropriate for contributor onboarding and reference.

CLAUDE.md (1)

1-419: Excellent contributor guidance document.

This comprehensive CLAUDE.md file provides outstanding onboarding and reference material for working with the QRcodly codebase. It effectively covers:

  • Monorepo structure and development commands
  • Technology decisions and architecture patterns
  • Backend clean architecture with strategy pattern and dependency injection
  • Frontend state management and API client patterns
  • Critical business logic (QR code creation, reserved URL flow, dynamic content)
  • Testing structure and utilities
  • Local development setup

This will significantly improve contributor efficiency and code consistency. Once the minor formatting issues are resolved, this document is ready.

apps/frontend/src/lib/color-picker.utils.ts (3)

76-84: LGTM! Solid defensive programming.

The use of Number.isFinite correctly guards against NaN, Infinity, and -Infinity values in offsets and rotation. This ensures valid CSS gradient strings are always generated, preventing rendering issues.


120-132: LGTM! Proper input validation.

The double validation (parseFloat followed by Number.isFinite check) correctly handles malformed gradient strings. The consistent use of defaults (0 for invalid values) ensures predictable behavior when parsing user-provided or stored gradient data.


209-210: LGTM! Necessary safety net.

The regex replacement handles edge cases where NaN% might slip into gradient strings (e.g., from external sources or serialization issues). This works in tandem with the Number.isFinite validations added elsewhere to ensure robust gradient handling.

apps/frontend/src/env.js (1)

26-27: LGTM! PostHog configuration is now optional.

The changes correctly make PostHog environment variables optional, allowing the application to run without PostHog analytics enabled. This aligns well with the corresponding guard added in PostHogProvider.tsx that skips initialization when these values are absent.

apps/frontend/content/docs/changelogs.mdx (5)

11-18: LGTM! Clear and accurate release header.

The version bump to 1.6.1 and the release description accurately reflect the preview feature additions mentioned in the PR summary.


20-26: Well-documented new features.

The feature descriptions are clear, user-focused, and accurately reflect the preview functionality additions in this PR.


28-33: Preview improvements are well-articulated.

The section accurately describes the UI/UX enhancements for the new preview functionality.


55-105: Good practice preserving release history.

Moving the complete v1.6.0 release notes to the Previous Releases section maintains proper documentation history.


35-39: The supported languages list in the changelog is accurate. The codebase confirms 8 supported languages (English, German, Spanish, French, Italian, Dutch, Polish, Russian) via the SUPPORTED_LANGUAGES array in apps/frontend/src/i18n/routing.ts and matching dictionary files in apps/frontend/src/dictionaries/. No changes needed.

apps/frontend/src/components/ui/dialog.tsx (1)

44-48: Clean implementation of the optional overlay feature.

The showOverlay prop is well-implemented with proper TypeScript typing and a sensible default value that maintains backward compatibility for existing dialogs.

apps/frontend/src/components/qr-generator/style/ColorPicker.tsx (1)

75-75: Verify color picker accessibility without overlay.

The showOverlay={false} prop is correctly applied. However, ensure this design choice works well for all users, particularly regarding focus management and keyboard navigation. See the accessibility concerns raised in apps/frontend/src/components/ui/dialog.tsx regarding the overlay suppression pattern.

Consider testing:

  • Keyboard navigation (Tab, Escape, Enter)
  • Screen reader announcements
  • Visual clarity when the color picker overlaps page content
.github/workflows/backend-test.yml (1)

51-55: LGTM! Proper build order for monorepo dependencies.

The workflow correctly builds the shared package before running backend checks, ensuring the new WebsiteScreenshotDto is available during testing.

packages/shared/src/index.ts (1)

26-26: LGTM! Consistent DTO export.

The export follows the established pattern for shared DTOs.

apps/backend/src/core/interface/request.interface.ts (1)

5-14: LGTM! Improved type parameter naming.

Renaming from single-letter type parameters (B, P, Q) to descriptive names (Body, Params, Query) significantly improves code readability without changing functionality.

apps/frontend/src/lib/screenshot.utils.ts (2)

10-36: LGTM! Robust URL validation.

The validation correctly checks for empty input, invalid format, and non-HTTP/HTTPS protocols with clear error messages.


92-107: LGTM! Clean Blob to data URL conversion.

The implementation correctly uses FileReader with proper error handling for both load and read failures.

apps/frontend/src/components/qr-generator/preview/hooks/useQrPosition.ts (1)

109-119: Verify boundary constraint logic when QR position is outside bounds.

The maxAllowedSize calculation on lines 116-117 uses boundary.width - prev.x and boundary.height - prev.y to constrain the QR size. If the QR element is dragged outside the boundary (i.e., prev.x < 0 or prev.x > boundary.width), this could result in incorrect size constraints.

For example:

  • If prev.x is negative, boundary.width - prev.x becomes larger than intended.
  • If prev.x > boundary.width, the constraint becomes negative or very small.

Consider adding position clamping or validation to ensure prev.x and prev.y are always within [0, boundary.width] and [0, boundary.height] respectively, or adjust the boundary calculation to handle out-of-bounds positions gracefully.

apps/frontend/src/components/qr-generator/QrCodeWithUpdateBtn.tsx (1)

5-5: LGTM! Clean integration of QrPreview component.

The QrPreview component is properly imported and rendered with appropriate props. The placement before the UpdateQrCodeBtn provides a logical user flow for previewing before updating.

Also applies to: 23-23

apps/frontend/src/dictionaries/fr.json (1)

498-569: LGTM! Comprehensive French localization for preview feature.

The new preview section provides complete French translations for the QR code background preview feature. The structure is well-organized with logical nesting for different preview modes (predefined, website, camera, upload) and UI elements (contrast, controls).

Note: Translation accuracy review is outside the scope of this technical review. If you have concerns about specific French translations, consider having them reviewed by a native speaker.

apps/frontend/src/dictionaries/en.json (1)

498-569: LGTM! Well-structured English localization for preview feature.

The new preview section provides comprehensive English translations for the QR code background preview feature. The text is clear, user-friendly, and includes helpful error messages and instructions for different preview modes.

The organization is logical with appropriate nesting for:

  • Predefined background categories
  • Website preview with detailed error handling
  • Camera capture workflow
  • File upload instructions
  • Contrast visibility levels
  • Interactive controls
apps/frontend/src/components/qr-generator/templates/QrCodeWithTemplateUpdateBtn.tsx (1)

5-5: LGTM! Consistent QrPreview integration.

The QrPreview component is properly integrated, matching the pattern used in QrCodeWithUpdateBtn.tsx. This consistency ensures a uniform user experience across both QR code and template update workflows.

Also applies to: 23-23

apps/frontend/src/components/qr-generator/preview/index.ts (1)

1-7: LGTM!

Clean barrel export file with appropriate component and type exports for the preview module.

apps/frontend/src/components/qr-generator/QrCodeWithDownloadBtn.tsx (1)

8-8: LGTM!

Clean integration of the QrPreview component with appropriate styling props. Logical placement before the download button in the non-bulk mode flow.

Also applies to: 28-28

apps/frontend/src/dictionaries/pl.json (1)

499-569: LGTM!

Comprehensive Polish translations for the preview feature. The structure matches the component requirements and is consistent with other language dictionary files.

apps/frontend/src/dictionaries/ru.json (1)

499-569: LGTM!

Complete Russian translations for the preview feature with proper structure matching the component requirements.

apps/backend/src/core/rate-limit/rate-limit.policy.ts (1)

8-8: LGTM!

Sensible rate limiting for the screenshot feature. Blocking anonymous users (0) is appropriate for this resource-intensive operation, and the tiered limits (3 for authenticated, 10 for pro) align with the existing policy patterns.

Also applies to: 46-50

apps/frontend/src/components/qr-generator/preview/WebsitePreview.tsx (1)

27-30: Verify z.httpUrl() availability in Zod.

Same as noted in the backend DTO—z.httpUrl() may not be a standard Zod 4 API. Ensure this method exists or use z.string().url() with additional validation if needed.

packages/shared/src/dtos/WebsiteScreenshotDto.ts (1)

3-5: No changes needed. The z.httpUrl() method is a valid, documented API in Zod 4 for validating HTTP/HTTPS URLs. The implementation in WebsiteScreenshotDtoSchema is correct.

Likely an incorrect or invalid review comment.

apps/frontend/src/components/qr-generator/preview/QrPreview.tsx (2)

1-23: LGTM!

The imports are well-organized and the QrPreviewProps interface is properly typed with sensible defaults for the button variants and sizes.


45-45: Verify mobile UX: preview button is hidden on small screens.

The button uses hidden sm:inline-flex, which hides the preview feature on mobile devices. Ensure this is intentional, as mobile users scanning QR codes might benefit from the preview functionality.

apps/frontend/src/dictionaries/es.json (1)

498-569: LGTM!

The Spanish translations for the preview feature are well-structured and comprehensive. The nested organization mirrors the component structure, covering all preview modes (website, camera, upload) and UI elements (controls, contrast indicators).

apps/frontend/src/components/qr-generator/preview/types.ts (1)

1-53: LGTM!

The type definitions are well-structured and thoroughly documented with JSDoc comments. The interfaces appropriately model the camera hook, position hook, and background source types for the preview feature.

apps/frontend/src/components/qr-generator/preview/BackgroundSelector.tsx (1)

82-163: LGTM!

The tab structure is well-implemented with proper icons, translations, and animations. The drag-and-drop zone in the upload tab follows good UX patterns with visual feedback via the isDragging state.

apps/frontend/src/components/qr-generator/content/ContentSwitch.tsx (2)

173-186: LGTM - Default behavior is appropriate.

The default to true when isEditable is undefined aligns with the localStorage default and provides a sensible fallback for new users.


188-201: LGTM - Consistent default behavior.

The default to true for isDynamic maintains consistency with the URL handling and matches user expectations.

apps/frontend/src/components/qr-generator/preview/hooks/useCamera.ts (1)

11-142: LGTM - Well-structured camera management hook.

The hook properly handles camera lifecycle, permissions, and cleanup. The error handling covers different failure scenarios with appropriate user-facing messages. Good use of React hooks and refs for managing stream lifecycle.

apps/frontend/src/components/qr-generator/preview/PredefinedBackgrounds.tsx (1)

92-186: LGTM - Well-implemented component with good UX.

The component provides a clear, accessible interface for background selection. Good use of Framer Motion for smooth animations, proper ARIA attributes for accessibility, and responsive grid layout. The visual feedback for selection is intuitive.

apps/frontend/src/dictionaries/nl.json (1)

498-569: Translation structure looks complete.

The new preview translations are well-organized and cover all necessary UI elements (website, camera, upload, contrast, controls). The structure is consistent with the rest of the file and follows the same naming conventions.

However, since I cannot verify Dutch translation accuracy, please ensure a native Dutch speaker reviews the translations for:

  • Grammar and proper phrasing
  • Consistency with existing Dutch UI terminology
  • Technical term accuracy (e.g., "Screenshot vastleggen", "Cameratoegang")
apps/frontend/src/components/qr-generator/preview/QrPreviewModal.tsx (1)

44-50: LGTM!

The localStorage access is properly guarded with typeof window !== 'undefined' for SSR safety, and using it as a state initializer function is the correct pattern.

apps/frontend/src/dictionaries/it.json (1)

499-569: LGTM!

The Italian translations for the preview feature are well-structured and consistent with the existing translation patterns. The nested structure for predefined, website, camera, upload, contrast, and controls sections is complete.

apps/frontend/src/dictionaries/de.json (1)

499-569: LGTM!

The German translations for the preview feature are complete and follow the same structure as other language files. The translations are natural and grammatically correct.

Comment on lines +14 to +21
async captureWebsite(url: string): Promise<Buffer> {
this.logger.info('screenshot.take', { url });

try {
// Using Thum.io (1000/month free, no API key required)
// Note: Thum.io expects the URL NOT to be encoded - just append it directly
// Removed crop parameter to capture full page height
const screenshotUrl = `https://image.thum.io/get/width/1200/noanimate/${url}`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Missing URL validation and SSRF protection.

The URL parameter is passed directly to the external service without validation. This creates security risks:

  1. SSRF vulnerability: An attacker could potentially use this to probe internal network resources by passing URLs like http://localhost:6379 or http://192.168.1.1/admin.
  2. No protocol validation: The service accepts any URL without verifying it uses http/https.
  3. No allowlist/denylist: Internal IP ranges and localhost should be blocked.
🔒 Recommended fix with URL validation

Add URL validation before calling the external service:

async captureWebsite(url: string): Promise<Buffer> {
	this.logger.info('screenshot.take', { url });

	// Validate URL format
	let parsedUrl: URL;
	try {
		parsedUrl = new URL(url);
	} catch (e) {
		throw new BadRequestError('Invalid URL format');
	}

	// Only allow http/https protocols
	if (!['http:', 'https:'].includes(parsedUrl.protocol)) {
		throw new BadRequestError('Only HTTP and HTTPS protocols are allowed');
	}

	// Block internal/private IP addresses (SSRF protection)
	const hostname = parsedUrl.hostname;
	const blockedPatterns = [
		/^localhost$/i,
		/^127\./,
		/^10\./,
		/^172\.(1[6-9]|2[0-9]|3[0-1])\./,
		/^192\.168\./,
		/^169\.254\./,
		/^::1$/,
		/^fe80:/i,
		/^fc00:/i,
	];

	if (blockedPatterns.some(pattern => pattern.test(hostname))) {
		throw new BadRequestError('Internal URLs are not allowed');
	}

	try {
		// ... rest of existing code
🤖 Prompt for AI Agents
In @apps/backend/src/core/services/screenshot.service.ts around lines 14 - 21,
captureWebsite is currently passing the raw url into the external service
(screenshotUrl) creating SSRF and protocol issues; before building
screenshotUrl, validate and parse the incoming url in captureWebsite: use URL
parsing to reject malformed URLs, enforce only http: and https: protocols, and
block localhost or private/internal IP hostnames (e.g., patterns for localhost,
127.*, 10.*, 172.16-31.*, 192.168.*, ::1, fe80:, fc00:, etc.); if validation
fails throw a BadRequestError (or existing equivalent) and only then construct
screenshotUrl and proceed with the request while keeping
this.logger.info('screenshot.take', { url }) as-is.

Comment on lines +24 to +28
const response = await fetch(screenshotUrl, {
headers: {
'User-Agent': 'QRcodly/1.0',
},
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add timeout to prevent hanging requests.

The fetch call has no timeout configured. If the external service is slow or unresponsive, this could tie up server resources indefinitely.

⏱️ Proposed fix with timeout
+		// Set a timeout for the request (e.g., 30 seconds)
+		const controller = new AbortController();
+		const timeoutId = setTimeout(() => controller.abort(), 30000);
+
 		const response = await fetch(screenshotUrl, {
 			headers: {
 				'User-Agent': 'QRcodly/1.0',
 			},
+			signal: controller.signal,
 		});
+		
+		clearTimeout(timeoutId);

And update the catch block to handle AbortError:

 	} catch (e: any) {
 		const error = e as Error;
+		if (error.name === 'AbortError') {
+			this.logger.error('screenshot.timeout', { url });
+			throw new BadRequestError('Screenshot request timed out');
+		}
 		this.logger.error('screenshot.error', {

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In @apps/backend/src/core/services/screenshot.service.ts around lines 24 - 28,
The fetch in screenshot.service.ts that calls fetch(screenshotUrl, { headers: {
'User-Agent': 'QRcodly/1.0' } }) needs an AbortController timeout to avoid
hanging: create an AbortController, pass its signal to fetch, start a setTimeout
that calls controller.abort() after a sensible timeout (e.g., 5–10s) and clear
the timeout on success; update the fetch invocation to include signal and ensure
the response variable is used as before; and update the catch block to
specifically handle an AbortError (e.name === 'AbortError') so aborts are
handled distinctly from other errors.

Comment on lines +57 to +64
} catch (e: any) {
const error = e as Error;
this.logger.error('screenshot.error', {
url,
error: error.message,
});
throw new BadRequestError(`Failed to capture screenshot: ${error.message}`);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Incorrect error classification for all failures.

All errors are wrapped as BadRequestError (400), but many failure scenarios are actually server errors (5xx) or service errors. For example:

  • Network failures should be 503 Service Unavailable
  • Timeout should be 504 Gateway Timeout
  • External service errors (non-200 responses) should be 502 Bad Gateway

Only invalid input (malformed URLs, blocked domains) should be 400.

Consider creating separate error types or at least differentiating in the catch block:

} catch (e: any) {
	const error = e as Error;
	this.logger.error('screenshot.error', { url, error: error.message });
	
	if (error.name === 'AbortError') {
		throw new ServiceUnavailableError('Screenshot service timed out');
	}
	if (error.message.includes('Screenshot service returned')) {
		throw new ServiceUnavailableError(`External service error: ${error.message}`);
	}
	// Only user input errors should be BadRequestError
	throw new BadRequestError(`Invalid screenshot request: ${error.message}`);
}
🤖 Prompt for AI Agents
In @apps/backend/src/core/services/screenshot.service.ts around lines 57 - 64,
The catch block in the screenshot capture flow currently wraps every error as
BadRequestError; update the catch in the method containing the shown catch (the
screenshot service method that calls this.logger.error) to map different failure
conditions to appropriate HTTP error types instead of always throwing
BadRequestError: keep the this.logger.error call but inspect the caught error
(e.g., error.name, error.message, or any response/status fields) and throw
ServiceUnavailableError (503) for network failures/timeouts, GatewayTimeoutError
(504) for request timeouts/AbortError, BadGatewayError (502) for upstream
non-200 responses, and only throw BadRequestError for genuine input/URL
validation problems; ensure the thrown errors use clear messages and
preserve/log the original error details.

Comment on lines +304 to +312
@Get('/screenshot', {
querySchema: WebsiteScreenshotDtoSchema,
config: {
rateLimitPolicy: RateLimitPolicy.SCREENSHOT_CREATE,
},
schema: {
hide: true,
},
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Route ordering issue: /screenshot may conflict with /:id routes.

The /screenshot route is defined after /:id/download and other /:id routes. Depending on how the routing framework registers these routes, a request to /qr-code/screenshot might incorrectly match /:id with id="screenshot", causing a 404 or unexpected behavior.

Move this route definition before the /:id routes (before line 155) to ensure it takes precedence.

Suggested route placement

The /screenshot endpoint should be placed before any routes with path parameters like /:id. Consider moving lines 303-328 to appear before line 155 (the @Get('/:id', ...) decorator).

🤖 Prompt for AI Agents
In @apps/backend/src/modules/qr-code/http/controller/qr-code.controller.ts
around lines 304 - 312, The /screenshot route decorator block (the
@Get('/screenshot', { querySchema: WebsiteScreenshotDtoSchema, ... }) method)
must be moved so it is registered before any parameterized routes like
@Get('/:id', ...) and @Get('/:id/download', ...) to avoid path collision where
"screenshot" is treated as an :id; relocate the entire screenshot route
(decorator and its handler method) to a position above the first @Get('/:id'...)
route declaration (i.e., before the controller methods handling :id paths) so
the static /screenshot route takes precedence.

Comment on lines +313 to +318
async screenshot(
request: IHttpRequest<unknown, unknown, TWebsiteScreenshotDto>,
): Promise<IHttpResponse<Buffer>> {
const { url } = request.query;

const imageBuffer = await this.screenshotService.captureWebsite(url);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "*.ts" -o -name "*.js" | xargs grep -l "WebsiteScreenshotDto" | head -20

Repository: FloB95/qrcodly

Length of output: 212


🏁 Script executed:

cat ./packages/shared/src/dtos/WebsiteScreenshotDto.ts

Repository: FloB95/qrcodly

Length of output: 240


🏁 Script executed:

fd -type f -name "*screenshot*service*" | head -10

Repository: FloB95/qrcodly

Length of output: 228


🏁 Script executed:

fd --type f "screenshot.*service" | head -10

Repository: FloB95/qrcodly

Length of output: 110


🏁 Script executed:

cat apps/backend/src/core/services/screenshot.service.ts

Repository: FloB95/qrcodly

Length of output: 2058


Implement SSRF protection before calling Thum.io service.

The endpoint passes user-provided URLs directly to the Thum.io external service without any validation. This creates a server-side request forgery (SSRF) vulnerability. Validate URLs client-side before passing to the screenshot service:

  1. Block private IP ranges: 127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16
  2. Block localhost and internal hostnames: localhost, *.local, *.internal, *.test
  3. Block metadata services: 0.0.0.0, 255.255.255.255, AWS/GCP/Azure metadata endpoints

Add validation in WebsiteScreenshotDtoSchema or in a middleware/validator that checks the URL's resolved IP address against a blocklist before allowing the screenshot service to process it.

return (
<DialogPortal data-slot="dialog-portal">
<DialogOverlay />
{showOverlay && <DialogOverlay />}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for showOverlay usage across the frontend
rg -n "showOverlay" --type tsx --type ts -C 3 apps/frontend/src/

Repository: FloB95/qrcodly

Length of output: 85


🏁 Script executed:

#!/bin/bash
# Examine the dialog.tsx file to understand the implementation
cat -n apps/frontend/src/components/ui/dialog.tsx | head -70

Repository: FloB95/qrcodly

Length of output: 3137


🏁 Script executed:

#!/bin/bash
# Check if there's any documentation or comments about accessibility in the dialog component
rg -n "accessibility|focus|modal|WCAG|a11y" apps/frontend/src/components/ui/dialog.tsx

Repository: FloB95/qrcodly

Length of output: 469


🏁 Script executed:

#!/bin/bash
# Search for showOverlay usage across the codebase
rg -n "showOverlay" apps/frontend/src/ -C 3

Repository: FloB95/qrcodly

Length of output: 2573


🏁 Script executed:

#!/bin/bash
# Also check if there are any TypeScript/JavaScript files using this dialog
fd -e "jsx" -e "js" -e "tsx" -e "ts" apps/frontend/src/ | head -20

Repository: FloB95/qrcodly

Length of output: 435


🏁 Script executed:

#!/bin/bash
# Read the ColorPicker component to see the full implementation
cat -n apps/frontend/src/components/qr-generator/style/ColorPicker.tsx | head -90

Repository: FloB95/qrcodly

Length of output: 3498


🏁 Script executed:

#!/bin/bash
# Check for any accessibility attributes or focus management in ColorPicker
rg -n "focus|aria|role|tabIndex" apps/frontend/src/components/qr-generator/style/ColorPicker.tsx -C 2

Repository: FloB95/qrcodly

Length of output: 601


🌐 Web query:

Radix UI Dialog focus trapping mechanism overlay

💡 Result:

Short answer — Radix Dialog traps focus using its internal FocusScope when modal={true}, which keeps keyboard focus inside the Dialog.Content (and blocks page tabbing/scroll). The Overlay is part of the Portal and visually covers the page while the FocusScope enforces the trap and restores focus to the trigger on close. [1]

Common behaviors and edge-cases

  • Modal dialogs: focus is automatically trapped; Esc restores focus to the trigger. [1]
  • If you mount/unmount dialogs without using a Trigger or when using Shadow DOM, the trap can persist or behave unexpectedly (reported issues). [2][4]
  • Force-mounting content (rendering it always) can also lock scroll and trap focus if modal is true. [3]

Workarounds

  • Disable Radix’s trap for a Content subtree by wrapping it in @radix-ui/react-focus-scope and setting trapped={false}, then implement your own focus handling where needed. This is the recommended approach for multi-dialog or special-host scenarios. Example pattern shown in docs/third‑party guides. [5][6]
  • Alternately set modal={false} on Dialog.Root and manage scroll/focus/overlay behavior yourself (used when Shadow DOM or custom focus logic is required). [4][3]
  • To prevent closing when clicking the overlay, use onPointerDownOutside and call event.preventDefault(). [5]

If you want, I can:

  • show a minimal code example of wrapping Dialog.Content with FocusScope trapped={false}, or
  • inspect a specific bug in your app (paste code) and suggest a tailored fix.

🌐 Web query:

Radix UI react-dialog focus management without overlay

💡 Result:

Short answer — you can't safely remove the Overlay and expect Radix's built‑in modal focus behaviour to keep working unchanged. Radix traps focus inside Dialog.Content via an internal FocusScope and the Overlay/Content pairing controls pointer-events and blocking behavior. Removing the Overlay commonly breaks pointer-events and focus trapping. See the docs and issues. [1][2][3][4]

Options (pick one):

  1. Keep an Overlay but make it invisible
  • Render Dialog.Overlay so Radix still has the element that enforces pointer-events and stacking, but style it transparent/hidden visually:
    <Dialog.Overlay style={{ background: 'transparent' }} />
  • This preserves Radix's focus trap and outside-pointer blocking while showing no backdrop. (Docs: Dialog uses an Overlay + Content.) [1]
  1. Disable Radix modal trapping and implement your own
  • Use <Dialog.Root modal={false}> to turn off Radix's modal mode (no automatic focus trap / page blocking), then use your own focus-containment (e.g. Radix's FocusScope or a library like focus-trap-react / react-aria FocusScope) to implement the exact behavior you want. This avoids pointer-events side effects. (Radix focus-trapping is implemented by a FocusScope internal utility; people have requested a prop to disable/customize it.) [1][4][3]
  1. Workarounds for 3rd-party or Shadow DOM cases
  • If you need to allow other modals or third‑party UI to receive focus (or you portal into shadow DOM), prefer option (2). There are multiple Radix issues describing pointer-events and focus-trap conflicts when Content disables outside pointer events or when multiple/dialogs/Shadow DOM are involved. [2][5]

Minimal example — invisible overlay (option 1):
<Dialog.Root>
<Dialog.Trigger>Open</Dialog.Trigger>
<Dialog.Portal>
<Dialog.Overlay style={{ background: 'transparent' }} />
<Dialog.Content>...</Dialog.Content>
</Dialog.Portal>
</Dialog.Root>

Minimal example — custom focus (option 2, using modal={false}):
<Dialog.Root modal={false}>
<Dialog.Trigger>Open</Dialog.Trigger>
<Dialog.Portal>
<Dialog.Content>
{/* content */}
</Dialog.Content>
</Dialog.Portal>
</Dialog.Root>

References

  • Radix Dialog docs (modal vs non-modal, Overlay/Content anatomy). [1]
  • Issue: DialogContent disables pointer events for whole page (discussion & suggested prop). [2]
  • Issue: request to allow disabling/customizing focus trap. [3]
  • Notes / examples showing Dialog.Content uses FocusScope internally. [4]

If you want, tell me which approach you prefer and I’ll give a ready-to-drop code example for your stack (React version, CSS-in-JS or Tailwind, Shadow DOM or not).


Fix focus trapping behavior when the overlay is suppressed.

Conditionally rendering DialogOverlay breaks Radix UI Dialog's focus management. The overlay isn't purely visual—it works with FocusScope to trap focus and manage pointer-events. When showOverlay={false}, keyboard users may be able to tab out of the dialog.

Two safe approaches:

  1. Keep an invisible overlay (preserves focus trapping automatically):

    {showOverlay && <DialogOverlay />}
    // becomes:
    <DialogOverlay style={{ background: 'transparent' }} />
  2. Disable modal mode and implement custom focus management (for floating dialogs):

    <Dialog modal={false}>
      {/* Use FocusScope or custom focus trap instead */}
    </Dialog>

The color picker use case may justify option 2, but ensure focus containment and pointer-events behavior remain correct.

Also applies to: 57-57

🤖 Prompt for AI Agents
In @apps/frontend/src/components/ui/dialog.tsx at line 51, The conditional
render of DialogOverlay when using showOverlay breaks Radix Dialog's focus
trapping; either always render DialogOverlay but make it visually transparent
when showOverlay is false (so FocusScope/pointer-event behavior is preserved) or
switch the Dialog to non-modal (Dialog modal={false}) and add an explicit
FocusScope/focus-trap to the dialog content; update the usage around the
DialogOverlay and Dialog components (references: DialogOverlay, showOverlay,
Dialog modal prop) to implement one of these approaches so keyboard focus cannot
escape the dialog.

Comment on lines +7 to +9
interface ScreenshotRequestDto {
url: string;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use the shared DTO instead of local interface.

The local ScreenshotRequestDto interface duplicates the shared WebsiteScreenshotDto from packages/shared. Use the shared DTO for consistency and type safety.

♻️ Proposed refactor to use shared DTO
+import { TWebsiteScreenshotDto } from '@shared/schemas';
-
-interface ScreenshotRequestDto {
-	url: string;
-}

Then update the mutation function signature:

-	mutationFn: async (dto: ScreenshotRequestDto): Promise<string> => {
+	mutationFn: async (dto: TWebsiteScreenshotDto): Promise<string> => {

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In @apps/frontend/src/lib/api/screenshot.ts around lines 7 - 9, Replace the
local ScreenshotRequestDto with the shared WebsiteScreenshotDto from
packages/shared: remove the local interface declaration for
ScreenshotRequestDto, import WebsiteScreenshotDto and use it everywhere the
local type was referenced (e.g., in the mutation function signature that
currently accepts ScreenshotRequestDto and any handlers/call sites in this
file), and update the mutation/function parameter types to WebsiteScreenshotDto
to ensure consistent types across the codebase.

Comment on lines +18 to +31
mutationFn: async (dto: ScreenshotRequestDto): Promise<string> => {
const token = await getToken();

// We need to handle the blob response differently from JSON
// So we use fetch directly but follow the same error pattern as apiRequest
const response = await fetch(
`${env.NEXT_PUBLIC_API_URL}/qr-code/screenshot?url=${encodeURIComponent(dto.url)}`,
{
method: 'GET',
headers: {
Authorization: `Bearer ${token}`,
},
},
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Consider POST method and request body to avoid URL length limitations.

Using GET with URL query parameters for screenshot capture has several issues:

  1. Semantic incorrectness: Screenshot capture is a mutation/action that should use POST, not GET
  2. URL length limits: URLs can be very long, and query parameter encoding may hit browser/server limits (typically ~2048 characters)
  3. Caching concerns: GET requests may be cached by proxies/CDNs, which is undesirable for mutations
🔧 Recommended fix to use POST with request body
 const response = await fetch(
-	`${env.NEXT_PUBLIC_API_URL}/qr-code/screenshot?url=${encodeURIComponent(dto.url)}`,
+	`${env.NEXT_PUBLIC_API_URL}/qr-code/screenshot`,
 	{
-		method: 'GET',
+		method: 'POST',
 		headers: {
 			Authorization: `Bearer ${token}`,
+			'Content-Type': 'application/json',
 		},
+		body: JSON.stringify(dto),
 	},
 );

Note: This change requires updating the backend endpoint to accept POST requests.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mutationFn: async (dto: ScreenshotRequestDto): Promise<string> => {
const token = await getToken();
// We need to handle the blob response differently from JSON
// So we use fetch directly but follow the same error pattern as apiRequest
const response = await fetch(
`${env.NEXT_PUBLIC_API_URL}/qr-code/screenshot?url=${encodeURIComponent(dto.url)}`,
{
method: 'GET',
headers: {
Authorization: `Bearer ${token}`,
},
},
);
mutationFn: async (dto: ScreenshotRequestDto): Promise<string> => {
const token = await getToken();
// We need to handle the blob response differently from JSON
// So we use fetch directly but follow the same error pattern as apiRequest
const response = await fetch(
`${env.NEXT_PUBLIC_API_URL}/qr-code/screenshot`,
{
method: 'POST',
headers: {
Authorization: `Bearer ${token}`,
'Content-Type': 'application/json',
},
body: JSON.stringify(dto),
},
);
🤖 Prompt for AI Agents
In @apps/frontend/src/lib/api/screenshot.ts around lines 18 - 31, The mutationFn
currently sends the screenshot URL as a GET query which risks URL length/caching
and is semantically a mutation; change the fetch to use POST to
`${env.NEXT_PUBLIC_API_URL}/qr-code/screenshot`, include Authorization and
Content-Type: application/json headers, send JSON.stringify({ url: dto.url }) in
the request body (keeping use of getToken() and ScreenshotRequestDto), and
update the backend /qr-code/screenshot handler to accept POST with a JSON body
and return the blob as before.

Comment on lines +44 to +85
export async function convertImageUrlToDataUrl(imageUrl: string): Promise<string> {
return new Promise((resolve, reject) => {
const img = new Image();

// Set crossOrigin to anonymous to enable CORS
img.crossOrigin = 'anonymous';

img.onload = () => {
try {
// Create canvas and draw image
const canvas = document.createElement('canvas');
canvas.width = img.naturalWidth || img.width;
canvas.height = img.naturalHeight || img.height;

const ctx = canvas.getContext('2d');
if (!ctx) {
reject(new Error('Failed to get canvas context'));
return;
}

ctx.drawImage(img, 0, 0);

// Convert to data URL
const dataUrl = canvas.toDataURL('image/png');
resolve(dataUrl);
} catch (error) {
reject(
new Error(
`Failed to convert image to data URL: ${error instanceof Error ? error.message : 'Unknown error'}`,
),
);
}
};

img.onerror = () => {
reject(new Error('Failed to load image from URL'));
};

// Start loading the image
img.src = imageUrl;
});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Consider adding image size constraints to prevent memory exhaustion.

The function loads and converts images without size validation, which could cause excessive memory consumption or browser crashes with very large images. Canvas operations also have browser-specific size limits (typically 4096-8192px per dimension).

🛡️ Proposed fix with size constraints
 export async function convertImageUrlToDataUrl(imageUrl: string): Promise<string> {
 	return new Promise((resolve, reject) => {
 		const img = new Image();
+		const MAX_DIMENSION = 4096; // Conservative limit for canvas
 
 		// Set crossOrigin to anonymous to enable CORS
 		img.crossOrigin = 'anonymous';
 
 		img.onload = () => {
 			try {
+				// Check image dimensions
+				const width = img.naturalWidth || img.width;
+				const height = img.naturalHeight || img.height;
+				
+				if (width > MAX_DIMENSION || height > MAX_DIMENSION) {
+					reject(new Error(
+						`Image dimensions (${width}x${height}) exceed maximum allowed size (${MAX_DIMENSION}x${MAX_DIMENSION})`
+					));
+					return;
+				}
+				
 				// Create canvas and draw image
 				const canvas = document.createElement('canvas');
-				canvas.width = img.naturalWidth || img.width;
-				canvas.height = img.naturalHeight || img.height;
+				canvas.width = width;
+				canvas.height = height;
 
 				const ctx = canvas.getContext('2d');
 				if (!ctx) {
 					reject(new Error('Failed to get canvas context'));
 					return;
 				}
 
 				ctx.drawImage(img, 0, 0);
 
 				// Convert to data URL
 				const dataUrl = canvas.toDataURL('image/png');
 				resolve(dataUrl);
 			} catch (error) {
 				reject(
 					new Error(
 						`Failed to convert image to data URL: ${error instanceof Error ? error.message : 'Unknown error'}`,
 					),
 				);
 			}
 		};
 
 		img.onerror = () => {
 			reject(new Error('Failed to load image from URL'));
 		};
 
 		// Start loading the image
 		img.src = imageUrl;
 	});
 }
🤖 Prompt for AI Agents
In @apps/frontend/src/lib/screenshot.utils.ts around lines 44 - 85, The
convertImageUrlToDataUrl function currently loads arbitrary-sized images; add
explicit size checks and safe downscaling to avoid memory exhaustion: after
img.onload read img.naturalWidth/naturalHeight, compare them against a defined
MAX_DIMENSION (e.g., 4096 or configurable) and a MAX_PIXELS cap (e.g.,
MAX_DIMENSION*MAX_DIMENSION); if either limit is exceeded either reject with a
clear error or compute scaledWidth/scaledHeight preserving aspect ratio and set
canvas.width/canvas.height to the scaled values before drawing; ensure the error
paths use the existing reject flow and keep crossOrigin/ctx handling unchanged
so oversized images are handled safely.

Comment thread CLAUDE.md
Comment on lines +172 to +173
free: { url: 10, text: 5, event: 0 },
pro: { url: null, text: null, event: 100 },
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix markdown formatting and grammar issues.

Static analysis identified several minor formatting issues in the documentation:

  1. Hard tabs (lines 172–173, 376–381): Replace with spaces for consistency with markdown standards.
  2. Missing code block language identifier (line 241): Add language specifier (e.g., typescript) to the fenced code block.
  3. Grammar (line 419): Compound adjective should be hyphenated: "rate-limiting policies" instead of "rate limiting policies."
✏️ Proposed fixes

Line 172–173 (hard tabs → spaces):

 QR_CODE_PLAN_LIMITS = {
-	free: { url: 10, text: 5, event: 0 },
-	pro: { url: null, text: null, event: 100 },
+	  free: { url: 10, text: 5, event: 0 },
+	  pro: { url: null, text: null, event: 100 },
 };

Line 241 (add language identifier):

-\`\`\`
+\`\`\`ts
 src/components/
 ├── qr-generator/        # QR generation feature

Line 376–381 (hard tabs → spaces):

 \`\`\`typescript
 const mutation = useCreateQrCode();
 mutation.mutate(dto, {
-	onSuccess: (data) => {
-		/* ... */
-	},
-	onError: (error) => {
-		/* ... */
-	},
+	  onSuccess: (data) => {
+	    /* ... */
+	  },
+	  onError: (error) => {
+	    /* ... */
+	  },
 });
 \`\`\`

Line 419 (grammar):

-6. Respect plan limits and rate limiting policies
+6. Respect plan limits and rate-limiting policies

Also applies to: 241-241, 376-381, 419-419

🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

172-172: Hard tabs
Column: 1

(MD010, no-hard-tabs)


173-173: Hard tabs
Column: 1

(MD010, no-hard-tabs)

🤖 Prompt for AI Agents
In @CLAUDE.md around lines 172 - 173, Replace hard tabs with spaces in the
markdown around the entries containing "free: { url: 10, text: 5, event: 0 }"
and "pro: { url: null, text: null, event: 100 }" and the other block with
similar tab indentation so all indentation uses spaces; add a fenced code block
language identifier (e.g., "```typescript") to the unnamed fenced block that
begins near the sample code snippet shown in the file; and correct the phrase
"rate limiting policies" to the hyphenated form "rate-limiting policies"
wherever it appears.

@FloB95 FloB95 merged commit 7878860 into main Jan 8, 2026
6 of 7 checks passed
@FloB95 FloB95 deleted the chore/add-preview-modal branch January 21, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Simple change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant