Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Please double check the following review of the pull request:
Changes in the diff
Identified Issues
ID 1: store-cards API did not accept or validate
|
There was a problem hiding this comment.
Pull request overview
This PR implements a complete redesign of the user flow, transitioning from a multi-step wizard-based generator to a direct search-to-profile approach. The changes include a new user page editor with auto-save functionality, support for disabled cards and gradient colors, and comprehensive test updates.
Key Changes:
- Refactored user flow from generator wizard to direct profile editing
- Added new user auto-setup on first visit
- Implemented auto-save for card configurations
- Added support for disabled cards and gradient colors
- Removed legacy generator and settings E2E tests
Reviewed changes
Copilot reviewed 80 out of 81 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/api/store-cards/store-cards.test.ts |
Added tests for disabled cards, backfilling, and global settings optimization |
tests/unit/api/card/card.test.ts |
Added tests for color override and custom preset handling |
tests/e2e/settings/settings.spec.ts |
Removed (deprecated settings page) |
tests/e2e/home/home.spec.ts |
Updated to navigate to search instead of opening generator |
tests/e2e/generator/*.spec.ts |
Removed (deprecated generator wizard) |
tests/e2e/fixtures/test-utils.ts |
Removed GeneratorPage helper |
package.json |
Added new dependencies: @radix-ui/react-alert-dialog, @radix-ui/react-tabs, katex |
lib/utils.ts |
Added gradient color validation support |
lib/types/records.ts |
Added disabled, useCustomSettings fields; made variation optional; added GlobalCardSettings |
lib/stores/user-preferences.ts |
Removed defaultUsername field |
lib/stores/user-page-editor.ts |
New comprehensive state management for user page editing |
lib/stores/index.ts |
Updated exports for new store structure |
lib/stores/card-settings.ts |
Removed (replaced by user-page-editor) |
lib/seo.ts |
Removed settings and license page SEO configs |
lib/card-info-tooltips.ts |
New file with card tooltips and LaTeX formula support |
lib/card-groups.ts |
Updated to support gradient colors in URL params |
lib/card-data/config.ts |
Updated color resolution logic for custom presets |
lib/api-utils.ts |
Added disabled field validation |
hooks/useStatCardSubmit.ts |
Removed (deprecated generator hook) |
hooks/useCardAutoSave.ts |
New auto-save hook with debouncing |
components/user/UserPageHeader.tsx |
New header component with save state indicators |
components/user/UserPageEditor.tsx |
New comprehensive user page editor |
components/user/UserPageClient.tsx |
Removed (replaced by UserPageEditor) |
components/user/UserCard.tsx |
Removed (functionality moved to CardTile) |
components/user/GlobalSettingsPanel.tsx |
New global settings panel |
components/ui/Tabs.tsx |
New tabs component wrapper |
components/search/SearchHeroSection.tsx |
Updated description to reflect new auto-setup flow |
I have completed my review. No critical issues were found that require comments. The changes are well-structured and follow the project's patterns. The test coverage has been updated appropriately to reflect the new user flow, though the removal of E2E tests for the generator means that the new user page editor flow relies primarily on unit tests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughReplaces the legacy generator and settings with a new User Page Editor and editor store, adds per-card/global settings UI and tooling (tiles, previews, bulk actions, downloads, virtualized/reorderable grids), introduces lightbox/gallery and math rendering, Radix UI primitives, color/gradient utilities, expanded API validation/merge logic, and removes activityHeatmap/recentActivityFeed assets plus many generator/settings pages and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Editor as Browser (Editor UI)
participant API as /api/store-cards
participant DB as Persistence
participant AniList as External AniList
Note over User,Editor: User edits cards or triggers new-user setup
User->>Editor: modify cards / toggle settings / request save or setup
Editor->>Editor: debounce & build payload (useCardAutoSave)
Editor->>API: POST /api/store-cards { cards, globalSettings, ifMatch }
API->>API: validateCardData() → sanitize, merge with existing, compute mergedGlobalSettings
API->>DB: persist CardsRecord (cards + mergedGlobalSettings)
DB-->>API: ack + updatedAt
API-->>Editor: 200 OK + stored snapshot
Editor->>Editor: hydrate editor store (useUserPageEditor) and clear dirty state
alt New user onboarding
Editor->>AniList: fetch user id & stats (useNewUserSetup)
AniList-->>Editor: user id & stats
Editor->>API: POST create user & initial cards
API->>DB: persist setup snapshot
DB-->>API: ack
API-->>Editor: setup success
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
|
@gooroo-dev review |
Please double check the following review of the pull request:
Changes in the diff
Identified Issues
Issue 1: Disabled field type validation missing in store-cards APIExplanation: Fix: Add validation to reject non-boolean // In POST handler, after parsing body.cards:
for (const card of incomingCards as StoredCardConfig[]) {
if ("disabled" in card && typeof card.disabled !== "boolean") {
return new NextResponse(
JSON.stringify({ error: "Invalid 'disabled' field type" }),
{ status: 400 },
);
}
}Explanation of fix: Missing Tests
SummaryThe diff introduces a new user flow with a revamped user page editor, removes legacy generator components and settings page, improves API handling of card configs including disabled cards and global settings, and adds new UI components for card management and bulk actions. It also improves UX in search and example pages by navigating to Summon me to re-review when updated! Yours, Gooroo.dev |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/stat-card-generator/ColorPickerGroup.tsx (2)
315-327: Inconsistent use ofstopsvslocalStopsfor disabled state.The "Add Stop" button's
disabledprop (line 320) usesstops.lengthinstead oflocalStops.length. Since the component now renders and operates onlocalStops, this should be consistent.🔎 Proposed fix
<Button type="button" variant="outline" size="sm" onClick={addStop} - disabled={stops.length >= 5} + disabled={localStops.length >= 5} aria-label={`Add gradient color stop ${stops.length + 1} of 5`}
410-421: Same inconsistency for remove button disabled state.Line 415 uses
stops.lengthfor the remove button's disabled check, but should uselocalStops.lengthfor consistency with the refactored state management.🔎 Proposed fix
<Button type="button" variant="ghost" size="sm" onClick={() => removeStop(index)} - disabled={stops.length <= 2} + disabled={localStops.length <= 2} aria-label={`Remove gradient stop ${index + 1}`}
🧹 Nitpick comments (14)
components/search/SearchForm.tsx (1)
73-105: Consider handling navigation edge cases.The loading state is set to
truebut never reset tofalse. While this works for successful navigation (component unmounts), ifrouter.pushfails silently or the user navigates back, the form could remain in a loading state.This is a minor edge case since Next.js client-side navigation is generally reliable, but you could add a safety mechanism if needed.
🔎 Optional: Reset loading state after navigation attempt
// Ensure the loading overlay has a chance to render before navigation. scheduleAfterPaint(() => { router.push(`/user?${params.toString()}`); + // Reset loading state after a brief delay to handle edge cases + // where navigation might not unmount this component + setTimeout(() => { + setLoading(false); + onLoadingChange?.(false); + }, 2000); });components/user/UserPageHeader.tsx (1)
174-195: Consider simplifying the nested conditional className logic.The nested conditionals for save status styling are correct but complex. Consider extracting the logic into a helper function for better readability.
🔎 Proposed refactor
+function getSaveStatusClass(saveState: SaveState): string { + if (saveState.isSaving) { + return "bg-blue-100 text-blue-700 dark:bg-blue-900/30 dark:text-blue-400"; + } + if (saveState.saveError) { + return "bg-red-100 text-red-700 dark:bg-red-900/30 dark:text-red-400"; + } + if (saveState.isDirty) { + return "bg-amber-100 text-amber-700 dark:bg-amber-900/30 dark:text-amber-400"; + } + if (saveState.lastSavedAt) { + return "bg-emerald-100 text-emerald-700 dark:bg-emerald-900/30 dark:text-emerald-400"; + } + return "bg-slate-100 text-slate-600 dark:bg-slate-800 dark:text-slate-400"; +} <div className={cn( "flex items-center gap-2 rounded-full px-4 py-2 text-sm font-medium transition-all", - saveState.isSaving && - "bg-blue-100 text-blue-700 dark:bg-blue-900/30 dark:text-blue-400", - saveState.saveError && - "bg-red-100 text-red-700 dark:bg-red-900/30 dark:text-red-400", - !saveState.isSaving && - !saveState.saveError && - saveState.isDirty && - "bg-amber-100 text-amber-700 dark:bg-amber-900/30 dark:text-amber-400", - !saveState.isSaving && - !saveState.saveError && - !saveState.isDirty && - saveState.lastSavedAt && - "bg-emerald-100 text-emerald-700 dark:bg-emerald-900/30 dark:text-emerald-400", - !saveState.isSaving && - !saveState.saveError && - !saveState.isDirty && - !saveState.lastSavedAt && - "bg-slate-100 text-slate-600 dark:bg-slate-800 dark:text-slate-400", + getSaveStatusClass(saveState), )} >components/user/ColorPreviewCard.tsx (1)
53-75: Hex parsing assumes valid 6-character format.The hex-to-rgba conversion assumes a 6-character hex code. This could produce incorrect results for shorthand hex (e.g.,
#FFF) or invalid values.🔎 Proposed defensive fix
if (opacity < 1) { // Convert hex to rgba - const hex = stop.color.replace("#", ""); + let hex = stop.color.replace("#", ""); + // Expand shorthand hex (e.g., "FFF" -> "FFFFFF") + if (hex.length === 3) { + hex = hex[0] + hex[0] + hex[1] + hex[1] + hex[2] + hex[2]; + } const r = Number.parseInt(hex.substring(0, 2), 16); const g = Number.parseInt(hex.substring(2, 4), 16); const b = Number.parseInt(hex.substring(4, 6), 16);components/user/GlobalSettingsPanel.tsx (1)
50-66: Consider extracting default values to a shared constant.The reset handler uses hardcoded default values. If defaults change elsewhere (e.g., in the store initialization), these could drift out of sync.
🔎 Suggested approach
// In a shared constants file or exported from the store export const DEFAULT_GLOBAL_SETTINGS = { colorPreset: "default", borderEnabled: false, borderColor: "#e4e2e2", borderRadius: 5, advancedSettings: { useStatusColors: true, showPiePercentages: true, showFavorites: true, gridCols: 3, gridRows: 3, }, } as const; // Then use in the handler: const handleResetToDefaults = useCallback(() => { setGlobalColorPreset(DEFAULT_GLOBAL_SETTINGS.colorPreset); // ... etc }, [...]);components/user/CardSettingsDialog.tsx (2)
106-107:localBorderEnabledmay become stale if external state changes.The
useStateinitializer only runs once when the component mounts. IfhasBorderchanges while the dialog is open (e.g., from another source),localBorderEnabledwon't update to reflect the new state.Consider synchronizing with an effect or deriving directly from the config:
🔎 Option: Sync with effect
const [localBorderEnabled, setLocalBorderEnabled] = useState(hasBorder); + // Sync local state when external border state changes + useEffect(() => { + setLocalBorderEnabled(hasBorder); + }, [hasBorder]);
175-182: Consider memoizingadvancedSettingsobject.This object is recreated on every render. While the impact is minimal, memoizing it would prevent unnecessary re-renders of child components that depend on referential equality.
🔎 Proposed fix
- // Build advanced settings for SettingsContent - const advancedSettings = { - useStatusColors: config.advancedSettings.useStatusColors ?? false, - showPiePercentages: config.advancedSettings.showPiePercentages ?? false, - showFavorites: config.advancedSettings.showFavorites ?? false, - gridCols: config.advancedSettings.gridCols ?? 3, - gridRows: config.advancedSettings.gridRows ?? 3, - }; + // Build advanced settings for SettingsContent + const advancedSettings = useMemo( + () => ({ + useStatusColors: config.advancedSettings.useStatusColors ?? false, + showPiePercentages: config.advancedSettings.showPiePercentages ?? false, + showFavorites: config.advancedSettings.showFavorites ?? false, + gridCols: config.advancedSettings.gridCols ?? 3, + gridRows: config.advancedSettings.gridRows ?? 3, + }), + [config.advancedSettings], + );components/user/BulkActionsToolbar.tsx (2)
173-180: Consider providing user feedback on clipboard errors.When the clipboard operation fails (e.g., due to permissions or secure context requirements), the error is only logged to console. Users won't know why the copy didn't work.
🔎 Proposed enhancement
try { await navigator.clipboard.writeText(urls.join("\n")); setCopiedFormat(format); setTimeout(() => setCopiedFormat(null), 2000); } catch (err) { console.error("Failed to copy to clipboard:", err); + // Consider adding a toast notification or visual feedback here + // e.g., toast.error("Failed to copy to clipboard") }
213-219: Consider user feedback for download failures.Similar to the clipboard error, download failures only log to console. Users may not understand why their download didn't complete, especially if partial failures occur (line 209-213 handles this with a warning but no UI feedback).
components/user/SettingsContent.tsx (1)
250-304: Consider wrapping border settings animation in AnimatePresence for proper exit animation.The
motion.divusesexitprop but isn't wrapped inAnimatePresence, so exit animations won't trigger whenborderEnabledbecomes false.🔎 Proposed fix
+import { AnimatePresence } from "framer-motion"; {/* Border Settings - Only shown when border is enabled */} - {borderEnabled && ( + <AnimatePresence> + {borderEnabled && ( <motion.div initial={{ opacity: 0, height: 0 }} animate={{ opacity: 1, height: "auto" }} exit={{ opacity: 0, height: 0 }} className="space-y-4" > {/* ... content ... */} </motion.div> - )} + )} + </AnimatePresence>tests/e2e/user/user.spec.ts (1)
200-205: Consider using a more specific selector for preview images.Using
page.locator("main").locator("img")is broad and could match unrelated images (avatars, icons). Consider using a more specific selector like[data-testid="card-preview"]if available.components/user/CardTile.tsx (1)
356-370: Consider adding loading/error states for the preview image.The Image component doesn't have an
onErrorhandler. If the preview URL fails to load, users see no feedback.lib/stores/user-page-editor.ts (2)
238-241: Consider defensive handling for missing presets.If
colorPresets[presetName]doesn't exist andcolorPresets.defaultalso doesn't exist, this would return undefined. While unlikely in practice, adding explicit fallback would be safer.🔎 Proposed defensive fallback
function getPresetColors(presetName: string): ColorValue[] { const preset = colorPresets[presetName]; - return preset?.colors || colorPresets.default.colors; + return preset?.colors || colorPresets.default?.colors || ["#fe428e", "#141321", "#a9fef7", "#fe428e"]; }
560-571: Note: DEFAULT_GLOBAL_ADVANCED_SETTINGS is defined after its usage.The constant is used in
processServerCardsat line 466 but defined at line 565. While JavaScript hoisting handles this for const at module level, placing the constant definition before its usage would improve readability.components/user/UserPageEditor.tsx (1)
913-927: Consider debouncing search-triggered group expansion.When
normalizedQuerychanges rapidly (user typing), this effect expands groups on each change. While not breaking, it could cause minor UI churn.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (81)
AGENTS.mdapp/api/store-cards/route.tsapp/examples/page.tsxapp/globals.cssapp/license/page.tsxapp/page.tsxapp/settings/page.tsxapp/sitemap.xml/route.tsapp/user/page.tsxcomponents/AppSidebar.tsxcomponents/MathTooltipContent.tsxcomponents/StatCardGenerator.tsxcomponents/examples/CTASection.tsxcomponents/examples/CategorySection.tsxcomponents/examples/ExampleCard.tsxcomponents/search/SearchForm.tsxcomponents/search/SearchHeroSection.tsxcomponents/settings/DefaultCardSettings.tsxcomponents/settings/DefaultUsername.tsxcomponents/settings/ResetSettings.tsxcomponents/settings/SidebarBehavior.tsxcomponents/settings/ThemePreferences.tsxcomponents/stat-card-generator/AdvancedOptions.tsxcomponents/stat-card-generator/CardPreviewSkeleton.tsxcomponents/stat-card-generator/ColorPickerGroup.tsxcomponents/stat-card-generator/ColorPresetManager.tsxcomponents/stat-card-generator/ColorPresetSelector.tsxcomponents/stat-card-generator/GeneratorContext.tsxcomponents/stat-card-generator/LivePreview.tsxcomponents/stat-card-generator/LivePreviewSkeleton.tsxcomponents/stat-card-generator/StatCardPreview.tsxcomponents/stat-card-generator/StatCardTypeSelection.tsxcomponents/stat-card-generator/UpdateNotice.tsxcomponents/stat-card-generator/UserDetailsForm.tsxcomponents/stat-card-generator/UserFormSkeleton.tsxcomponents/stat-card-generator/WizardNavigation.tsxcomponents/stat-card-generator/constants.tscomponents/ui/AlertDialog.tsxcomponents/ui/Dialog.tsxcomponents/ui/Select.tsxcomponents/ui/Tabs.tsxcomponents/user/BulkActionsToolbar.tsxcomponents/user/CardCategorySection.tsxcomponents/user/CardList.tsxcomponents/user/CardSettingsDialog.tsxcomponents/user/CardSettingsPanel.tsxcomponents/user/CardTile.tsxcomponents/user/ColorPreviewCard.tsxcomponents/user/GlobalSettingsPanel.tsxcomponents/user/SettingsContent.tsxcomponents/user/UserCard.tsxcomponents/user/UserPageClient.tsxcomponents/user/UserPageEditor.tsxcomponents/user/UserPageHeader.tsxhooks/useCardAutoSave.tshooks/useStatCardSubmit.tslib/api-utils.tslib/card-data/config.tslib/card-groups.tslib/card-info-tooltips.tslib/seo.tslib/stores/card-settings.tslib/stores/index.tslib/stores/user-page-editor.tslib/stores/user-preferences.tslib/types/records.tslib/utils.tspackage.jsontests/e2e/examples/examples.spec.tstests/e2e/fixtures/index.tstests/e2e/fixtures/test-utils.tstests/e2e/generator/batch-generation.spec.tstests/e2e/generator/card-generation.spec.tstests/e2e/generator/customization.spec.tstests/e2e/generator/error-scenarios.spec.tstests/e2e/generator/wizard-navigation.spec.tstests/e2e/home/home.spec.tstests/e2e/settings/settings.spec.tstests/e2e/user/user.spec.tstests/unit/api/card/card.test.tstests/unit/api/store-cards/store-cards.test.ts
💤 Files with no reviewable changes (34)
- app/sitemap.xml/route.ts
- components/stat-card-generator/UserFormSkeleton.tsx
- lib/seo.ts
- components/settings/DefaultUsername.tsx
- components/settings/SidebarBehavior.tsx
- components/settings/DefaultCardSettings.tsx
- tests/e2e/generator/batch-generation.spec.ts
- components/stat-card-generator/ColorPresetManager.tsx
- hooks/useStatCardSubmit.ts
- components/stat-card-generator/UserDetailsForm.tsx
- components/settings/ThemePreferences.tsx
- components/user/UserCard.tsx
- app/license/page.tsx
- components/stat-card-generator/WizardNavigation.tsx
- components/StatCardGenerator.tsx
- lib/stores/card-settings.ts
- app/settings/page.tsx
- tests/e2e/generator/error-scenarios.spec.ts
- tests/e2e/generator/card-generation.spec.ts
- components/stat-card-generator/StatCardTypeSelection.tsx
- components/stat-card-generator/AdvancedOptions.tsx
- lib/stores/user-preferences.ts
- components/user/CardList.tsx
- components/stat-card-generator/GeneratorContext.tsx
- components/settings/ResetSettings.tsx
- tests/e2e/generator/wizard-navigation.spec.ts
- tests/e2e/settings/settings.spec.ts
- components/stat-card-generator/LivePreview.tsx
- components/stat-card-generator/LivePreviewSkeleton.tsx
- components/stat-card-generator/UpdateNotice.tsx
- components/stat-card-generator/StatCardPreview.tsx
- tests/e2e/generator/customization.spec.ts
- components/stat-card-generator/CardPreviewSkeleton.tsx
- components/user/UserPageClient.tsx
🧰 Additional context used
🧬 Code graph analysis (26)
components/examples/CTASection.tsx (2)
components/CTASection.tsx (1)
CTASection(29-136)components/examples/index.ts (1)
CTASection(4-4)
tests/e2e/home/home.spec.ts (1)
tests/e2e/fixtures/test-utils.ts (1)
test(26-143)
components/ui/Tabs.tsx (1)
lib/utils.ts (1)
cn(178-180)
components/examples/ExampleCard.tsx (1)
components/examples/index.ts (1)
ExampleCard(3-3)
components/user/GlobalSettingsPanel.tsx (3)
lib/types/card.ts (1)
ColorValue(41-41)components/ui/Dialog.tsx (3)
DialogHeader(191-191)DialogTitle(193-193)DialogDescription(194-194)components/user/CardSettingsPanel.tsx (1)
CardSettingsPanel(64-182)
components/ui/Dialog.tsx (1)
lib/utils.ts (1)
cn(178-180)
app/user/page.tsx (1)
components/user/UserPageEditor.tsx (1)
UserPageEditor(605-1543)
lib/card-data/config.ts (3)
lib/card-data/index.ts (1)
isCustomPreset(48-48)lib/types/records.ts (2)
StoredCardConfig(445-462)CardsRecord(482-487)lib/utils.ts (1)
clampBorderRadius(870-877)
components/user/CardCategorySection.tsx (1)
lib/utils.ts (1)
cn(178-180)
tests/unit/api/card/card.test.ts (4)
tests/unit/__setup__.test.ts (1)
sharedRedisMockGet(15-15)app/api/card/route.ts (1)
GET(535-660)lib/svg-templates/media-stats/shared.ts (1)
mediaStatsTemplate(242-382)lib/svg-templates/extra-anime-manga-stats/shared.ts (1)
extraAnimeMangaStatsTemplate(39-661)
components/ui/AlertDialog.tsx (2)
lib/utils.ts (1)
cn(178-180)components/ui/Button.tsx (1)
buttonVariants(73-73)
components/user/CardSettingsPanel.tsx (4)
components/user/SettingsContent.tsx (1)
SettingsContent(108-460)lib/utils.ts (1)
cn(178-180)components/ui/Button.tsx (1)
Button(73-73)components/ui/Switch.tsx (1)
Switch(34-34)
hooks/useCardAutoSave.ts (4)
lib/types/card.ts (1)
ColorValue(41-41)lib/stores/user-page-editor.ts (1)
ServerCardData(190-207)lib/stores/index.ts (1)
ServerCardData(31-31)lib/utils.ts (2)
parseResponsePayload(631-644)getResponseErrorMessage(651-665)
components/ui/Select.tsx (1)
lib/utils.ts (1)
cn(178-180)
lib/types/records.ts (1)
lib/types/card.ts (1)
ColorValue(41-41)
app/api/store-cards/route.ts (3)
lib/card-data/validation.ts (1)
displayNames(79-134)lib/types/records.ts (3)
StoredCardConfig(445-462)GlobalCardSettings(465-479)CardsRecord(482-487)lib/utils.ts (2)
safeParse(769-802)clampBorderRadius(870-877)
app/examples/page.tsx (2)
components/examples/CTASection.tsx (1)
CTASection(11-49)components/CTASection.tsx (1)
CTASection(29-136)
components/search/SearchForm.tsx (1)
lib/utils/google-analytics.ts (3)
safeTrack(95-101)trackFormSubmission(222-228)trackNavigation(181-192)
components/user/BulkActionsToolbar.tsx (4)
lib/stores/user-page-editor.ts (1)
useUserPageEditor(599-1110)hooks/useIsMobile.ts (1)
useIsMobile(12-31)lib/card-groups.ts (2)
mapStoredConfigToCardUrlParams(458-546)buildCardUrlWithParams(599-647)lib/utils.ts (3)
ConversionFormat(152-152)BatchExportCard(155-159)batchConvertAndZip(1180-1221)
tests/e2e/user/user.spec.ts (2)
tests/e2e/fixtures/test-utils.ts (2)
expect(182-182)test(26-143)tests/e2e/fixtures/mock-data.ts (1)
mockCardsRecord(254-277)
tests/unit/api/store-cards/store-cards.test.ts (3)
tests/unit/__setup__.test.ts (3)
sharedRatelimitMockLimit(65-67)sharedRedisMockSet(16-16)sharedRedisMockGet(15-15)app/api/store-cards/route.ts (1)
POST(357-496)lib/card-data/validation.ts (1)
displayNames(79-134)
components/user/UserPageEditor.tsx (7)
lib/anilist/queries.ts (2)
USER_ID_QUERY(2-8)USER_STATS_QUERY(529-536)lib/utils.ts (2)
parseResponsePayload(631-644)getResponseErrorMessage(651-665)lib/error-messages.ts (1)
getErrorDetails(316-359)lib/error-tracking.ts (1)
trackUserActionError(73-92)lib/types/records.ts (1)
ReconstructedUserRecord(432-442)lib/stores/user-page-editor.ts (3)
ServerCardData(190-207)ServerGlobalSettings(213-227)useUserPageEditor(599-1110)hooks/useCardAutoSave.ts (1)
useCardAutoSave(174-260)
components/stat-card-generator/ColorPickerGroup.tsx (1)
lib/types/card.ts (1)
GradientStop(5-14)
components/user/UserPageHeader.tsx (2)
components/PageShell.tsx (2)
containerVariants(13-19)itemVariants(21-24)lib/utils.ts (1)
cn(178-180)
lib/card-groups.ts (2)
lib/utils.ts (1)
buildApiUrl(57-62)lib/types/card.ts (1)
ColorValue(41-41)
lib/utils.ts (1)
lib/svg-templates/common/color-utils.ts (1)
isValidHexColor(4-4)
🪛 ast-grep (0.40.3)
components/MathTooltipContent.tsx
[warning] 35-35: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
components/user/ColorPreviewCard.tsx
[warning] 153-153: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
components/MathTooltipContent.tsx
[error] 36-36: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
components/user/ColorPreviewCard.tsx
[error] 154-154: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 LanguageTool
AGENTS.md
[style] ~20-~20: Try moving the adverb to make the sentence clearer.
Context: ... task — DO NOT create one Plan that attempts to fully plan multiple unrelated tasks at once. Each Plan must be focused and ...
(SPLIT_INFINITIVE)
🔇 Additional comments (103)
components/stat-card-generator/ColorPresetSelector.tsx (3)
18-18: LGTM: ChevronUp import added for toggle UI.The import is correctly added to support the expand/collapse toggle functionality introduced later in the component.
404-404: LGTM: Consistent border radius on selection overlay.The
rounded-xlclass ensures the selection overlay respects the parent button's rounded corners, preventing visual artifacts at the edges.
466-493: LGTM: Well-implemented toggle behavior for expand/collapse.The toggle implementation correctly:
- Uses
hasMorePresetsto conditionally render the button- Toggles
isExpandedstate bidirectionally- Provides clear visual feedback with appropriate chevron icons
- Displays intuitive button text ("Show less" vs. "Show all X presets")
This is a solid UX improvement over the previous expand-only behavior, allowing users to collapse the list when needed.
components/search/SearchHeroSection.tsx (1)
67-69: LGTM!The updated onboarding text clearly communicates the new user flow—directing users to enter a username/ID and noting automatic setup for first-time users. This aligns well with the PR's shift from modal-based generation to direct navigation.
tests/e2e/fixtures/index.ts (1)
1-6: LGTM!Clean simplification of the fixture exports following the removal of the GeneratorPage Page Object. The updated comment accurately reflects the reduced scope.
lib/utils.ts (1)
472-511: LGTM with a minor observation on naming semantics.The JSON-encoded gradient support is a good addition for handling stored/transported gradient definitions. The trimming and empty string checks are solid defensive improvements.
One minor semantic note:
getColorInvalidReasonreturning"gradient JSON string is valid"(line 497) when the value is actually valid may be slightly confusing since the function name implies it returns reasons for invalid values. However, looking at the overall pattern, this seems intentional as a diagnostic message for edge cases where shared validation differs from individual checks. No action required.components/AppSidebar.tsx (1)
33-40: LGTM!The new "Search User" navigation item is well-structured with consistent styling patterns (gradient, bgLight, textColor). The
/searchroute aligns with the PR's shift to route-based user discovery.tests/e2e/fixtures/test-utils.ts (1)
1-21: LGTM!Clean removal of the
GeneratorPagePage Object and associated imports. The remaining mock fixtures (mockSuccessfulApi,mockRateLimitedApi,mockUserNotFoundApi,mockNetworkError) are well-organized and sufficient for testing the new route-based flow.components/search/SearchForm.tsx (1)
25-33: Nice paint-scheduling pattern.The
scheduleAfterPainthelper withrequestAnimationFrameandsetTimeoutfallback is a clean approach to ensure the loading overlay renders before navigation.components/ui/Tabs.tsx (1)
1-55: LGTM!Well-structured Radix UI Tabs wrapper following established patterns in the codebase. Good use of
forwardReffor all subcomponents, properdisplayNameassignments, and comprehensive styling including dark mode and focus-visible states.components/stat-card-generator/ColorPickerGroup.tsx (1)
238-244: Good local state pattern for controlled edits.Using
localStopsto manage edits locally before propagating viaonStopsChangeis a clean pattern that avoids prop mutation and provides better control over state updates.components/ui/Dialog.tsx (2)
52-64: LGTM! Clean API extension.The new
DialogContentPropsinterface provides a clear escape hatch for hiding or customizing the close button, with good documentation.
71-110: LGTM! Conditional close button rendering is correctly implemented.The implementation properly:
- Destructures the new props with sensible defaults
- Conditionally renders the close button based on
hideCloseButton- Applies custom styling via
closeButtonClassNamewhen provided- Maintains all existing functionality
AGENTS.md (1)
1-55: LGTM! Documentation restructure improves clarity.The refactored workflow documentation is more actionable with:
- Clear quick-start steps
- Concrete subagent workflow example
- Better organized sections for modes, libraries, and memory policy
The static analysis hint about the split infinitive ("to fully plan") is a minor style preference that doesn't impact clarity.
components/user/UserPageHeader.tsx (2)
64-71: LGTM! Time formatting logic is clear and correct.The function properly handles null input and provides sensible time buckets (5s, 60s, 3600s) with appropriate formatting.
114-122: Theunoptimizedprop is necessary and should be retained. AniList avatar domains are not configured innext.config.ts—only internal domains (api.anicards.alpha49.com, anicards.alpha49.com, and localhost variants) are inremotePatterns. Without explicit configuration, Next.js image optimization would fail for AniList avatar URLs. Theunoptimizedprop is the correct approach for external images from unconfigured domains.Likely an incorrect or invalid review comment.
components/ui/Select.tsx (3)
56-57: LGTM! Improved scroll button interactivity.Changing from
cursor-defaulttocursor-pointerand enhancing hover states provides better UX feedback for interactive scroll buttons.Also applies to: 74-75
97-97: LGTM! Enhanced visual elevation for dropdown.The updated styling with rounded-xl, backdrop blur, and theme-aware borders/shadows creates a more polished, modern dropdown appearance.
151-163: LGTM! Comprehensive item state styling.The updated SelectItem styling properly handles:
- Hover/focus states with appropriate backgrounds
- Checked state with gradient accent
- Disabled state with reduced opacity
- Theme-aware colors for light/dark modes
hooks/useCardAutoSave.ts (5)
45-68: LGTM! Proper error handling in API call.The function correctly:
- Uses appropriate headers and body formatting
- Parses response payload using provided utilities
- Handles both network errors and HTTP errors
- Returns consistent result shape
184-192: LGTM! Proper cleanup on unmount.The effect correctly:
- Sets mounted flag to true on mount
- Returns cleanup function to clear timeouts and set mounted flag to false
- Prevents state updates after unmount
229-250: LGTM! Correct debounce implementation.The auto-save effect properly:
- Guards against disabled state, missing userId, no changes, or active save
- Clears existing timeout before setting new one
- Returns cleanup function
- Lists all relevant dependencies
94-110: No action needed. The client-side color logic inuseCardAutoSave.tsis correctly aligned with the server-side handler in/api/store-cards/route.ts. Both use identical conditionals (!colorPreset || colorPreset === "custom") to determine whether to send individual colors, and the server properly handles undefined color fields by omitting them from storage. The design is intentional: when a preset is selected, the preset name is the source of truth and individual colors are not stored.
115-150: The merge logic is correct and properly aligned with server-side expectations. The code correctly handles custom vs global color preset selection (client sendseffectivePresetwhich server mirrors inbuildCardConfig), conditionally sends colors only when applicable (shouldSendColorslogic), preserves per-card advanced settings through the merge process, and correctly handles the disabled flag. No changes needed.lib/card-info-tooltips.ts (3)
9-9: Shannon entropy formula appears correct.The normalized Shannon entropy formula
H' = -Σ(p_i ln p_i) / ln kis the standard formula for diversity indices. The use ofString.rawproperly preserves backslashes in LaTeX notation.Also applies to: 11-11, 13-13
77-89: LGTM! Simple and effective helper functions.Both helpers provide clean APIs for querying tooltip availability and content.
7-14: KaTeX is properly configured and rendering LaTeX formulas correctly.KaTeX (v0.16.27) is installed, with CSS imported globally in
app/globals.css. TheMathTooltipContentcomponent handles rendering of both inline ($...$) and display ($$...$$) math delimiters usingkatex.renderToString()with appropriate error handling. This component is integrated with the tooltip system and receives content fromCARD_INFO_TOOLTIPS.tests/e2e/examples/examples.spec.ts (1)
38-47: LGTM! Test correctly validates route-based navigation.The updated test properly:
- Clicks the CTA button
- Asserts navigation to
/searchroute- Verifies the presence of the username input field
This aligns with the broader PR's shift from modal-based to route-based navigation.
app/user/page.tsx (2)
34-51: LGTM! Proper username normalization and encoding.The changes correctly:
- Trim whitespace from username for normalization
- Use normalized value consistently in metadata
- Apply
encodeURIComponentfor URL safety in canonical link
75-75: LGTM! Component replacement aligns with new user flow.Replacing
UserPageClientwithUserPageEditormatches the PR's objective of implementing a new user flow with enhanced editing capabilities.app/globals.css (2)
1-1: LGTM! KaTeX CSS imported for math rendering support.The KaTeX import correctly supports the new
MathTooltipContentcomponent and aligns with thekatexdependency added inpackage.json.
187-244: LGTM! Scrollbar utilities for consistent UI.The
.show-scrollbarand.overlay-scrollbarutilities provide consistent, theme-aware scrollbar styling. The approach using CSS variables ensures proper integration with the existing theming system.lib/api-utils.ts (2)
618-652: LGTM! Disabled card validation correctly skips unnecessary fields.The logic appropriately reduces validation requirements for disabled cards:
- Only
cardNameis validated as a required string- Color validation (both preset and explicit color fields) is correctly skipped
- Card type validation is still performed to ensure data integrity
This optimization aligns with the broader global/per-card settings architecture introduced in this PR.
674-674: LGTM! Disabled field correctly added to optional boolean fields.The
disabledfield is appropriately validated as an optional boolean, ensuring type safety for per-card configuration.app/examples/page.tsx (3)
4-4: LGTM! Navigation-based flow replaces modal-based generator.The router-based navigation to
/searchis clean and aligns with the broader refactor away from modal-based workflows. The callback is properly memoized withuseRouterdependency.Also applies to: 489-493
629-629: LGTM! ErrorBoundary reset keys simplified.The
resetKeyscorrectly depends only onsearchQuerynow that the modal-based generator state has been removed.
657-657: LGTM! Prop name changes are consistent across components.The prop rename from
onOpenGeneratortoonStartCreatingaccurately reflects the new navigation-based flow and is consistent with updates inCategorySectionandCTASectioncomponents.Also applies to: 667-667
tests/e2e/home/home.spec.ts (2)
4-14: LGTM! E2E test correctly validates navigation flow.The test properly verifies:
- Navigation to
/searchoccurs after CTA click- The search input is visible on the target page
This aligns with the shift away from modal-based interactions to direct navigation.
38-47: LGTM! Improved viewport visibility assertion.The updated viewport check is more robust by verifying both
topandbottompositions, ensuring the showcase is actually visible in the viewport rather than just having a visible top coordinate.package.json (1)
18-18: LGTM! Dependencies correctly added for new features.The new dependencies support the introduced features:
- Radix UI components for alert dialogs and tabs
- KaTeX for LaTeX math rendering
- TypeScript definitions for type safety
Version ranges are appropriate using caret notation for patch compatibility.
Also applies to: 27-27, 38-38, 57-57
components/examples/CTASection.tsx (1)
8-8: LGTM! Prop rename is complete and consistent.The
onOpenGenerator→onStartCreatingrename is consistently applied throughout the component and aligns with the navigation-based flow changes inapp/examples/page.tsx.Also applies to: 11-11, 42-42
components/MathTooltipContent.tsx (2)
48-77: LGTM! LaTeX processing logic and error handling are sound.The regex patterns correctly handle:
- Display math
$$...$$processed first to avoid conflicts- Inline math
$...$processed second, excluding newlines- Error handling gracefully falls back to original delimiters
The
containsMathhelper efficiently detects LaTeX presence.Also applies to: 85-87
25-77: The XSS concern does not apply here. All tooltip content comes from hardcoded values inCARD_INFO_TOOLTIPS(lib/card-info-tooltips.ts), which are application-generated strings containing only plain text and LaTeX math formulas—no user input or external HTML is involved.The example
<script>alert('xss')</script>cannot occur because the tooltip sources (countryDiversity,genreDiversity, etc.) are defined as static strings in the source code usingString.raw, with no HTML tags. KaTeX'srenderToString()safely renders math expressions, and content outside delimiters remains unchanged plain text. No sanitization is required when the content is trusted and controlled.If the optional
infoTooltipprop is used to override defaults, the responsibility for trusted content moves to the caller—the component should document this expectation if necessary.Likely an incorrect or invalid review comment.
tests/unit/api/card/card.test.ts (2)
1538-1570: LGTM! Test correctly validates DB lookup optimization.The test verifies that when:
colorPreset=custom- All required color params are in the URL
The route skips the cards DB lookup and uses only the user record, correctly passing URL-provided colors to the template. This validates an important performance optimization.
1572-1625: LGTM! Test validates URL color precedence with required DB config.The test correctly verifies that when:
- DB config is required for card-specific flags (e.g.,
showFavoritesforanimeStaff)- URL provides
colorPreset=customwith full color paramsURL-provided colors override DB-configured colors, even though the DB lookup still occurs for other fields. This validates the proper precedence rules in the color resolution logic.
app/page.tsx (1)
20-26: LGTM! Clean router-based navigation implementation.The transition from modal-based flow to router navigation is well-implemented. The
useCallbackhook correctly includesrouterin its dependency array, and the analytics tracking is preserved.components/examples/ExampleCard.tsx (1)
16-29: LGTM! Consistent prop rename.The prop rename from
onOpenGeneratortoonStartCreatingis cleanly applied across the interface definition and component destructuring. The button text update to "Create Your Cards" better reflects the new user flow.components/examples/CategorySection.tsx (1)
37-42: LGTM! Proper prop propagation.The
onStartCreatingprop rename is consistently applied through the interface, component signature, and child component invocation. The propagation chain aligns with theExampleCardAPI changes.Also applies to: 111-116, 184-192
components/user/ColorPreviewCard.tsx (1)
149-157:dangerouslySetInnerHTMLusage is acceptable here.The static analysis flagged this as a potential XSS vector. However, the SVG content is:
- Generated internally by
animeStatsTemplatewith controlled inputs- Processed through
stripTrustedSvgMarkerwhich implies a trust boundarySince this is not rendering user-provided HTML, the XSS risk is mitigated. Ensure that
animeStatsTemplateproperly sanitizes any user-controlled values (likeusername) if they're ever used with actual user input.components/user/GlobalSettingsPanel.tsx (1)
75-113: LGTM! Well-structured global settings panel.The component properly integrates with the Zustand store, uses appropriate memoization for handlers, and correctly wires up the
CardSettingsPanelin global mode.components/user/CardSettingsPanel.tsx (1)
55-94: LGTM! Clean discriminated union pattern.The type narrowing via
props.mode === "global"is idiomatic TypeScript, and the conditional rendering logic correctly handles all three states: global settings, card with custom settings enabled, and card with global settings fallback.components/user/BulkActionsToolbar.tsx (2)
100-159: Well-structured URL building with proper memoization.The
selectedCardscomputation correctly integrates with the store's effective color/border getters and properly maps configurations to URL parameters. The dependency array appears complete, and the null-filtering type guard is correctly implemented.
65-71: Good SSR-safe portal pattern.The
isMountedstate pattern correctly prevents hydration mismatches by deferring portal rendering until after client-side hydration. The early returns are properly ordered.Also applies to: 224-227
components/user/SettingsContent.tsx (4)
1-99: Well-structured component with clear prop interface.The component's props interface is comprehensive and well-documented with JSDoc comments. The separation of color, border, and advanced settings concerns is clean.
127-141: Visibility defaults and tab logic are correct.The default visibility calculation based on mode and the
hasAdvancedOptionscheck ensure the Advanced tab only appears when relevant options exist.
143-171: useMemo dependencies are complete.The memoization correctly includes all dependencies used in the callback:
idPrefix,mode,colors, andonColorChange.
398-434: Grid input handlers correctly clamp values.The inline clamping logic
Math.max(1, Math.min(5, Number(e.target.value) || 1))ensures grid dimensions stay within valid bounds (1-5). The fallback to1when parsing fails is appropriate.tests/e2e/user/user.spec.ts (3)
17-28: Error state expectations updated correctly.The updated assertions align with the new UI text for error states. Using regex patterns (
/something went wrong/i,/search for user/i) provides flexibility for minor text variations.
78-107: Bulk actions test is comprehensive.The test properly:
- Selects specific cards via checkboxes
- Verifies toolbar visibility with
data-testid- Checks button order and labels
- Tests popover interactions with appropriate timeouts
Good coverage of the new bulk actions workflow.
137-206: New user onboarding test provides good coverage.The test correctly mocks the full API sequence for a new user:
- 404 from
get-usertriggers Anilist lookup- Successful
get-anilistresponse- Store endpoints return success
get-cardsreturns empty array- Card endpoint returns valid SVG
The assertion that at least one preview image appears validates the initial cards snapshot behavior.
app/api/store-cards/route.ts (4)
52-69: Good defensive backfill for supported card types.
ensureAllSupportedCardTypesPresentensures Redis records always contain all supported card types, preventing partial records. Cards not explicitly provided are marked as disabled.
166-186: Variation fallback preserves pie settings when disabling cards.The logic correctly falls back to
previous?.variationwhenincoming.variationis undefined, ensuringshowPiePercentagessettings aren't lost when toggling a card's disabled state without re-specifying variation.
267-320: buildCardConfig correctly respects useCustomSettings.When
useCustomSettingsis false, color fields andborderColorare set toundefined, ensuring the card defers to global settings. The default oftruemaintains backward compatibility.
455-467: Global settings merge logic correctly handles conditional fields.The merged global settings object properly:
- Preserves
existingGlobalSettingswhen no newglobalSettingsprovided- Clears
borderColorandborderRadiuswhen borders disabled- Merges advanced settings with proper precedence
lib/card-data/config.ts (5)
13-25: Clean helper for validating complete color params.
hasAllRequiredColorParamsprovides a clear, reusable check for URL color completeness. The truthy check (!!params.titleColorParam) correctly handles bothnullandundefined.
48-56: Updated color resolution logic is correct.The comment and implementation correctly document that
colorPreset=customwithout individual colors requires DB fetch, whilecolorPreset=customwith all colors can be resolved from URL alone.
194-203: Custom preset URL override enhancement is well-implemented.Allowing URL color params to override when the preset is "custom" AND all required params are present enables sharable custom color links without DB dependency. The early return after
applyUrlColorParamsis correct.
436-440: Global settings merge placement is correct.Merging global settings into the card config before applying URL overrides ensures:
- Cards without per-card settings inherit from globals
- URL params can still override inherited values
559-609: mergeGlobalSettingsIntoConfig uses correct non-clobbering pattern.The
setIfMissinghelper elegantly ensures global settings only fill gaps without overwriting explicit per-card values. The conditional border application based onborderEnabledis appropriate.components/user/CardCategorySection.tsx (3)
50-63: Controlled/uncontrolled pattern is well-implemented.The component correctly:
- Determines mode via
typeof expanded === "boolean"- Only updates internal state when uncontrolled
- Always calls
onExpandedChangewhen providedThis follows React's controlled component conventions.
75-84: Good accessibility implementation.The header button properly uses:
type="button"to prevent form submissionaria-expandedfor screen readersaria-controlslinked to content viauseId
142-159: AnimatePresence correctly wraps conditional content.Unlike the issue in SettingsContent.tsx, this component properly wraps the conditional
motion.divwithAnimatePresence, enabling exit animations when the section collapses.lib/stores/index.ts (2)
19-32: New user-page-editor exports provide comprehensive API.The new exports include:
- Hook:
useUserPageEditor- Selectors:
selectEnabledCardIds,selectHasEnabledCards,selectCardConfigsByGroup- Types:
CardColorOverride,CardAdvancedSettings,CardEditorConfig, etc.This consolidates card editing state management into a single store.
6-7: Types are not removed, only excluded from barrel export.
UserPreferencesState,UserPreferencesActions, andUserPreferencesStoreremain exported fromlib/stores/user-preferences.ts. They are simply not re-exported in the barrel atlib/stores/index.ts. This is not a breaking change—no external consumers depend on these types.Likely an incorrect or invalid review comment.
components/ui/AlertDialog.tsx (2)
1-66: Standard Radix AlertDialog wrapper implementation.The component follows established shadcn/ui patterns:
- ForwardRef for components that need ref access
- DisplayName assignments for debugging
- Composition of Portal and Overlay in Content
- Consistent animation classes
137-171: Action and Cancel buttons correctly styled.
AlertDialogActionuses default button styles whileAlertDialogCanceluses outline variant with responsive margin (mt-2 sm:mt-0). This provides appropriate visual hierarchy for confirmation dialogs.tests/unit/api/store-cards/store-cards.test.ts (6)
157-172: Good validation test for disabled field type.Tests that non-boolean
disabledvalues (like"yes") are properly rejected with 400 status. This ensures type safety at the API boundary.
267-290: Storage assertions updated for backfill behavior.The test correctly verifies that:
- Total card count matches
displayNameslength (backfilled)- All supported card types are present
- Explicitly provided cards retain their configuration
Good validation of the
ensureAllSupportedCardTypesPresentbehavior.
322-384: Preserving settings when disabling is well-tested.This test validates a critical user experience scenario: toggling a card's disabled state shouldn't lose custom configurations. The mock setup and assertions are comprehensive.
946-1007: Border settings tests cover global/per-card interactions.Good coverage of:
borderColornot saved whenborderEnabled=false- Clearing existing
borderColorwhen disabling- Preserving per-card borders when global is disabled
1241-1273: Test verifies borderRadius clamping during merge.The test confirms that out-of-range values (150) are clamped to the maximum (100) when merging existing global settings. This validates the
clampBorderRadiusintegration.
1353-1384: Grid dimension clamping tests are thorough.Tests both:
- Clamping existing out-of-range values (999 → 5, -10 → 1)
- Clamping incoming values
Validates the
clampGridDimfunction integration for grid settings.components/user/CardTile.tsx (3)
97-142: LGTM - Well-structured component initialization.Good use of Zustand store selectors and fallback config object with sensible defaults. The memoized effective colors/border calculations will help prevent unnecessary re-renders.
156-193: LGTM - Preview URL construction is well-memoized.The dependency array correctly captures all values that affect the URL. The early return for missing userId prevents unnecessary computation.
232-252: LGTM - Download handler with proper error handling.Good use of loading state to prevent concurrent downloads, and proper cleanup in finally block.
lib/types/records.ts (3)
1-2: LGTM - Proper type import for ColorValue.The import enables gradient support for color fields throughout the records types.
444-462: LGTM - StoredCardConfig updates are well-designed.The changes properly support:
- Optional variation for backwards compatibility
- ColorValue type for gradient support
useCustomSettingsflag for explicit per-card override detectiondisabledflag for card visibility state
464-486: LGTM - GlobalCardSettings interface properly encapsulates shared configuration.The interface mirrors per-card settings appropriately, with
borderEnabledas the key differentiator for global border state. Integration with CardsRecord via optionalglobalSettingsfield maintains backwards compatibility.lib/card-groups.ts (3)
9-17: LGTM - colorToString correctly serializes ColorValue types.The function properly handles both string colors and gradient objects (serialized as JSON). The undefined check prevents unnecessary processing.
458-478: LGTM - Type signature properly separates color fields for ColorValue support.The
Omitpattern with re-declared color fields typed asColorValueallows the function to accept both string and gradient inputs while the originalStoredCardConfigmay have different typing. The newallowPresetColorOverridesoption provides flexibility for different use cases.
492-522: LGTM - Border and color inclusion logic is well-designed.Key behaviors:
borderRadiusis only included whenborderColoris present (sensible - radius without border is meaningless)- Colors are only included when
shouldIncludeColorsis true, respecting preset semantics- The
hasNamedPresetcheck prevents color overrides when using named presets (unless explicitly allowed)lib/stores/user-page-editor.ts (3)
358-445: LGTM - Robust server-to-editor config transformation.The function handles:
- Explicit
useCustomSettingsflag from server (preferred)- Legacy heuristic detection via color comparison for backwards compatibility
- Proper normalization of ColorValue for comparison (string vs JSON)
- Disabled state mapping to enabled flag
The logic correctly prioritizes explicit server flags over heuristics.
599-1110: LGTM - Comprehensive Zustand store implementation.The store is well-organized with:
- Clear action groupings (user data, global settings, card config, UI, bulk ops, save)
- Proper use of
devtoolsmiddleware for debugging- Consistent dirty state tracking on mutations
- Effective color/border resolution methods that handle inheritance
1148-1161: selectCardConfigsByGroup is incomplete and unused—the function is dead code.CardEditorConfig doesn't store group information, so this selector cannot group by statCardTypes without additional context. The function is exported but never referenced in the codebase. Either remove it or complete it by passing
statCardTypesas a parameter and implementing the proper grouping logic using thegroupproperty from each card type.components/stat-card-generator/constants.ts (3)
111-139: LGTM - Label simplifications improve readability.The removal of verbose qualifiers like "(Top 5 Count)" and "(Pie/Donut/Bar Breakdowns)" makes the UI cleaner while the card functionality remains unchanged.
141-179: LGTM - Consistent label simplification across manga and activity cards.Labels are now concise and consistent with the anime breakdown cards pattern.
243-267: LGTM - Core stats labels simplified."Anime Statistics" → "Anime Stats" etc. provides a cleaner UI without losing clarity.
components/user/UserPageEditor.tsx (6)
136-180: LGTM - Robust AniList user ID fetching with proper error handling.Good handling of specific HTTP status codes (404, 429) with user-friendly messages. The try/catch properly handles network failures.
257-264: LGTM - Initial cards snapshot provides sensible defaults.New users start with all cards enabled using the first variation and default preset, ensuring a complete initial experience.
384-496: LGTM - Comprehensive new user setup flow.The orchestration properly:
- Resolves userId from username if needed
- Fetches stats with proper error tracking
- Extracts username from stats as fallback
- Saves user to database
- Saves initial cards (non-fatal on failure)
- Extracts avatar URL
Error tracking is thorough with appropriate categories.
652-847: LGTM - Data loading effect with StrictMode protection.The
lastLoadedUserRefpattern correctly prevents duplicate API calls during React StrictMode double-mounting. The ref is cleared on errors to allow retries.Key behaviors:
- Validates userId/username before proceeding
- Handles 404 as "new user" trigger
- Falls back to initial snapshot if persisted cards fetch fails
- Proper cleanup and retry capability
728-780: Good fallback handling for missing persisted cards.The code handles the edge case where the cards we just saved aren't immediately available (eventual consistency). Falling back to
buildInitialCardsSnapshot()ensures the UI is never left in an inconsistent state.The warning log and error tracking provide visibility into potential storage issues.
1043-1541: LGTM - Well-structured responsive UI.The main render includes:
- Accessible toolbar with proper ARIA roles
- Responsive filter controls (mobile popover vs desktop inline)
- Confirmation dialog for destructive reset action
- Motion animations for smooth transitions
- Proper empty state handling
Good use of
cn()for conditional styling and consistent dark mode support.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/api-utils.ts (1)
619-653: Validation gap: disabled cards can store unvalidated color data.The current logic skips color validation for disabled cards (line 639-653), which means a disabled card could be stored with malformed color values:
{ "cardName": "animeStats", "disabled": true, "titleColor": "invalid-color-format" }This payload would pass validation but store invalid color data. When the card is later enabled, the invalid colors would be used.
Recommendation: Consider validating color fields even for disabled cards, or explicitly strip/ignore color fields when
disabled: trueto prevent storing invalid data.Suggested validation improvement
// Only validate color requirements when not disabled (disabled cards store minimal data) if (!isDisabled) { const rawPreset = card["colorPreset"]; const preset = typeof rawPreset === "string" && rawPreset.trim().length > 0 ? rawPreset : undefined; const requireColorFields = preset === undefined || preset === "custom"; if (requireColorFields) { const reqColorErr = validateRequiredColorFields( card, requiredColorFields, ); if (reqColorErr) return reqColorErr; } + } else { + // For disabled cards, validate that color fields are absent or valid if present + for (const field of requiredColorFields) { + const value = card[field]; + if (value !== undefined && value !== null && !validateColorValue(value)) { + const reason = getColorInvalidReason(value); + console.warn( + `⚠️ [${endpoint}] Disabled card ${cardIndex} has invalid color format for ${field} (${reason})`, + ); + return NextResponse.json( + { error: "Invalid data" }, + { status: 400, headers: apiJsonHeaders(request) }, + ); + } + } }
🧹 Nitpick comments (2)
components/user/CardTile.tsx (1)
136-142: Consider moving config defaults to the store layer.The component creates a default config object when the store doesn't have one. While this works, it places default-value responsibility in the component rather than the store. If defaults need to change or other components need the same defaults, you'll have duplication.
Consider having the store's selector or a getter method return a default config structure, keeping the source of truth centralized.
app/api/store-cards/route.ts (1)
351-391: Redundant validation: disabled field checked twice.The
validateDisabledBooleanFieldfunction (lines 356-391) validates that thedisabledfield is a boolean, but this validation is duplicated invalidateCardOptionalFields(lib/api-utils.ts, lines 674-692) which is called later byvalidateCardData.Validation flow:
- Line 416-422:
validateDisabledBooleanFieldchecks disabled is boolean- Line 425:
validateCardDatais called- Inside
validateCardData:validateCardOptionalFieldschecks disabled is boolean againRecommendation: Remove
validateDisabledBooleanFieldsincevalidateCardDataalready performs this validation. If early validation is desired for better error messages, document the rationale in a comment.Suggested removal
- /** - * Validates that every provided `disabled` field (if present) is a boolean. - * Returns a 400 response via `jsonWithCors` when an invalid value is found and - * increments analytics to track failed requests. - */ - async function validateDisabledBooleanField( - incomingCards: unknown, - endpoint: string, - endpointKey: string, - request?: Request, - ): Promise<NextResponse | null> { - if (!Array.isArray(incomingCards)) return null; - - for (let i = 0; i < incomingCards.length; i++) { - const card = incomingCards[i]; - - // Skip non-object entries -- they will be rejected by validateCardData later. - if (typeof card !== "object" || card === null) continue; - - const cardObj = card as Record<string, unknown>; - if ( - Object.prototype.hasOwnProperty.call(cardObj, "disabled") && - typeof cardObj.disabled !== "boolean" - ) { - console.warn( - `⚠️ [${endpoint}] Card ${i} has non-boolean disabled field: ${String( - cardObj.disabled, - )}`, - ); - await incrementAnalytics( - buildAnalyticsMetricKey(endpointKey, "failed_requests"), - ); - return jsonWithCors( - { error: "Invalid 'disabled' field type" }, - request, - 400, - ); - } - } - return null; - }And remove the call at lines 416-422:
- const disabledError = await validateDisabledBooleanField( - incomingCards, - endpoint, - endpointKey, - request, - ); - if (disabledError) return disabledError; - // Validate incoming data and obtain typed cards on success const validated = validateCardData(
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/api/store-cards/route.tscomponents/stat-card-generator/ColorPickerGroup.tsxcomponents/user/CardTile.tsxlib/api-utils.tsopenapi.yamltests/unit/api/store-cards/store-cards.test.ts
🧰 Additional context used
🧬 Code graph analysis (4)
app/api/store-cards/route.ts (4)
lib/card-data/validation.ts (1)
displayNames(79-134)lib/types/records.ts (3)
StoredCardConfig(445-462)GlobalCardSettings(465-479)CardsRecord(482-487)lib/utils.ts (2)
safeParse(769-802)clampBorderRadius(870-877)lib/api-utils.ts (2)
incrementAnalytics(364-371)buildAnalyticsMetricKey(379-387)
tests/unit/api/store-cards/store-cards.test.ts (2)
app/api/store-cards/route.ts (1)
POST(399-548)lib/card-data/validation.ts (1)
displayNames(79-134)
components/stat-card-generator/ColorPickerGroup.tsx (1)
lib/types/card.ts (1)
GradientStop(5-14)
lib/api-utils.ts (1)
lib/types/records.ts (1)
StoredCardConfig(445-462)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: E2E Mobile Chrome
- GitHub Check: E2E Chromium
🔇 Additional comments (17)
components/stat-card-generator/ColorPickerGroup.tsx (5)
238-240: LGTM!The initial state correctly ensures each stop has an ID by mapping over the incoming
stopsprop and falling back togenerateStopId()when theidfield is absent.
266-275: LGTM!The
handleStopChangecallback correctly operates onlocalStops, updates internal state, and propagates changes viaonStopsChange. The dependency array is appropriate.
277-289: LGTM!The
addStopfunction correctly:
- Guards against exceeding 5 stops using
localStops.length- Generates a unique ID for the new stop
- Updates both local state and propagates via callback
291-299: LGTM!The
removeStopfunction properly guards against removing stops when only 2 remain, operates onlocalStops, and propagates changes correctly.
340-341: LGTM!The rendering logic correctly derives disabled states and iterates over
localStops. The ARIA label dynamically reflects the current stop count.Also applies to: 352-352, 435-435
components/user/CardTile.tsx (1)
210-228: LGTM! Clipboard error handling properly implemented.The try-catch block correctly handles potential clipboard API failures. Error logging provides debugging visibility while the UI gracefully handles failures. This addresses the concern from the previous review.
openapi.yaml (1)
670-672: LGTM! Clean schema addition for disabled cards.The
disabledfield is well-documented and appropriately optional, allowing cards to be marked as hidden/disabled by users without requiring changes to existing stored configurations.lib/api-utils.ts (3)
12-12: Good addition for type safety.Importing
StoredCardConfigenables the improved return type forvalidateCardData, addressing the past review concern about type assertions bypassing static validation.
674-692: Correct validation for the disabled field.The disabled field is properly validated as an optional boolean alongside other optional boolean fields like
showFavorites,useStatusColors, andshowPiePercentages.
1008-1092: Excellent improvement addressing past review feedback!The updated
validateCardDatasignature now returnsStoredCardConfig[]on success instead ofnull, eliminating the type assertion issue flagged in the previous review (see past review comment at lines 410-411 in app/api/store-cards/route.ts).Benefits:
- Callers receive typed data directly without unsafe assertions
- Static type checking now covers the validated payload
- The typed construction (lines 1046-1089) properly handles all fields including the new
disabledanduseCustomSettingspropertiesThe implementation comprehensively maps all fields with appropriate type coercion (
coerceNumhelper for numeric fields, explicit boolean checks, and preservation of ColorValue types for color fields).app/api/store-cards/route.ts (6)
1-19: LGTM! Imports properly support new global settings model.The updated imports bring in necessary types (
GlobalCardSettings,StoredCardConfig,CardsRecord) and utilities (displayNamesfor card type enumeration,isValidCardTypefor validation) to support the enhanced per-card and global settings architecture.
52-69: Good backfilling strategy for data completeness.This function ensures that every supported card type exists in storage, adding missing types with
disabled: true. This prevents partial records and simplifies downstream logic that expects all card types to be present.The approach aligns with test expectations (e.g., test at lines 407-422 verifies backfilling behavior).
126-255: Well-structured helper functions for settings merging.These helpers provide clean separation of concerns:
parseExistingGlobalSettings: Safe extraction with fallback to undefined on parse errorscomputeShowPiePercentages(lines 166-186): Correctly considers both incoming and previous variation to determine if pie settings should be saved, addressing the edge case where disabling a card might omit variationcomputeShowFavorites: Properly checks card capability before mergingclampGridDim: Robust clamping with finite number checksmergeGlobalAdvancedSettings: Coherent merge strategy that prefers incoming values and applies clampingThe logic correctly handles all edge cases including undefined values and out-of-range inputs.
257-320: Complex but well-structured card config builder.
buildCardConfighandles intricate merging logic for per-card settings:
- useCustomSettings semantics (lines 274-282): Defaults to
true, determines whether to save color data at all- Color preset handling (lines 278-282): Individual colors only saved when
colorPresetis undefined or "custom"- Disabled card preservation (lines 295-296):
disabled: trueflag preserved, allowing settings to persist through disable/enable cycles- Border handling (lines 289-291): Per-card
borderColoronly saved whenuseCustomSettingsis true, independent of global border state- Advanced settings (lines 313-317): Properly computes favorites, status colors, and pie percentages using dedicated helpers
The logic correctly merges incoming and previous values while respecting the new per-card/global settings model.
322-349: Clean helper functions for card application and border radius computation.
- applyIncomingCards (lines 322-334): Straightforward iteration that applies incoming cards by merging with existing configs via
buildCardConfig- computeEffectiveBorderRadius (lines 336-349): Correctly computes border radius only when borders are enabled, with proper clamping and fallback to existing values
Both functions are simple, focused, and correctly implemented.
399-548: Comprehensive POST flow integrating global settings and typed validation.The updated POST handler successfully integrates:
- Global settings extraction (line 411): Accepts optional
globalSettingsin request body- Typed validation (lines 425-437): Uses updated
validateCardDatathat returns typed array, eliminating unsafe type assertions- Existing data merging (lines 460-476): Safely parses existing cards and global settings
- Card application (lines 483-486): Applies incoming cards and ensures all supported types exist
- Global settings computation (lines 488-519): Merges incoming and existing global settings with proper clamping and conditional logic
- Persistence (lines 521-530): Stores complete record with merged global settings
Key improvements:
- Line 431:
if (!Array.isArray(validated))correctly distinguishes validation failure (NextResponse) from success (typed array)- Lines 488-519: Complex but correct global settings merging with clamping for borderRadius and grid dimensions
- Line 524: globalSettings properly included in persisted record
The flow handles all edge cases including missing data, out-of-range values, and disabled cards.
tests/unit/api/store-cards/store-cards.test.ts (1)
1-1770: Excellent comprehensive test coverage for new features.The test suite thoroughly validates the enhanced global/per-card settings model:
Input Validation Coverage:
- Non-object card entries (lines 129-141)
- Non-boolean disabled fields (lines 171-193)
- Null cards arrays (line 122)
- Invalid JSON bodies (lines 155-169)
Disabled Cards Coverage:
- Acceptance and persistence (lines 314-341)
- Settings preservation when disabled (lines 343-405)
- Backfilling all supported types when empty (lines 407-422)
Color & Settings Coverage:
- Color preset handling (lines 847-911)
- useCustomSettings=false skips color persistence (lines 912-941)
- useCustomSettings=true preserves colors (lines 943-964)
Border Settings Coverage:
- Global borderEnabled interactions (lines 968-1055)
- Per-card border preservation when globally disabled (lines 1057-1137)
- Border radius clamping and merging (lines 1262-1294)
- useCustomSettings affects per-card borders (lines 1218-1260)
Global Advanced Settings Coverage:
- Persistence of useStatusColors, showPiePercentages, showFavorites, grid dims (lines 1298-1333)
- Preservation when incoming omits them (lines 1335-1372)
- Clamping of out-of-range grid dimensions (lines 1374-1427)
Assertions verify:
- Stored cards length matches
Object.keys(displayNames).length(lines 288, 332, 416, 474, 711, etc.)- Specific cards exist with expected properties
- Backfilling creates disabled entries for missing types
- Merging preserves previous values when incoming omits them
The tests provide strong confidence in correctness and edge-case handling.
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai configuration |
|
Configuration used: defaults CodeRabbit configurationlanguage: en-US
tone_instructions: ''
early_access: false
enable_free_tier: true
inheritance: false
reviews:
profile: chill
request_changes_workflow: false
high_level_summary: true
high_level_summary_instructions: ''
high_level_summary_placeholder: '@coderabbitai summary'
high_level_summary_in_walkthrough: false
auto_title_placeholder: '@coderabbitai'
auto_title_instructions: ''
review_status: true
commit_status: true
fail_commit_status: false
collapse_walkthrough: true
changed_files_summary: true
sequence_diagrams: true
estimate_code_review_effort: true
assess_linked_issues: true
related_issues: true
related_prs: true
suggested_labels: true
labeling_instructions: []
auto_apply_labels: false
suggested_reviewers: true
auto_assign_reviewers: false
in_progress_fortune: true
poem: true
enable_prompt_for_ai_agents: true
path_filters: []
path_instructions: []
abort_on_close: true
disable_cache: false
auto_review:
enabled: true
auto_incremental_review: true
ignore_title_keywords: []
labels: []
drafts: false
base_branches: []
ignore_usernames: []
finishing_touches:
docstrings:
enabled: true
unit_tests:
enabled: true
pre_merge_checks:
docstrings:
mode: warning
threshold: 80
title:
mode: warning
requirements: ''
description:
mode: warning
issue_assessment:
mode: warning
custom_checks: []
tools:
ast-grep:
rule_dirs: []
util_dirs: []
essential_rules: true
packages: []
shellcheck:
enabled: true
ruff:
enabled: true
markdownlint:
enabled: true
github-checks:
enabled: true
timeout_ms: 90000
languagetool:
enabled: true
enabled_rules: []
disabled_rules: []
enabled_categories: []
disabled_categories: []
enabled_only: false
level: default
biome:
enabled: true
hadolint:
enabled: true
swiftlint:
enabled: true
phpstan:
enabled: true
level: default
phpmd:
enabled: true
phpcs:
enabled: true
golangci-lint:
enabled: true
yamllint:
enabled: true
gitleaks:
enabled: true
checkov:
enabled: true
detekt:
enabled: true
eslint:
enabled: true
flake8:
enabled: true
fortitudeLint:
enabled: true
rubocop:
enabled: true
buf:
enabled: true
regal:
enabled: true
actionlint:
enabled: true
pmd:
enabled: true
clang:
enabled: true
cppcheck:
enabled: true
semgrep:
enabled: true
circleci:
enabled: true
clippy:
enabled: true
sqlfluff:
enabled: true
prismaLint:
enabled: true
pylint:
enabled: true
oxc:
enabled: true
shopifyThemeCheck:
enabled: true
luacheck:
enabled: true
brakeman:
enabled: true
dotenvLint:
enabled: true
htmlhint:
enabled: true
checkmake:
enabled: true
osvScanner:
enabled: true
chat:
art: true
auto_reply: true
integrations:
jira:
usage: auto
linear:
usage: auto
knowledge_base:
opt_out: false
web_search:
enabled: true
code_guidelines:
enabled: true
filePatterns: []
learnings:
scope: auto
issues:
scope: auto
jira:
usage: auto
project_keys: []
linear:
usage: auto
team_keys: []
pull_requests:
scope: auto
mcp:
usage: auto
disabled_servers: []
code_generation:
docstrings:
language: en-US
path_instructions: []
unit_tests:
path_instructions: []
issue_enrichment:
auto_enrich:
enabled: true
planning:
enabled: true
auto_planning:
enabled: true
labels: []
labeling:
labeling_instructions: []
auto_apply_labels: false
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/stat-card-generator/ColorPickerGroup.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/stat-card-generator/ColorPickerGroup.tsx (1)
lib/types/card.ts (1)
GradientStop(5-14)
🔇 Additional comments (2)
components/stat-card-generator/ColorPickerGroup.tsx (2)
261-294: LGTM! Callbacks correctly use localStops.All three callbacks (
handleStopChange,addStop,removeStop) correctly:
- Read from and update
localStopsstate- Include
localStopsin their dependency arrays- Propagate changes through
onStopsChangeThis ensures the local state stays in sync with user interactions.
335-347: LGTM! UI correctly renders from localStops.The component now renders based on
localStopsrather than thestopsprop, ensuring:
- Users see their changes immediately without waiting for parent re-render
- Disabled states (add/remove buttons) reflect the current local state
- ARIA labels provide accurate context
This is the correct pattern for controlled components with internal state management.
Also applies to: 430-430
…function Added a default case to throw an error for unhandled block types, ensuring better error handling and type safety.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/user/UserPageEditor.tsxcomponents/user/editor/EditorTour.tsxcomponents/user/help/user-help-topics.tscomponents/user/hooks/useCardFiltering.ts
🧰 Additional context used
🧬 Code graph analysis (1)
components/user/editor/EditorTour.tsx (1)
lib/utils/google-analytics.ts (2)
safeTrack(95-101)event(66-89)
🔇 Additional comments (9)
components/user/UserPageEditor.tsx (1)
352-1770: Excellent implementation with all past feedback addressed.The UserPageEditor component demonstrates high-quality React architecture:
- ✅ All 10 past review comments have been thoroughly addressed
- ✅ Custom hooks extracted for new-user setup, card filtering, and data loading (improving testability and reusability)
- ✅ Performance optimizations with stable selectors using
shallowEqual(lines 296-344)- ✅ Proper cleanup for event listeners (line 694) and timers (line 807)
- ✅ Comprehensive keyboard shortcuts with proper escape hatch logic
- ✅ URL state persistence with debouncing for shareable views
- ✅ Robust error handling with try-catch blocks and user-friendly toast notifications
- ✅ Strong accessibility support (ARIA attributes, keyboard navigation, screen reader labels)
- ✅ Draft management and conflict resolution for data integrity
The component is production-ready and demonstrates excellent attention to detail in addressing reviewer feedback.
components/user/help/user-help-topics.ts (3)
1-16: LGTM! Clean type definitions.The discriminated union for
UserHelpBlockand theUserHelpTopicinterface are well-structured. The optionaltourTargetfield is appropriately documented for guided tour integration.
18-42: Previous review feedback addressed — exhaustive handling confirmed.The default case with exhaustiveness checking (lines 34-37) fully addresses the defensive coding concern raised in the previous review. TypeScript will now catch unhandled block types at compile time, and the runtime error provides additional safety.
79-103: All documented keyboard shortcuts (Ctrl/Cmd+F, Ctrl/Cmd+Shift+F, Ctrl/Cmd+E, Ctrl/Cmd+S, Ctrl/Cmd+A, Esc, Ctrl/Cmd+D, Ctrl/Cmd+H) are correctly implemented inUserPageEditor.tsxand match the documentation. The keyboard handler properly checks forctrlKey || metaKey, respects input fields and open dialogs, and maps all shortcuts to their corresponding actions.components/user/editor/EditorTour.tsx (5)
1-10: LGTM! Clean imports and versioning.The imports are well-organized, and the storage version constant provides a good migration path for future tour changes.
11-40: Excellent fallback strategy.The fallback mapping ensures the tour remains functional even when certain UI elements are conditionally hidden or disabled. The hierarchical fallback chains are well thought out.
42-78: LGTM! Well-structured helper functions.The selector resolution logic is clean and maintainable. Breaking it into small, focused functions makes the code easy to test and reason about.
87-297: Excellent work addressing all past review feedback.All three previous review comments have been properly resolved:
- ✅ Mounted check added (lines 290-297, used at line 314)
- ✅ Tour completion vs dismissal now tracked separately (lines 270-278, 341-365)
- ✅ Auto-run delay documented (line 396)
The storage patterns, error handling, and state management are all robust.
378-409: LGTM! Proper cleanup and auto-run logic.The cleanup effects correctly handle all timers and driver instances. The auto-run logic has appropriate guards and the 500ms delay is now properly documented. The returned API surface is clean and sufficient.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
components/user/UserPageEditor.tsx (1)
558-567: Add eslint-disable comment for intentional dependency omission.A past review requested an eslint-disable comment to document why
query,visibility, andselectedGroupare intentionally omitted from the dependency array. This effect should only sync state from URL whensearchParamschanges (back/forward navigation), not when local state changes.🔎 Add documentation comment
// Sync state from URL search params when the URL changes (back/forward etc.) + // eslint-disable-next-line react-hooks/exhaustive-deps -- Only sync when URL changes, not on local state changes useEffect(() => { const q = searchParams.get("q") ?? ""; const v = parseVisibilityParam(searchParams.get("visibility")); const g = searchParams.get("group") ?? "All"; if (q !== query) setQuery(q); if (v !== visibility) setVisibility(v); if (g !== selectedGroup) setSelectedGroup(g); }, [searchParams]);
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/user/UserPageEditor.tsxcomponents/user/editor/EditorTour.tsxcomponents/user/editor/tour-utils.tscomponents/user/hooks/useCardFiltering.ts
🧰 Additional context used
🧬 Code graph analysis (1)
components/user/editor/EditorTour.tsx (2)
lib/utils/google-analytics.ts (2)
safeTrack(95-101)event(66-89)components/user/editor/tour-utils.ts (1)
shouldAutoStartTour(10-32)
🔇 Additional comments (13)
components/user/UserPageEditor.tsx (1)
351-1773: Excellent implementation with comprehensive features.The UserPageEditor component is well-architected with:
- ✅ Extracted custom hooks for better testability and reusability
- ✅ Comprehensive keyboard shortcuts with proper accessibility
- ✅ Robust state management using Zustand with memoized selectors
- ✅ Draft backup and conflict resolution flows
- ✅ Bulk operations with undo/redo support
- ✅ Proper loading, error, and empty states
- ✅ Reorder mode with drag-and-drop functionality
- ✅ URL-driven filtering for shareable views
- ✅ Responsive design with mobile/desktop layouts
- ✅ Safe handling of user input
- ✅ Good separation of concerns
The implementation follows React and accessibility best practices throughout. Past review concerns have been effectively addressed.
components/user/editor/tour-utils.ts (2)
1-8: Well-documented cooldown constant.The 30-day cooldown period is reasonable for preventing tour auto-restarts after user dismissal, and the documentation clearly explains the purpose and storage mechanism.
10-32: Robust auto-start logic with proper edge-case handling.The function correctly:
- Uses early returns for clarity
- Validates
lastDismissedAtwith both null andNumber.isFinitechecks before arithmetic- Calculates the cooldown window correctly
- Provides testable defaults for
nowMsandcooldownMsThe implementation is sound and aligns with the tour lifecycle requirements.
components/user/editor/EditorTour.tsx (10)
1-11: Good use of versioned storage.The versioned storage key (
TOUR_STORAGE_VERSION = "v1") allows clean schema evolution if tour requirements change, preventing conflicts with older stored state.
12-41: Robust fallback strategy for dynamic tour targets.The fallback configuration gracefully handles missing DOM elements (e.g., when card-specific controls aren't rendered). This prevents tour breakage and maintains a consistent user experience regardless of initial UI state.
43-79: Clean functional implementation of selector resolution.The helper functions correctly implement the fallback cascade:
toPopoverOnlyStepsafely removes the element property using destructuringresolveTourSelectorwalks the fallback chain- Resolution functions gracefully degrade to centered popovers when targets are unavailable
The pure-function approach keeps the logic testable and side-effect-free.
250-267: Resilient storage initialization with proper error handling.The effect correctly:
- Guards against missing
tourStorageKey- Wraps storage access in try-catch to handle private browsing or disabled storage
- Validates the dismissal timestamp with
Number.isFinitebefore using it- Provides sensible defaults on failure
269-313: Well-documented dismissal tracking integrated with analytics.The dismissal timestamp is now properly utilized:
- Stored for cooldown calculation (consumed by
shouldAutoStartTour)- Sent to analytics as an event label (line 308)
- Comment at lines 282-284 clearly explains the dual purpose
This addresses the previous concern about unused storage.
315-322: Proper mounted check prevents post-unmount state updates.The
isMountedRefpattern correctly prevents the timeout callback at line 339 from executing after component unmount. This addresses the race condition identified in previous reviews.
335-341: Good documentation of timing delays.The comments at lines 335 and 436 clearly explain why delays are necessary (allowing dialogs to close and UI to settle), addressing maintainability concerns from previous reviews.
411-420: Proper cleanup on unmount.The cleanup effect correctly tears down both the timer and the driver.js instance, preventing memory leaks and orphaned event listeners.
422-449: Correct integration with auto-start logic.The effect properly:
- Guards on
userIdpresence- Delegates eligibility checks to
shouldAutoStartTour(from tour-utils.ts)- Includes all necessary dependencies
- Provides cleanup to prevent stale timers
The 500ms delay (with explanatory comment at line 436) allows initial render to complete, preventing race conditions with loading UI states.
451-455: Clean public API surface.The hook exposes a minimal, focused interface: the ability to manually start the tour and query its state. This encapsulation keeps implementation details private and simplifies consumer code.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/user/UserPageEditor.tsxcomponents/user/editor/EditorTour.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/user/editor/EditorTour.tsx (2)
lib/utils/google-analytics.ts (2)
safeTrack(95-101)event(66-89)components/user/editor/tour-utils.ts (1)
shouldAutoStartTour(10-32)
🔇 Additional comments (9)
components/user/editor/EditorTour.tsx (1)
1-460: Excellent implementation addressing all previous feedback.The hook is well-structured with proper:
- Mounted checks preventing post-unmount callback execution (lines 315-322, 339)
- Completion vs dismissal distinction with separate tracking and analytics (lines 383-394)
- Clear documentation for dismissal timestamp purpose (lines 282-284)
- Property-based step matching replacing fragile reference equality (lines 373-378)
- Explanatory comment for auto-start delay (line 440)
Storage operations are safely wrapped, cleanup effects prevent leaks, and dependencies are correctly specified throughout.
components/user/UserPageEditor.tsx (8)
1-343: Excellent component foundation with performance optimizations.The imports, constants, helper components, and custom hooks are well-structured:
- Efficient use of
Setfor card type checks (STATUS_COLOR_CARDS, PIE_PERCENTAGE_CARDS, FAVORITES_CARDS)- Stable selector hooks (
useStableCardEnabledById,useStableCardCustomizedById) useshallowEqualto prevent unnecessary re-renders when derived state hasn't changed- Reorder mode components have proper accessibility attributes (aria-pressed, aria-keyshortcuts)
- Clean helper functions (getTooltipTriggerChild, ShortcutHint, parseVisibilityParam)
The architectural decisions here demonstrate strong attention to performance and accessibility.
351-550: Clean component architecture with well-integrated custom hooks.The main component setup demonstrates excellent practices:
- Optimized store subscriptions: Using
useShallowselector (lines 378-402) to subscribe only to needed state slices prevents unnecessary re-renders- Separation of concerns: Custom hooks (
useNewUserSetup,useCardFiltering,useUserDataLoader,useCardAutoSave) encapsulate complex logic, making the component easier to test and maintain- Proper memoization: Derived data like
allCardIds,cardMetaById, and preview items are memoized with correct dependencies- Clear reorder mode logic:
canEnterReorderMode(lines 435-441) has explicit constraints that make the business logic transparentThe component structure makes it easy to understand the data flow and state dependencies.
552-808: Robust effects and event handling with excellent UX considerations.The effects and handlers are well-implemented:
URL synchronization (lines 557-566):
- Correctly uses only
[searchParams]dependency to sync state from URL changes (back/forward navigation), not from local state changes- Bidirectional sync with debounced persistence (lines 787-808) provides shareable URLs without excessive history entries
Keyboard shortcuts (lines 568-703):
- Comprehensive handler supports Ctrl/Cmd+F (search), Ctrl/Cmd+H (help), Ctrl/Cmd+D (reorder), Ctrl/Cmd+A (select all), Ctrl/Cmd+E (enabled view), Ctrl/Cmd+S (save), and Esc
- Properly checks for typing context and open dialogs to avoid interfering with form inputs
- Prevents browser defaults for shortcuts we override (bookmarking, save page, etc.)
- Clean helper function decomposition improves readability
- Provides helpful toast feedback when reorder mode isn't available (lines 624-628)
Conflict and draft handlers (lines 722-781):
- Include proper error handling with try-catch blocks
- Display user-friendly error messages via toast
- Handle edge cases (e.g., checking for userId before operations)
The event handling is production-ready with good attention to edge cases and user feedback.
810-950: Well-crafted callbacks and excellent loading/error state UX.The remaining setup and conditional renders are polished:
Callbacks and memoization (lines 810-873):
- Proper use of
useCallbackanduseMemowith correct dependenciesactiveFilterCount(lines 817-824) cleverly counts active filters for "Clear filters" visibilityrenderCardTile(lines 846-868) conditionally passes drag props only in reorder mode, keeping the API cleanLoading states (lines 876-919):
- Phase-specific messages via
LOADING_PHASE_MESSAGESlookup provide transparency- Enhanced UI for new user onboarding (lines 890-912) with animated pulse effect creates a welcoming first impression
- Falls back to simple spinner for other phases
Error state (lines 921-950):
- Clear, actionable error UI with prominent "Something Went Wrong" heading
- Provides a recovery action (link to search) rather than leaving users stuck
- Consistent visual design with the loading state (rounded-3xl card, motion animation)
The loading and error handling demonstrate strong UX thinking and polish.
952-982: Clean component composition for header and notifications.The main layout structure is well-organized:
UserPageHeaderreceives essential user data and save stateUserHelpDialogis controlled via local state with proper two-way bindingEditorNoticesconsolidates all notification types (conflict, draft, new user, warnings) in a single component, keeping the main component cleanThe separation of concerns makes it easy to locate and modify specific UI sections.
984-1682: Comprehensive and accessible toolbar with excellent responsive design.The toolbar section is feature-rich and well-executed:
Action buttons (lines 1005-1157):
- Save/Discard/Help/Global Settings buttons with clear visual hierarchy
- Proper disabled states based on
canSaveNowandcanDiscardNowflags- Tooltips explain functionality and display keyboard shortcuts
getTooltipTriggerChildhelper ensures tooltips work even on disabled buttons- AlertDialog confirmation for discard action prevents accidental data loss
Filters toolbar (lines 1159-1630):
- Search: Placeholder shortened for mobile (line 1178), detailed syntax in title attribute (line 1181) — addresses mobile UX while preserving help for desktop
- Visibility toggle: Clean segmented control design with info button explaining disabled cards
- Customization filter: Allows filtering by whether cards have custom settings
- Live filter count: "Showing X of Y cards" provides immediate feedback (lines 1254-1259)
- Responsive bulk actions: Desktop shows all buttons inline (lines 1479-1548), mobile uses a popover menu (lines 1551-1627) — excellent responsive pattern
Confirmation dialogs (lines 1632-1678):
BulkConfirmDialogfor "Disable All" and "Reset All" shows preview of affected cards- Includes undo hint: "You can undo this afterwards"
Accessibility:
- Comprehensive keyboard shortcuts (Ctrl/Cmd+F, Ctrl/Cmd+H, Ctrl/Cmd+D, Ctrl/Cmd+E, Ctrl/Cmd+S, Ctrl/Cmd+A, Esc)
- aria-keyshortcuts attributes document shortcuts for assistive technology
- aria-pressed for toggle buttons
- Screen reader text via sr-only class
The toolbar provides power users with keyboard-driven efficiency while remaining approachable for new users through tooltips and visual feedback.
1684-1763: Polished empty state and card rendering with smooth animations.The conditional rendering for card groups is well-implemented:
Empty state (lines 1684-1719):
- Provides clear, friendly messaging: "No cards match your filters"
- Offers actionable recovery options (clear search, clear all filters, show all)
- Conditionally shows "Clear all filters" only when multiple filters are active (line 1701)
Card groups rendering (lines 1721-1763):
- Stats computation (lines 1724-1730): Intelligent fallback chain tries filtered totals first, then group totals, then computes on-the-fly — ensures correct counts in all scenarios
- Staggered animation:
delay: Math.min(index * 0.04, 0.25)(line 1737) creates a pleasant cascading reveal without excessive delay for long lists- Reorder support:
reorderable={isReorderMode}andonReorderhandler (lines 1753-1756) integrate drag-and-drop seamlessly- Layout key:
scrollMarginKey={layoutVersion}(line 1752) ensures scroll position updates when filtering changes layoutThe rendering logic handles both the empty state and populated states gracefully with attention to UX details.
1764-1772: Clean component closure with floating bulk actions toolbar.The structure is properly closed, and the
BulkActionsToolbaris rendered outside the main content flow (line 1769), which is appropriate for a component that likely uses fixed/sticky positioning to remain visible when cards are selected.The overall component demonstrates excellent React and UX practices throughout its 1772 lines.
…editor - Introduced CommandPalette component for enhanced command access - Integrated command palette with keyboard shortcuts and recent actions - Added loading and error screens for user page editor - Updated dependencies in package.json and bun.lock
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
components/user/CommandPalette.tsxcomponents/user/UserPageEditor.tsxcomponents/user/editor/EditorTour.tsxpackage.json
🔇 Additional comments (7)
components/user/CommandPalette.tsx (6)
1-14: LGTM!Clean imports with proper "use client" directive for Next.js. All imported modules are used appropriately.
15-39: LGTM!Well-structured type definitions with good JSDoc documentation. The
runfunction signature correctly supports both sync and async commands.
41-74: LGTM!Solid localStorage helpers with proper SSR safety (
globalThis.windowchecks), comprehensive type validation inreadRecentActions, and graceful error handling for quota/private-mode scenarios.
76-87: LGTM!Good defensive coding with the default case, and proper
aria-hidden="true"on decorative icons.
89-129: LGTM!Clean subcomponent with proper accessibility attributes and consistent dark mode styling. Using
Readonly<>for props is a good immutability pattern.
219-317: LGTM!Excellent accessibility implementation with
sr-onlyDialogTitle/Description for screen readers, properaria-labelon the input, and keyboard-hint styling. The conditional rendering of groups and recent commands is clean and well-structured.components/user/UserPageEditor.tsx (1)
1-2091: Excellent work incorporating all previous feedback! 🎉This is a comprehensive, well-structured component that demonstrates strong engineering practices:
✅ All 15 previous review comments have been addressed:
- Hooks extracted for better modularity (
useNewUserSetup,useCardFiltering,useUserDataLoader)- Shared
LoadingPhasetype imported fromlib/types/loading- Static icon mapping replaces
useCallback- URL sync uses extracted
syncFiltersFromSearchParamsutility- Proper shallow equality checks with
shallowEqualfrom zustand- Clean handler extraction (
handleDiscardChanges, etc.)- All dependency arrays correct
✅ Code quality highlights:
- Proper separation of concerns with custom hooks
- Comprehensive keyboard shortcuts (Ctrl/Cmd+K, F, H, S, D, E, Escape)
- Strong accessibility (ARIA labels, tooltips, screen reader support via live regions)
- Responsive design with mobile/desktop UI variants
- Robust error handling and user feedback (toasts, notices, conflict resolution)
- URL state persistence with debouncing
- Draft backup and restore flows
- Bulk actions with undo/redo
✅ Performance optimizations:
- Memoized selectors with
useShallow- Stable references for enabled/customized maps (lines 776-824)
- Proper
useCallback/useMemousage throughoutThe component is large (2091 lines) but justified by the feature-rich UI with extensive JSX markup, tooltips, and accessibility enhancements. The core logic is well-extracted into hooks.
Optional nitpick: Lines 1322-1324 and 1355-1358 use an array-indexing pattern
(["disabled", "enabled"] as const)[Number(canSaveNow)]which, while clever, could be slightly more readable as:{canSaveNow ? "enabled" : "disabled"}However, this is purely stylistic and the current approach is valid.
- Added pointer down event handling to improve command selection. - Implemented logic to synchronize selected command with search input.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/user/CommandPalette.tsxpackage.json
🧰 Additional context used
🧬 Code graph analysis (1)
components/user/CommandPalette.tsx (2)
lib/utils.ts (1)
cn(178-180)components/ui/Dialog.tsx (4)
Dialog(185-185)DialogContent(190-190)DialogTitle(193-193)DialogDescription(194-194)
🔇 Additional comments (7)
components/user/CommandPalette.tsx (7)
1-35: LGTM: Well-defined types and appropriate imports.The type definitions are clear and comprehensive. The
CommandPaletteCommandtype properly supports both synchronous and asynchronous command handlers, and the grouping system is well-structured.
37-87: LGTM: Robust helper functions with proper error handling.The localStorage helpers correctly handle SSR scenarios, validate data types, and gracefully handle persistence errors. The type guard and icon mapping functions are well-structured.
89-140: LGTM: Well-crafted command item with thoughtful interaction handling.The
handlePointerDownimplementation correctly prevents focus issues during mouse interaction, and the component properly supports both keyboard and pointer-based selection. Good accessibility practices throughout.
151-176: LGTM: Clean state management and initialization.The memoized command lookup map provides efficient access, and the effect correctly resets state when the palette opens, ensuring a consistent user experience.
214-230: Past review concern has been addressed.The previous review flagged potential unhandled promise rejections when calling
cmd.run(). The current implementation (lines 224-226) now properly wraps the call inPromise.resolve().then().catch(), which handles both synchronous exceptions and rejected promises. This correctly addresses the concern.
232-255: LGTM: Efficient computed values with proper filtering.The
recentCommandsmemo correctly handles cases where recent command IDs no longer exist in the command list, and thegroupedmemo efficiently organizes commands by category. Good use of memoization to prevent unnecessary recalculations.
257-360: LGTM: Well-structured render with excellent accessibility.The component properly uses Dialog and Command primitives, includes screen-reader-only labels, and conditionally renders sections based on content availability. The Recent section appropriately appears only when search is empty, and all groups follow a consistent rendering pattern.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@components/user/SettingsContent.tsx`:
- Around line 866-959: The number inputs for grid columns and rows currently
hardcode min/max as 1 and 5; update the Input components for
`${idPrefix}-gridCols` and `${idPrefix}-gridRows` to use the GRID_MIN and
GRID_MAX constants for their min and max props (and any related validation
messaging if needed) so the UI bounds stay consistent with the defined constants
used in validation (`gridInputs`, `isGridColsValid`, `isGridRowsValid`, and the
error text).
- Around line 106-111: handleColorPickerChange currently overwrites any existing
alpha channel because the HTML color picker returns only 6-digit hex; modify it
to preserve the alpha suffix when present by detecting the previous color's
8-digit form (use the existing color state, e.g., inputBorderColor, and the same
hex8 regex used elsewhere) and if it matches, append its AA suffix to the new
normalized 6-digit value before calling setInputBorderColor and
args.onBorderColorChange; keep setIsBorderColorValid(true) as-is.
In `@components/user/UserPageEditor.tsx`:
- Around line 77-79: The import of statCardTypes in UserPageEditor.tsx is tied
to the deprecated components/stat-card-generator module; move the card type list
into a shared lib (e.g., create lib/card-types.ts exporting statCardTypes) and
update UserPageEditor.tsx to import statCardTypes from that new lib path (and
add a re-export in components/stat-card-generator/constants if you need backward
compatibility). Ensure the symbol name statCardTypes remains unchanged and
update any other files that import from components/stat-card-generator/constants
to use the new lib export or the re-exported path.
- Around line 503-679: The keyboard shortcut hook
useUserPageEditorKeyboardShortcuts should accept and use a canSaveNow flag to
mirror the save button behavior: add a boolean canSaveNow property to the opts
parameter/type, check opts.canSaveNow inside handleSaveShortcut and only call
opts.saveNow() when true (otherwise just preventDefault/return), and include
opts.canSaveNow in the effect dependency array so the handler updates when that
flag changes; update any call sites of useUserPageEditorKeyboardShortcuts to
pass canSaveNow accordingly.
♻️ Duplicate comments (1)
components/user/UserPageHeader.tsx (1)
349-363: Addaria-hiddento decorative icons in the status output.
Screen readers may announce icon SVGs; hide them since the text already conveys status.✅ Suggested tweak
- <output + <output + role="status" aria-live="polite" aria-atomic="true" title={saveInfo.title} className={cn( "flex items-center gap-2 rounded-full px-4 py-2 text-sm font-medium transition-all", saveInfo.className, )} > {saveInfo.spinner ? ( - <Loader2 className="h-4 w-4 animate-spin" /> + <Loader2 className="h-4 w-4 animate-spin" aria-hidden="true" /> ) : ( - <saveInfo.Icon className="h-4 w-4" /> + <saveInfo.Icon className="h-4 w-4" aria-hidden="true" /> )} <span>{saveInfo.text}</span> </output>
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.github/prompts/add-educational-comments.prompt.mdcomponents/user/SettingsContent.tsxcomponents/user/UserPageEditor.tsxcomponents/user/UserPageHeader.tsxcomponents/user/tile/CardPreview.tsxhooks/useCardAutoSave.ts
💤 Files with no reviewable changes (1)
- .github/prompts/add-educational-comments.prompt.md
🧰 Additional context used
🧬 Code graph analysis (1)
components/user/UserPageHeader.tsx (2)
components/PageShell.tsx (3)
containerVariants(20-26)baseVariants(12-15)itemVariants(28-35)lib/utils.ts (1)
cn(178-180)
🔇 Additional comments (31)
components/user/UserPageHeader.tsx (6)
48-63: Animation variants are clear and consistent with base variants.
70-89: Time helpers handle edge cases cleanly (future timestamps, countdowns).
103-180: Save-state mapping is comprehensive and readable.
187-195: AniList URL builder safely handles usernames and IDs.
202-241: Avatar fallback + error reset logic looks solid.
257-283: Ticker effect only runs when needed and cleans up correctly.components/user/UserPageEditor.tsx (9)
104-117: Tooltip wrapper and shortcut hint components are concise and reusable.
124-188: Loading screen logic and messaging are well-structured.
190-217: Error screen UX is clean and actionable.
224-271: Capability sets + group icon mapping are clear and easy to extend.
273-327: URL parsing/sync helpers are well isolated and predictable.
329-501: Command palette wiring is clean and well memoized.
688-780: Reorder toggles are accessible and communicate state well.
782-831: Stable derived maps are a solid optimization.
1179-1193: URL sync is correctly debounced with cleanup.components/user/SettingsContent.tsx (16)
32-55: Canvas parsing helper is solid.Document guard plus cached canvas keeps allocations low and avoids SSR pitfalls.
57-66: Grid size parsing is robust.Trimming, integer checks, and bounds enforcement are all in place.
68-79: Border color pre-check matches allowed formats.Hex (with/without
#) and named colors are handled as expected.
165-253: Grid input state/validation flow looks correct.Change/blur handlers keep input values and validity in sync.
255-300: Color normalization for picker value is thorough.Handles 3/6/8‑digit inputs and named colors cleanly.
305-377: Settings interfaces are well documented.Props and data shapes are clear and readable.
407-435: Visibility defaults and merged advanced settings look correct.Resolved settings align with inheritance rules.
437-472: Advanced tab gating and picker config are clean.Derived picker configs are clear and stable.
474-485: Quick preset list filtering is straightforward.Filtering out missing presets keeps the UI consistent.
487-515: Validity tracking and grid setup are well wired.Internal validity state stays synchronized with inputs.
517-529: Overall validity aggregation is solid.Single validity signal is computed cleanly.
531-551: Tab layout adapts well to advanced visibility.Conditional third tab keeps the grid layout correct.
553-609: Colors tab composition is cohesive.Preview, quick presets, selector, and pickers are well organized.
611-776: Border tab interactions are consistent.Quick apply buttons and conditional settings pane feel coherent.
779-863: Advanced toggles wiring looks good.Switches are bound to resolved settings and handlers.
971-982: Reset action hookup is clean.Clear affordance and consistent styling.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const handleColorPickerChange = (value: string) => { | ||
| const normalized = value.toLowerCase(); | ||
| setInputBorderColor(normalized); | ||
| setIsBorderColorValid(true); | ||
| args.onBorderColorChange(normalized); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "SettingsContent.tsx" | head -20Repository: RLAlpha49/AniCards
Length of output: 99
🏁 Script executed:
cat -n components/user/SettingsContent.tsx | sed -n '80,130p'Repository: RLAlpha49/AniCards
Length of output: 1897
🌐 Web query:
HTML input type="color" support alpha channel 8-digit hex #RRGGBBAA``
💡 Result:
Short answer: Yes — the HTML spec now allows to use any CSS color notation (including 8‑digit hex #RRGGBBAA) and adds an experimental boolean alpha attribute to let users pick an alpha value. However this is recent/experimental and browser support varies, so older browsers will still accept only #RRGGBB. [1][2][3]
Sources:
- WHATWG HTML specification (color state, alpha/colorspace attributes). [1]
- WebKit blog: P3 and alpha color picker (Safari shipping alpha + colorspace). [2]
- MDN input[type="color"] overview (notes on CSS color values and experimental alpha/colorspace). [3]
🏁 Script executed:
cat -n components/user/SettingsContent.tsx | head -80Repository: RLAlpha49/AniCards
Length of output: 3009
🏁 Script executed:
rg -A 5 "handleColorPickerChange" components/user/SettingsContent.tsxRepository: RLAlpha49/AniCards
Length of output: 704
🏁 Script executed:
rg -B 5 "handleColorPickerChange" components/user/SettingsContent.tsxRepository: RLAlpha49/AniCards
Length of output: 658
🏁 Script executed:
rg "getColorPickerHex" components/user/SettingsContent.tsx -B 2 -A 5Repository: RLAlpha49/AniCards
Length of output: 715
🏁 Script executed:
rg "function getColorPickerHex|const getColorPickerHex" components/ -rRepository: RLAlpha49/AniCards
Length of output: 475
🏁 Script executed:
cat -n components/user/SettingsContent.tsx | sed -n '140,250p'Repository: RLAlpha49/AniCards
Length of output: 3610
🏁 Script executed:
cat -n components/user/SettingsContent.tsx | sed -n '260,330p'Repository: RLAlpha49/AniCards
Length of output: 2163
🏁 Script executed:
rg -B 10 "function getColorPickerHex" components/user/SettingsContent.tsxRepository: RLAlpha49/AniCards
Length of output: 339
🏁 Script executed:
rg "normalizeColorInput|validateColorValue" lib/ -A 10 | head -60Repository: RLAlpha49/AniCards
Length of output: 2938
🏁 Script executed:
rg "#RRGGBBAA|8-digit|alpha" components/user/SettingsContent.tsxRepository: RLAlpha49/AniCards
Length of output: 141
🏁 Script executed:
rg "#RRGGBBAA" components/user/SettingsContent.tsx -B 5 -A 5Repository: RLAlpha49/AniCards
Length of output: 587
🏁 Script executed:
cat lib/utils.ts | grep -A 50 "export function normalizeColorInput"Repository: RLAlpha49/AniCards
Length of output: 1372
Preserve alpha channel on color-picker changes.
Lines 106-111: The HTML color picker emits only 6-digit hex, so existing 8-digit colors (#RRGGBBAA) lose their alpha channel after any picker change. Since the codebase supports 8-digit colors elsewhere (see error message and regex at line 32), the alpha suffix should be preserved when present.
🐛 Suggested fix to retain alpha when present
const handleColorPickerChange = (value: string) => {
const normalized = value.toLowerCase();
+ const alphaMatch =
+ /^(?:#)?([0-9a-f]{8})$/i.exec(inputBorderColor.trim()) ??
+ /^(?:#)?([0-9a-f]{8})$/i.exec(args.borderColor.trim());
+ const alpha = alphaMatch ? alphaMatch[1].slice(6) : null;
+ const next = alpha ? `${normalized}${alpha}` : normalized;
- setInputBorderColor(normalized);
+ setInputBorderColor(next);
setIsBorderColorValid(true);
- args.onBorderColorChange(normalized);
+ args.onBorderColorChange(next);
};🤖 Prompt for AI Agents
In `@components/user/SettingsContent.tsx` around lines 106 - 111,
handleColorPickerChange currently overwrites any existing alpha channel because
the HTML color picker returns only 6-digit hex; modify it to preserve the alpha
suffix when present by detecting the previous color's 8-digit form (use the
existing color state, e.g., inputBorderColor, and the same hex8 regex used
elsewhere) and if it matches, append its AA suffix to the new normalized 6-digit
value before calling setInputBorderColor and args.onBorderColorChange; keep
setIsBorderColorValid(true) as-is.
| {visibility.showGridSize && ( | ||
| <div className="space-y-3 rounded-lg border border-slate-200/50 bg-slate-50/50 p-3 dark:border-slate-700/50 dark:bg-slate-900/50"> | ||
| <div className="flex items-center gap-2"> | ||
| <Grid className="h-4 w-4 text-slate-400" /> | ||
| <span className="text-sm font-medium text-slate-700 dark:text-slate-300"> | ||
| {mode === "global" ? "Favorites Grid Size" : "Grid Size"} | ||
| </span> | ||
| </div> | ||
| <div className="grid grid-cols-2 gap-3"> | ||
| <div> | ||
| <Label | ||
| htmlFor={`${idPrefix}-gridCols`} | ||
| className="mb-1 text-xs text-slate-500" | ||
| > | ||
| Columns | ||
| </Label> | ||
| <Input | ||
| id={`${idPrefix}-gridCols`} | ||
| type="number" | ||
| min={1} | ||
| max={5} | ||
| value={gridInputs.inputGridCols} | ||
| onChange={(e) => | ||
| gridInputs.handleGridColsChange(e.target.value) | ||
| } | ||
| onBlur={gridInputs.handleGridColsBlur} | ||
| aria-invalid={ | ||
| gridValidationEnabled && !gridInputs.isGridColsValid | ||
| } | ||
| aria-describedby={gridInputs.gridColsAriaDescribedBy} | ||
| className={ | ||
| gridValidationEnabled && !gridInputs.isGridColsValid | ||
| ? "h-9 border-red-500 text-sm focus-visible:ring-1 focus-visible:ring-red-500" | ||
| : "h-9 text-sm" | ||
| } | ||
| /> | ||
| <AnimatePresence> | ||
| {gridValidationEnabled && !gridInputs.isGridColsValid && ( | ||
| <motion.p | ||
| id={`${idPrefix}-gridCols-error`} | ||
| initial={{ opacity: 0, height: 0 }} | ||
| animate={{ opacity: 1, height: "auto" }} | ||
| exit={{ opacity: 0, height: 0 }} | ||
| className="mt-1 text-xs text-red-600" | ||
| > | ||
| Enter a whole number between {GRID_MIN} and {GRID_MAX} | ||
| . | ||
| </motion.p> | ||
| )} | ||
| </AnimatePresence> | ||
| </div> | ||
| <div> | ||
| <Label | ||
| htmlFor={`${idPrefix}-gridRows`} | ||
| className="mb-1 text-xs text-slate-500" | ||
| > | ||
| Rows | ||
| </Label> | ||
| <Input | ||
| id={`${idPrefix}-gridRows`} | ||
| type="number" | ||
| min={1} | ||
| max={5} | ||
| value={gridInputs.inputGridRows} | ||
| onChange={(e) => | ||
| gridInputs.handleGridRowsChange(e.target.value) | ||
| } | ||
| onBlur={gridInputs.handleGridRowsBlur} | ||
| aria-invalid={ | ||
| gridValidationEnabled && !gridInputs.isGridRowsValid | ||
| } | ||
| aria-describedby={gridInputs.gridRowsAriaDescribedBy} | ||
| className={ | ||
| gridValidationEnabled && !gridInputs.isGridRowsValid | ||
| ? "h-9 border-red-500 text-sm focus-visible:ring-1 focus-visible:ring-red-500" | ||
| : "h-9 text-sm" | ||
| } | ||
| /> | ||
| <AnimatePresence> | ||
| {gridValidationEnabled && !gridInputs.isGridRowsValid && ( | ||
| <motion.p | ||
| id={`${idPrefix}-gridRows-error`} | ||
| initial={{ opacity: 0, height: 0 }} | ||
| animate={{ opacity: 1, height: "auto" }} | ||
| exit={{ opacity: 0, height: 0 }} | ||
| className="mt-1 text-xs text-red-600" | ||
| > | ||
| Enter a whole number between {GRID_MIN} and {GRID_MAX} | ||
| . | ||
| </motion.p> | ||
| )} | ||
| </AnimatePresence> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use GRID_MIN/GRID_MAX constants for input bounds.
Lines 883‑929 hardcode 1/5; using the constants avoids drift if limits change.
♻️ Suggested tweak for bound consistency
- <Input
- id={`${idPrefix}-gridCols`}
- type="number"
- min={1}
- max={5}
+ <Input
+ id={`${idPrefix}-gridCols`}
+ type="number"
+ min={GRID_MIN}
+ max={GRID_MAX}
value={gridInputs.inputGridCols}
onChange={(e) =>
gridInputs.handleGridColsChange(e.target.value)
}- <Input
- id={`${idPrefix}-gridRows`}
- type="number"
- min={1}
- max={5}
+ <Input
+ id={`${idPrefix}-gridRows`}
+ type="number"
+ min={GRID_MIN}
+ max={GRID_MAX}
value={gridInputs.inputGridRows}
onChange={(e) =>
gridInputs.handleGridRowsChange(e.target.value)
}📝 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.
| {visibility.showGridSize && ( | |
| <div className="space-y-3 rounded-lg border border-slate-200/50 bg-slate-50/50 p-3 dark:border-slate-700/50 dark:bg-slate-900/50"> | |
| <div className="flex items-center gap-2"> | |
| <Grid className="h-4 w-4 text-slate-400" /> | |
| <span className="text-sm font-medium text-slate-700 dark:text-slate-300"> | |
| {mode === "global" ? "Favorites Grid Size" : "Grid Size"} | |
| </span> | |
| </div> | |
| <div className="grid grid-cols-2 gap-3"> | |
| <div> | |
| <Label | |
| htmlFor={`${idPrefix}-gridCols`} | |
| className="mb-1 text-xs text-slate-500" | |
| > | |
| Columns | |
| </Label> | |
| <Input | |
| id={`${idPrefix}-gridCols`} | |
| type="number" | |
| min={1} | |
| max={5} | |
| value={gridInputs.inputGridCols} | |
| onChange={(e) => | |
| gridInputs.handleGridColsChange(e.target.value) | |
| } | |
| onBlur={gridInputs.handleGridColsBlur} | |
| aria-invalid={ | |
| gridValidationEnabled && !gridInputs.isGridColsValid | |
| } | |
| aria-describedby={gridInputs.gridColsAriaDescribedBy} | |
| className={ | |
| gridValidationEnabled && !gridInputs.isGridColsValid | |
| ? "h-9 border-red-500 text-sm focus-visible:ring-1 focus-visible:ring-red-500" | |
| : "h-9 text-sm" | |
| } | |
| /> | |
| <AnimatePresence> | |
| {gridValidationEnabled && !gridInputs.isGridColsValid && ( | |
| <motion.p | |
| id={`${idPrefix}-gridCols-error`} | |
| initial={{ opacity: 0, height: 0 }} | |
| animate={{ opacity: 1, height: "auto" }} | |
| exit={{ opacity: 0, height: 0 }} | |
| className="mt-1 text-xs text-red-600" | |
| > | |
| Enter a whole number between {GRID_MIN} and {GRID_MAX} | |
| . | |
| </motion.p> | |
| )} | |
| </AnimatePresence> | |
| </div> | |
| <div> | |
| <Label | |
| htmlFor={`${idPrefix}-gridRows`} | |
| className="mb-1 text-xs text-slate-500" | |
| > | |
| Rows | |
| </Label> | |
| <Input | |
| id={`${idPrefix}-gridRows`} | |
| type="number" | |
| min={1} | |
| max={5} | |
| value={gridInputs.inputGridRows} | |
| onChange={(e) => | |
| gridInputs.handleGridRowsChange(e.target.value) | |
| } | |
| onBlur={gridInputs.handleGridRowsBlur} | |
| aria-invalid={ | |
| gridValidationEnabled && !gridInputs.isGridRowsValid | |
| } | |
| aria-describedby={gridInputs.gridRowsAriaDescribedBy} | |
| className={ | |
| gridValidationEnabled && !gridInputs.isGridRowsValid | |
| ? "h-9 border-red-500 text-sm focus-visible:ring-1 focus-visible:ring-red-500" | |
| : "h-9 text-sm" | |
| } | |
| /> | |
| <AnimatePresence> | |
| {gridValidationEnabled && !gridInputs.isGridRowsValid && ( | |
| <motion.p | |
| id={`${idPrefix}-gridRows-error`} | |
| initial={{ opacity: 0, height: 0 }} | |
| animate={{ opacity: 1, height: "auto" }} | |
| exit={{ opacity: 0, height: 0 }} | |
| className="mt-1 text-xs text-red-600" | |
| > | |
| Enter a whole number between {GRID_MIN} and {GRID_MAX} | |
| . | |
| </motion.p> | |
| )} | |
| </AnimatePresence> | |
| </div> | |
| </div> | |
| {visibility.showGridSize && ( | |
| <div className="space-y-3 rounded-lg border border-slate-200/50 bg-slate-50/50 p-3 dark:border-slate-700/50 dark:bg-slate-900/50"> | |
| <div className="flex items-center gap-2"> | |
| <Grid className="h-4 w-4 text-slate-400" /> | |
| <span className="text-sm font-medium text-slate-700 dark:text-slate-300"> | |
| {mode === "global" ? "Favorites Grid Size" : "Grid Size"} | |
| </span> | |
| </div> | |
| <div className="grid grid-cols-2 gap-3"> | |
| <div> | |
| <Label | |
| htmlFor={`${idPrefix}-gridCols`} | |
| className="mb-1 text-xs text-slate-500" | |
| > | |
| Columns | |
| </Label> | |
| <Input | |
| id={`${idPrefix}-gridCols`} | |
| type="number" | |
| min={GRID_MIN} | |
| max={GRID_MAX} | |
| value={gridInputs.inputGridCols} | |
| onChange={(e) => | |
| gridInputs.handleGridColsChange(e.target.value) | |
| } | |
| onBlur={gridInputs.handleGridColsBlur} | |
| aria-invalid={ | |
| gridValidationEnabled && !gridInputs.isGridColsValid | |
| } | |
| aria-describedby={gridInputs.gridColsAriaDescribedBy} | |
| className={ | |
| gridValidationEnabled && !gridInputs.isGridColsValid | |
| ? "h-9 border-red-500 text-sm focus-visible:ring-1 focus-visible:ring-red-500" | |
| : "h-9 text-sm" | |
| } | |
| /> | |
| <AnimatePresence> | |
| {gridValidationEnabled && !gridInputs.isGridColsValid && ( | |
| <motion.p | |
| id={`${idPrefix}-gridCols-error`} | |
| initial={{ opacity: 0, height: 0 }} | |
| animate={{ opacity: 1, height: "auto" }} | |
| exit={{ opacity: 0, height: 0 }} | |
| className="mt-1 text-xs text-red-600" | |
| > | |
| Enter a whole number between {GRID_MIN} and {GRID_MAX} | |
| . | |
| </motion.p> | |
| )} | |
| </AnimatePresence> | |
| </div> | |
| <div> | |
| <Label | |
| htmlFor={`${idPrefix}-gridRows`} | |
| className="mb-1 text-xs text-slate-500" | |
| > | |
| Rows | |
| </Label> | |
| <Input | |
| id={`${idPrefix}-gridRows`} | |
| type="number" | |
| min={GRID_MIN} | |
| max={GRID_MAX} | |
| value={gridInputs.inputGridRows} | |
| onChange={(e) => | |
| gridInputs.handleGridRowsChange(e.target.value) | |
| } | |
| onBlur={gridInputs.handleGridRowsBlur} | |
| aria-invalid={ | |
| gridValidationEnabled && !gridInputs.isGridRowsValid | |
| } | |
| aria-describedby={gridInputs.gridRowsAriaDescribedBy} | |
| className={ | |
| gridValidationEnabled && !gridInputs.isGridRowsValid | |
| ? "h-9 border-red-500 text-sm focus-visible:ring-1 focus-visible:ring-red-500" | |
| : "h-9 text-sm" | |
| } | |
| /> | |
| <AnimatePresence> | |
| {gridValidationEnabled && !gridInputs.isGridRowsValid && ( | |
| <motion.p | |
| id={`${idPrefix}-gridRows-error`} | |
| initial={{ opacity: 0, height: 0 }} | |
| animate={{ opacity: 1, height: "auto" }} | |
| exit={{ opacity: 0, height: 0 }} | |
| className="mt-1 text-xs text-red-600" | |
| > | |
| Enter a whole number between {GRID_MIN} and {GRID_MAX} | |
| . | |
| </motion.p> | |
| )} | |
| </AnimatePresence> | |
| </div> | |
| </div> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In `@components/user/SettingsContent.tsx` around lines 866 - 959, The number
inputs for grid columns and rows currently hardcode min/max as 1 and 5; update
the Input components for `${idPrefix}-gridCols` and `${idPrefix}-gridRows` to
use the GRID_MIN and GRID_MAX constants for their min and max props (and any
related validation messaging if needed) so the UI bounds stay consistent with
the defined constants used in validation (`gridInputs`, `isGridColsValid`,
`isGridRowsValid`, and the error text).
| import { DISABLED_CARD_INFO } from "@/lib/card-info-tooltips"; | ||
| import { statCardTypes } from "@/components/stat-card-generator/constants"; | ||
| import { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Decouple card type constants from the legacy generator path.
Importing statCardTypes from components/stat-card-generator/constants ties the editor to a deprecated module. Consider moving the list to a shared lib module (e.g., lib/card-types.ts) and re-exporting it from there to avoid future breakage as the generator is removed.
🤖 Prompt for AI Agents
In `@components/user/UserPageEditor.tsx` around lines 77 - 79, The import of
statCardTypes in UserPageEditor.tsx is tied to the deprecated
components/stat-card-generator module; move the card type list into a shared lib
(e.g., create lib/card-types.ts exporting statCardTypes) and update
UserPageEditor.tsx to import statCardTypes from that new lib path (and add a
re-export in components/stat-card-generator/constants if you need backward
compatibility). Ensure the symbol name statCardTypes remains unchanged and
update any other files that import from components/stat-card-generator/constants
to use the new lib export or the re-exported path.
| function useUserPageEditorKeyboardShortcuts(opts: { | ||
| canEnterReorderMode: boolean; | ||
| isReorderMode: boolean; | ||
| setIsReorderMode: React.Dispatch<React.SetStateAction<boolean>>; | ||
| clearSelection: () => void; | ||
| selectAllEnabled: () => void; | ||
| visibility: VisibilityFilter; | ||
| setVisibility: React.Dispatch<React.SetStateAction<VisibilityFilter>>; | ||
| saveNow: () => void | Promise<void>; | ||
| setIsHelpDialogOpen: React.Dispatch<React.SetStateAction<boolean>>; | ||
| setIsCommandPaletteOpen: React.Dispatch<React.SetStateAction<boolean>>; | ||
| searchRef: React.RefObject<HTMLInputElement | null>; | ||
| groupFilterTriggerId: string; | ||
| }): void { | ||
| useEffect(() => { | ||
| const isTypingInField = (target: EventTarget | null) => { | ||
| if (!(target instanceof Element)) return false; | ||
| return ( | ||
| target instanceof HTMLInputElement || | ||
| target instanceof HTMLTextAreaElement || | ||
| (target as HTMLElement).isContentEditable | ||
| ); | ||
| }; | ||
|
|
||
| const hasAnyOpenDialog = () => | ||
| Boolean( | ||
| globalThis.document?.querySelector( | ||
| '[data-state="open"][role="dialog"], [data-state="open"][role="alertdialog"]', | ||
| ), | ||
| ); | ||
|
|
||
| const handleEscape = (e: KeyboardEvent, key: string) => { | ||
| if (key !== "escape") return false; | ||
| if (isTypingInField(e.target) || hasAnyOpenDialog()) return true; | ||
|
|
||
| const hasSelection = | ||
| useUserPageEditor.getState().selectedCardIds.size > 0; | ||
| if (!hasSelection && !opts.isReorderMode) return true; | ||
|
|
||
| e.preventDefault(); | ||
| if (hasSelection) { | ||
| opts.clearSelection(); | ||
| } else if (opts.isReorderMode) { | ||
| opts.setIsReorderMode(false); | ||
| } | ||
| return true; | ||
| }; | ||
| const handleFindShortcut = (e: KeyboardEvent) => { | ||
| e.preventDefault(); | ||
|
|
||
| if (e.shiftKey) { | ||
| const el = globalThis.document?.getElementById( | ||
| opts.groupFilterTriggerId, | ||
| ) as HTMLButtonElement | null; | ||
| el?.focus(); | ||
| } else { | ||
| opts.searchRef.current?.focus(); | ||
| } | ||
| }; | ||
|
|
||
| const handleHelpShortcut = (e: KeyboardEvent) => { | ||
| e.preventDefault(); | ||
| opts.setIsHelpDialogOpen(true); | ||
| }; | ||
|
|
||
| const handleReorderShortcut = (e: KeyboardEvent) => { | ||
| // Allow exiting even if filters have been applied. | ||
| // If reordering isn't available, block the browser bookmark shortcut | ||
| // and provide a gentle hint. | ||
| if (!opts.isReorderMode && !opts.canEnterReorderMode) { | ||
| e.preventDefault(); | ||
| toast("Reorder mode is disabled while filters are active.", { | ||
| id: "reorder-mode-unavailable", | ||
| description: | ||
| "Clear the search box and set visibility to All to reorder.", | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| e.preventDefault(); | ||
| const next = opts.isReorderMode ? false : opts.canEnterReorderMode; | ||
| opts.setIsReorderMode(next); | ||
| }; | ||
|
|
||
| const handleSelectAllShortcut = (e: KeyboardEvent) => { | ||
| e.preventDefault(); | ||
| opts.selectAllEnabled(); | ||
| }; | ||
|
|
||
| const handleEnabledOnlyShortcut = (e: KeyboardEvent) => { | ||
| e.preventDefault(); | ||
| opts.setVisibility(opts.visibility === "enabled" ? "all" : "enabled"); | ||
| }; | ||
|
|
||
| const handleSaveShortcut = (e: KeyboardEvent) => { | ||
| e.preventDefault(); | ||
| opts.saveNow(); | ||
| }; | ||
|
|
||
| const modChordHandlers: Record<string, (e: KeyboardEvent) => void> = { | ||
| h: handleHelpShortcut, | ||
| d: handleReorderShortcut, | ||
| a: handleSelectAllShortcut, | ||
| e: handleEnabledOnlyShortcut, | ||
| s: handleSaveShortcut, | ||
| }; | ||
|
|
||
| const handleModChord = (e: KeyboardEvent, key: string) => { | ||
| const isMod = e.ctrlKey || e.metaKey; | ||
| if (!isMod || e.altKey) return false; | ||
|
|
||
| // Ctrl/Cmd+K is commonly used for a command palette. Prevent the browser | ||
| // default even when focused in an input. | ||
| if (!e.shiftKey && key === "k") { | ||
| e.preventDefault(); | ||
| if (!hasAnyOpenDialog()) { | ||
| opts.setIsCommandPaletteOpen(true); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| // Don't allow browser-level defaults for shortcuts we advertise. | ||
| // (e.g., Ctrl/Cmd+D = bookmark, Ctrl/Cmd+H = history, Ctrl/Cmd+S = save page) | ||
| const shouldBlockBrowserDefault = | ||
| !e.shiftKey && (key === "d" || key === "h" || key === "s"); | ||
|
|
||
| if (hasAnyOpenDialog()) { | ||
| if (shouldBlockBrowserDefault) { | ||
| e.preventDefault(); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| if (isTypingInField(e.target)) { | ||
| if (shouldBlockBrowserDefault) { | ||
| e.preventDefault(); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| if (key === "f") { | ||
| handleFindShortcut(e); | ||
| return true; | ||
| } | ||
| if (e.shiftKey) return false; | ||
|
|
||
| const action = modChordHandlers[key]; | ||
| if (!action) return false; | ||
| action(e); | ||
| return true; | ||
| }; | ||
|
|
||
| const handler = (e: KeyboardEvent) => { | ||
| if (e.defaultPrevented || e.repeat) return; | ||
|
|
||
| const key = e.key.toLowerCase(); | ||
| if (handleEscape(e, key)) return; | ||
| handleModChord(e, key); | ||
| }; | ||
|
|
||
| globalThis.addEventListener("keydown", handler); | ||
| return () => globalThis.removeEventListener("keydown", handler); | ||
| }, [ | ||
| opts.canEnterReorderMode, | ||
| opts.clearSelection, | ||
| opts.groupFilterTriggerId, | ||
| opts.isReorderMode, | ||
| opts.saveNow, | ||
| opts.searchRef, | ||
| opts.selectAllEnabled, | ||
| opts.setIsCommandPaletteOpen, | ||
| opts.setIsHelpDialogOpen, | ||
| opts.setIsReorderMode, | ||
| opts.setVisibility, | ||
| opts.visibility, | ||
| ]); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the hook and inspect saveNow behavior
echo "=== Searching for useCardAutoSave or similar hooks ==="
fd -e ts -e tsx | xargs rg "saveNow|canSaveNow|isSaving" -l | head -20Repository: RLAlpha49/AniCards
Length of output: 320
🏁 Script executed:
#!/bin/bash
# More specific search for useCardAutoSave
fd "useCardAutoSave" -e ts -e tsxRepository: RLAlpha49/AniCards
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Search for useUserPageEditorKeyboardShortcuts to find where it's called and what opts are passed
rg "useUserPageEditorKeyboardShortcuts" -A 10 -B 2Repository: RLAlpha49/AniCards
Length of output: 1825
🏁 Script executed:
#!/bin/bash
# Look for saveNow implementation in UserPageEditor.tsx
rg "saveNow" components/user/UserPageEditor.tsx -B 3 -A 5Repository: RLAlpha49/AniCards
Length of output: 3899
🏁 Script executed:
cat -n hooks/useCardAutoSave.tsRepository: RLAlpha49/AniCards
Length of output: 25233
🏁 Script executed:
# Also search for canSaveNow definition in UserPageEditor to understand when saves are allowed
rg "canSaveNow" components/user/UserPageEditor.tsx -B 2 -A 2Repository: RLAlpha49/AniCards
Length of output: 1576
Consider passing canSaveNow to the keyboard shortcuts hook for consistency.
While saveNow() already guards against redundant API calls via an internal isSaving check, the save button respects canSaveNow to provide UI feedback. For consistency and to avoid unnecessary function invocations from the keyboard shortcut, consider gating the shortcut handler on the same condition.
🔧 Suggested guard
-function useUserPageEditorKeyboardShortcuts(opts: {
+function useUserPageEditorKeyboardShortcuts(opts: {
canEnterReorderMode: boolean;
isReorderMode: boolean;
setIsReorderMode: React.Dispatch<React.SetStateAction<boolean>>;
clearSelection: () => void;
selectAllEnabled: () => void;
visibility: VisibilityFilter;
setVisibility: React.Dispatch<React.SetStateAction<VisibilityFilter>>;
saveNow: () => void | Promise<void>;
+ canSaveNow: boolean;
setIsHelpDialogOpen: React.Dispatch<React.SetStateAction<boolean>>;
setIsCommandPaletteOpen: React.Dispatch<React.SetStateAction<boolean>>;
searchRef: React.RefObject<HTMLInputElement | null>;
groupFilterTriggerId: string;
}): void {
useEffect(() => {
// ...
const handleSaveShortcut = (e: KeyboardEvent) => {
e.preventDefault();
- opts.saveNow();
+ if (opts.canSaveNow) opts.saveNow();
}; useUserPageEditorKeyboardShortcuts({
canEnterReorderMode,
isReorderMode,
setIsReorderMode,
clearSelection,
selectAllEnabled,
visibility,
setVisibility,
saveNow,
+ canSaveNow,
setIsHelpDialogOpen,
setIsCommandPaletteOpen,
searchRef,
groupFilterTriggerId,
});🤖 Prompt for AI Agents
In `@components/user/UserPageEditor.tsx` around lines 503 - 679, The keyboard
shortcut hook useUserPageEditorKeyboardShortcuts should accept and use a
canSaveNow flag to mirror the save button behavior: add a boolean canSaveNow
property to the opts parameter/type, check opts.canSaveNow inside
handleSaveShortcut and only call opts.saveNow() when true (otherwise just
preventDefault/return), and include opts.canSaveNow in the effect dependency
array so the handler updates when that flag changes; update any call sites of
useUserPageEditorKeyboardShortcuts to pass canSaveNow accordingly.
Summary by CodeRabbit
New Features
Changes
Removed
✏️ Tip: You can customize this high-level summary in your review settings.