Conversation
Replace the muted style-picker icons with a dedicated StyleGridPicker and a pure-SVG DotStylePreviewTile that renders a real QR-code excerpt per dot style. Unify section labels via FormLabel inside FormItem and add a compact mode for the corner-style grids so their tiles stay proportional next to the dot picker. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wraps the right-side QR output in a sticky container so the live preview, download buttons and "Save as template" stay visible while the long content forms (e.g. vCard) scroll. Drops the page-level Container's overflow-hidden so position: sticky is not trapped by an ancestor scroll context. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughThis PR enhances the QR code generator's user interface by introducing new style selection components and improving layout behavior. The changes include adding a sticky QR output container, creating reusable grid-based style picker and preview tile components, and integrating them into the settings form with responsive design patterns. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/frontend/src/components/qr-generator/style/DotStylePreviewTile.tsx (1)
55-59: Consider usingreplaceChildren()for cleanup.The while-loop cleanup works, but
replaceChildren()is more concise and equally performant.♻️ Proposed simplification
return () => { - while (node.firstChild) { - node.removeChild(node.firstChild); - } + node.replaceChildren(); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/components/qr-generator/style/DotStylePreviewTile.tsx` around lines 55 - 59, In the cleanup returned from the DotStylePreviewTile component's effect (the anonymous function that currently removes children with a while loop), replace the manual loop with node.replaceChildren() for concise and equivalent cleanup; locate the effect where node.firstChild is checked and swap the loop body to a single node.replaceChildren() call to remove all children.apps/frontend/src/components/qr-generator/style/SettingsForm.tsx (1)
349-375: Consider extracting option labels to reduce duplication.The translation keys (e.g.,
t('dotStyle.optionLabelSquare')) are duplicated between the mobileSelectitems and the memoizeddotStyleOptionsarray. You could iterate overdotStyleOptionsfor theSelectas well.♻️ Example using shared options
<div className="md:hidden"> - <Select onValueChange={field.onChange} value={field.value}> - <SelectTrigger> - <SelectValue placeholder={t('dotStyle.placeholder')} /> - </SelectTrigger> - <SelectContent> - <SelectItem value="square"> - {t('dotStyle.optionLabelSquare')} - </SelectItem> - {/* ... other items ... */} - </SelectContent> - </Select> + <Select onValueChange={field.onChange} value={field.value}> + <SelectTrigger> + <SelectValue placeholder={t('dotStyle.placeholder')} /> + </SelectTrigger> + <SelectContent> + {dotStyleOptions.map((opt) => ( + <SelectItem key={opt.value} value={opt.value}> + {opt.label} + </SelectItem> + ))} + </SelectContent> + </Select> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/components/qr-generator/style/SettingsForm.tsx` around lines 349 - 375, The mobile Select block in SettingsForm.tsx repeats translation labels already defined in the memoized dotStyleOptions array; refactor the md:hidden Select (the Select/SelectTrigger/SelectContent/SelectItem group using field.onChange and field.value) to iterate over dotStyleOptions and render each option (using its value and label) instead of hardcoding t('dotStyle.optionLabel...') entries so both desktop and mobile use the same source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/frontend/src/components/qr-generator/style/SettingsForm.tsx`:
- Around line 426-437: The SelectItem icon paths in SettingsForm (the SelectItem
elements using icon="icons/corners-square-square.svg",
"icons/corners-square-dot.svg", and "icons/corners-square-rounded.svg" and their
other occurrences) are missing a leading slash and therefore won't resolve with
Next.js Image; update the icon prop values on those SelectItem instances to
include a leading slash (e.g., change "icons/corners-square-square.svg" to
"/icons/corners-square-square.svg", "icons/corners-square-dot.svg" to
"/icons/corners-square-dot.svg", and "icons/corners-square-rounded.svg" /
"icons/corners-square-rounded.svg" to "/icons/…") so they match the icon maps
and resolve from the public folder when passed to the Image component.
In `@apps/frontend/src/components/qr-generator/style/StyleGridPicker.tsx`:
- Around line 76-82: The container in the StyleGridPicker return uses "flex
flex-wrap" but accepts a columnsClassName (default "grid-cols-3") which only
applies to CSS grid; update the component so the container uses CSS grid instead
of flex (replace the "flex flex-wrap" usage with "grid" and keep
columnsClassName) or alternatively change columnsClassName to a flex-based class
set and its default; locate the return JSX for the div with role="radiogroup" /
id={groupId} and adjust the className to a grid layout (e.g., include "grid" +
columnsClassName) so grid-cols-* utilities take effect consistently with the
columnsClassName prop.
---
Nitpick comments:
In `@apps/frontend/src/components/qr-generator/style/DotStylePreviewTile.tsx`:
- Around line 55-59: In the cleanup returned from the DotStylePreviewTile
component's effect (the anonymous function that currently removes children with
a while loop), replace the manual loop with node.replaceChildren() for concise
and equivalent cleanup; locate the effect where node.firstChild is checked and
swap the loop body to a single node.replaceChildren() call to remove all
children.
In `@apps/frontend/src/components/qr-generator/style/SettingsForm.tsx`:
- Around line 349-375: The mobile Select block in SettingsForm.tsx repeats
translation labels already defined in the memoized dotStyleOptions array;
refactor the md:hidden Select (the Select/SelectTrigger/SelectContent/SelectItem
group using field.onChange and field.value) to iterate over dotStyleOptions and
render each option (using its value and label) instead of hardcoding
t('dotStyle.optionLabel...') entries so both desktop and mobile use the same
source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e14f6b3f-feba-47be-88cd-0aa499e07709
📒 Files selected for processing (5)
apps/frontend/src/app/[locale]/page.tsxapps/frontend/src/components/qr-generator/QRcodeGenerator.tsxapps/frontend/src/components/qr-generator/style/DotStylePreviewTile.tsxapps/frontend/src/components/qr-generator/style/SettingsForm.tsxapps/frontend/src/components/qr-generator/style/StyleGridPicker.tsx
Comment on lines
+426
to
+437
| <SelectItem icon="icons/corners-square-square.svg" value="square"> | ||
| {t('cornersSquareOptions.optionLabelSquare')} | ||
| </SelectItem> | ||
| <SelectItem icon="icons/corners-square-dot.svg" value="dot"> | ||
| {t('cornersSquareOptions.optionLabelDot')} | ||
| </SelectItem> | ||
| <SelectItem | ||
| icon="icons/corners-square-rounded.svg" | ||
| value="extra-rounded" | ||
| > | ||
| {t('cornersSquareOptions.optionLabelExtraRound')} | ||
| </SelectItem> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how SelectItem handles the icon prop
ast-grep --pattern $'<SelectItem icon=$_'
# Also check the SelectItem component definition
rg -n "icon.*prop|icon.*:" --type=ts --type=tsx -g '**/select*.tsx' -A 3Repository: FloB95/qrcodly
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Find SettingsForm.tsx
fd -t f "SettingsForm.tsx"Repository: FloB95/qrcodly
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Find SelectItem component
fd -t f -i "select" | grep -E "\.(ts|tsx|js|jsx)$"Repository: FloB95/qrcodly
Length of output: 382
🏁 Script executed:
#!/bin/bash
# Check the SettingsForm.tsx file around lines 57-66 and 426-437
if [ -f "apps/frontend/src/components/qr-generator/style/SettingsForm.tsx" ]; then
echo "=== Lines 57-66 (Icon maps) ==="
sed -n '57,66p' "apps/frontend/src/components/qr-generator/style/SettingsForm.tsx"
echo ""
echo "=== Lines 426-437 (SelectItem usage) ==="
sed -n '426,437p' "apps/frontend/src/components/qr-generator/style/SettingsForm.tsx"
fiRepository: FloB95/qrcodly
Length of output: 1069
🏁 Script executed:
#!/bin/bash
# Read the SelectItem component definition
cat -n "apps/frontend/src/components/ui/select.tsx" | head -100Repository: FloB95/qrcodly
Length of output: 4613
🏁 Script executed:
#!/bin/bash
# Search for icon prop handling in select.tsx
rg "icon" "apps/frontend/src/components/ui/select.tsx" -B 2 -A 2Repository: FloB95/qrcodly
Length of output: 805
🏁 Script executed:
#!/bin/bash
# Search for SelectItem icon prop usage across the codebase
rg 'SelectItem.*icon=' --type ts --type tsx -B 1 -A 1Repository: FloB95/qrcodly
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Search for SelectItem icon prop usage without file type filter
rg 'SelectItem.*icon=' -B 1 -A 1 apps/frontend/srcRepository: FloB95/qrcodly
Length of output: 1516
Add leading slash to all SelectItem icon paths to match the icon maps and resolve correctly with Next.js Image component.
The SelectItem components use icon paths without a leading slash (e.g., "icons/corners-square-square.svg"), while the icon maps define them with a leading slash (e.g., "/icons/corners-square-square.svg"). Since SelectItem passes the icon string directly to Next.js Image component, the paths must include a leading slash to resolve from the public folder. Without it, the icons will fail to load at runtime. Update lines 426, 428, 431, 450, 452, and 454 to include the leading slash:
Example fix
-<SelectItem icon="icons/corners-square-square.svg" value="square">
+<SelectItem icon="/icons/corners-square-square.svg" value="square">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/frontend/src/components/qr-generator/style/SettingsForm.tsx` around
lines 426 - 437, The SelectItem icon paths in SettingsForm (the SelectItem
elements using icon="icons/corners-square-square.svg",
"icons/corners-square-dot.svg", and "icons/corners-square-rounded.svg" and their
other occurrences) are missing a leading slash and therefore won't resolve with
Next.js Image; update the icon prop values on those SelectItem instances to
include a leading slash (e.g., change "icons/corners-square-square.svg" to
"/icons/corners-square-square.svg", "icons/corners-square-dot.svg" to
"/icons/corners-square-dot.svg", and "icons/corners-square-rounded.svg" /
"icons/corners-square-rounded.svg" to "/icons/…") so they match the icon maps
and resolve from the public folder when passed to the Image component.
Comment on lines
+76
to
+82
| return ( | ||
| <div | ||
| role="radiogroup" | ||
| aria-label={ariaLabel} | ||
| id={groupId} | ||
| className={cn('flex flex-wrap gap-2', columnsClassName)} | ||
| > |
There was a problem hiding this comment.
Layout conflict: columnsClassName expects grid but container uses flexbox.
The columnsClassName prop defaults to 'grid-cols-3', but the container uses flex flex-wrap instead of grid. Tailwind's grid-cols-* utilities only work with display: grid.
🛠️ Proposed fix
<div
role="radiogroup"
aria-label={ariaLabel}
id={groupId}
- className={cn('flex flex-wrap gap-2', columnsClassName)}
+ className={cn('grid gap-2', columnsClassName)}
>📝 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
| return ( | |
| <div | |
| role="radiogroup" | |
| aria-label={ariaLabel} | |
| id={groupId} | |
| className={cn('flex flex-wrap gap-2', columnsClassName)} | |
| > | |
| return ( | |
| <div | |
| role="radiogroup" | |
| aria-label={ariaLabel} | |
| id={groupId} | |
| className={cn('grid gap-2', columnsClassName)} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/frontend/src/components/qr-generator/style/StyleGridPicker.tsx` around
lines 76 - 82, The container in the StyleGridPicker return uses "flex flex-wrap"
but accepts a columnsClassName (default "grid-cols-3") which only applies to CSS
grid; update the component so the container uses CSS grid instead of flex
(replace the "flex flex-wrap" usage with "grid" and keep columnsClassName) or
alternatively change columnsClassName to a flex-based class set and its default;
locate the return JSX for the div with role="radiogroup" / id={groupId} and
adjust the className to a grid layout (e.g., include "grid" + columnsClassName)
so grid-cols-* utilities take effect consistently with the columnsClassName
prop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes