Refactor segmented control, schemas, and image selector#433
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughGeneralizes ThreeWayToggle to SegmentedControl, centralizes SortDirection into a shared schema and updates many schema modules, moves success-rate color logic to badge-colors, introduces PROFILE_ACCESS_REASONS and type, and adds IGDB as an image provider with direction-based animations. ChangesCore Refactorings and Generalizations
IGDB Image Provider Expansion
Estimated code review effort: Possibly related PRs:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
|
|
||
| export const ApiKeySortFieldSchema = z.enum(['name', 'createdAt', 'lastUsedAt', 'monthlyQuota']) | ||
| export const SortDirectionSchema = z.enum(['asc', 'desc']) | ||
| export { SortDirectionSchema } |
There was a problem hiding this comment.
just import this directly from schemas/common whereever it is needed. avoid renaming using as in the import unless it conflicts with anything
| import { AuditAction, AuditEntityType } from '@orm' | ||
|
|
||
| export const SortDirection = z.enum(['asc', 'desc']) | ||
| export { SortDirection } |
There was a problem hiding this comment.
| @@ -1,11 +1,14 @@ | |||
| import { z } from 'zod' | |||
|
|
|||
| export const SortDirection = z.enum(['asc', 'desc']) | |||
There was a problem hiding this comment.
maybe renaming this to SortDirectionSchema makes more sense and keeps it consistent everywhere
| import { SortDirection } from '@/schemas/common' | ||
|
|
||
| export const DeviceBrandSortField = z.enum(['name', 'devicesCount']) | ||
| export const SortDirection = z.enum(['asc', 'desc']) | ||
| export { SortDirection } |
There was a problem hiding this comment.
| ]) | ||
|
|
||
| export const SortDirection = z.enum(['asc', 'desc']) | ||
| export { SortDirection } |
There was a problem hiding this comment.
same here https://github.com/Producdevity/EmuReady/pull/433/changes#r3362439440 do not import and reexport unless the schema requires extension of changes
| // Type exports for repository use | ||
| // Use z.input for types that include defaults (what you pass in) | ||
| // Use z.output for types after defaults are applied (what you get out) | ||
| // TODO: figure out why we use z.infer | ||
| // The remaining schemas do not apply defaults or transforms, so z.infer matches their parsed shape. |
There was a problem hiding this comment.
get rid of slop comments
| export const PermissionSortField = z.enum(['label', 'key', 'category', 'createdAt', 'updatedAt']) | ||
|
|
||
| export const SortDirection = z.enum(['asc', 'desc']) | ||
| export { SortDirection } |
There was a problem hiding this comment.
|
|
||
| export const SoCSortField = z.enum(['name', 'manufacturer', 'devicesCount']) | ||
| export const SortDirection = z.enum(['asc', 'desc']) | ||
| export { SortDirection } |
There was a problem hiding this comment.
|
|
||
| export const SystemSortField = z.enum(['name', 'key', 'gamesCount']) | ||
| export const SortDirection = z.enum(['asc', 'desc']) | ||
| export { SortDirection } |
There was a problem hiding this comment.
same here and everywhere else where this pattern is repeated. https://github.com/Producdevity/EmuReady/pull/433/changes#r3362439440
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)
src/schemas/gpu.ts (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winImport
SortDirectionfrom the consolidated location.This file imports
SortDirectionfrom@/schemas/soc, but the consolidation effort moved the canonical definition to@/schemas/common. Other schema modules (apiAccess.ts,system.ts,user.ts) import directly from@/schemas/common.Update the import to match the consolidation pattern:
🔄 Recommended fix
-import { SortDirection } from '`@/schemas/soc`' +import { SortDirection } from '`@/schemas/common`'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/schemas/gpu.ts` at line 2, Update the import of SortDirection in src/schemas/gpu.ts to use the consolidated canonical module; replace the existing import from '`@/schemas/soc`' with an import from '`@/schemas/common`' so SortDirection is imported from the same central location used by apiAccess.ts, system.ts, and user.ts.
🧹 Nitpick comments (2)
src/components/ui/image-selectors/ImageSelectorSwitcher.tsx (1)
156-156: 💤 Low valueRemove redundant
customprop onAnimatePresence.The
customprop on line 156 is overridden by each childmotion.div's owncustomvalue (lines 160, 177, 193), so the parent's value is never used.♻️ Simplify by removing unused prop
- <AnimatePresence mode="wait" custom={imageServiceDirection[selectedService]}> + <AnimatePresence mode="wait">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/image-selectors/ImageSelectorSwitcher.tsx` at line 156, In ImageSelectorSwitcher, remove the redundant custom prop on the AnimatePresence component (the prop currently set to imageServiceDirection[selectedService]) because each child motion.div already provides its own custom value; update the JSX by deleting the custom attribute on AnimatePresence so the per-child custom values on the motion.div elements (the ones rendering each service) take effect without being overridden.src/schemas/apiAccess.ts (1)
2-9: 💤 Low valueNaming inconsistency across schema modules.
This module re-exports as
SortDirectionSchema(via import alias) while other modules (system.ts,user.ts, etc.) re-export asSortDirection. The aliasing maintains backwards compatibility with existing imports, but creates a naming discrepancy across the codebase.Consider renaming the re-export to
SortDirectionfor consistency with other schema modules, and update any downstream imports ofSortDirectionSchemafrom this module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/schemas/apiAccess.ts` around lines 2 - 9, Rename the re-exported SortDirectionSchema to match the codebase convention by exporting it as SortDirection instead of SortDirectionSchema: change the current import alias or re-export so the symbol exported from this module is SortDirection (referencing the existing SortDirectionSchema identifier), and then update any downstream imports that currently import SortDirectionSchema from this module to import SortDirection to maintain consistency with other schema modules (e.g., system.ts, user.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/ui/image-selectors/ImageSelectorSwitcher.tsx`:
- Around line 29-33: The current imageServiceDirection mapping is wrong because
it encodes absolute positions instead of transition direction; modify
ImageSelectorSwitcher to compute the transition direction dynamically from the
previous and new service indices (e.g., direction = newIndex - prevIndex), store
the previous index in a ref or state, and create a handler (replace direct
setSelectedService calls with handleServiceChange) that updates selectedService
and prev index and sets a numeric direction state; pass that direction into
AnimatePresence/motion via the custom prop (custom={direction}) and use that
custom value in your animation variants so enter/exit use the computed
transition direction for smooth left/right animations.
---
Outside diff comments:
In `@src/schemas/gpu.ts`:
- Line 2: Update the import of SortDirection in src/schemas/gpu.ts to use the
consolidated canonical module; replace the existing import from '`@/schemas/soc`'
with an import from '`@/schemas/common`' so SortDirection is imported from the
same central location used by apiAccess.ts, system.ts, and user.ts.
---
Nitpick comments:
In `@src/components/ui/image-selectors/ImageSelectorSwitcher.tsx`:
- Line 156: In ImageSelectorSwitcher, remove the redundant custom prop on the
AnimatePresence component (the prop currently set to
imageServiceDirection[selectedService]) because each child motion.div already
provides its own custom value; update the JSX by deleting the custom attribute
on AnimatePresence so the per-child custom values on the motion.div elements
(the ones rendering each service) take effect without being overridden.
In `@src/schemas/apiAccess.ts`:
- Around line 2-9: Rename the re-exported SortDirectionSchema to match the
codebase convention by exporting it as SortDirection instead of
SortDirectionSchema: change the current import alias or re-export so the symbol
exported from this module is SortDirection (referencing the existing
SortDirectionSchema identifier), and then update any downstream imports that
currently import SortDirectionSchema from this module to import SortDirection to
maintain consistency with other schema modules (e.g., system.ts, user.ts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8986b048-b4af-4618-8be5-f7c15825431b
📒 Files selected for processing (20)
src/app/games/components/GameFilters.tsxsrc/components/ui/SegmentedControl.tsxsrc/components/ui/SuccessRateBar.tsxsrc/components/ui/VoteButtons.tsxsrc/components/ui/image-selectors/ImageSelectorSwitcher.tsxsrc/components/ui/index.tssrc/schemas/apiAccess.tssrc/schemas/audit.tssrc/schemas/common.tssrc/schemas/deviceBrand.tssrc/schemas/game.tssrc/schemas/gpu.tssrc/schemas/permission.tssrc/schemas/soc.tssrc/schemas/system.tssrc/schemas/user.tssrc/server/services/user-profile.service.test.tssrc/server/services/user-profile.service.tssrc/utils/badge-colors.tssrc/utils/vote.ts
💤 Files with no reviewable changes (1)
- src/utils/vote.ts
| return permissionCategoryVariantMap[permissionCategory] || 'default' | ||
| } | ||
|
|
||
| export function getSuccessRateBarColor(rate: number): string { |
There was a problem hiding this comment.
this was called something in a useMemo, i don't think that is needed for this small function
…tor/code-quality-todos # Conflicts: # src/schemas/deviceBrand.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ui/image-selectors/ImageSelectorSwitcher.tsx (1)
57-130:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpose selected state on provider buttons for assistive tech.
The segmented buttons visually indicate selection, but they don’t expose state semantically. Add
aria-pressed(or tab semantics) so screen readers can identify the active provider (e.g., Line 57, Line 80, Line 106).Suggested patch
<button type="button" onClick={() => handleServiceChange('rawg')} + aria-pressed={selectedService === 'rawg'} className={cn( @@ <button type="button" onClick={() => handleServiceChange('tgdb')} + aria-pressed={selectedService === 'tgdb'} className={cn( @@ <button type="button" onClick={() => handleServiceChange('igdb')} + aria-pressed={selectedService === 'igdb'} className={cn(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/image-selectors/ImageSelectorSwitcher.tsx` around lines 57 - 130, The provider buttons in ImageSelectorSwitcher.tsx do not expose their selected state to assistive tech; update each button (the ones using onClick={() => handleServiceChange('rawg'|'tgdb'|'igdb')} and reading selectedService) to include aria-pressed={selectedService === 'rawg'|'tgdb'|'igdb'} so screen readers can detect the active provider; ensure the attribute value is a boolean expression matching the current selectedService for each respective button.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/ui/image-selectors/ImageSelectorSwitcher.tsx`:
- Around line 57-130: The provider buttons in ImageSelectorSwitcher.tsx do not
expose their selected state to assistive tech; update each button (the ones
using onClick={() => handleServiceChange('rawg'|'tgdb'|'igdb')} and reading
selectedService) to include aria-pressed={selectedService ===
'rawg'|'tgdb'|'igdb'} so screen readers can detect the active provider; ensure
the attribute value is a boolean expression matching the current selectedService
for each respective button.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 28010aa7-2c2d-45d4-a27a-2394fa01d097
📒 Files selected for processing (19)
src/components/ui/SuccessRateBar.tsxsrc/components/ui/image-selectors/ImageSelectorSwitcher.tsxsrc/schemas/apiAccess.tssrc/schemas/audit.tssrc/schemas/common.tssrc/schemas/cpu.tssrc/schemas/device.tssrc/schemas/deviceBrand.tssrc/schemas/emulator.tssrc/schemas/game.tssrc/schemas/gpu.tssrc/schemas/listingReport.tssrc/schemas/performanceScale.tssrc/schemas/permission.tssrc/schemas/soc.tssrc/schemas/system.tssrc/schemas/user.tssrc/schemas/userBan.tssrc/schemas/voteInvestigation.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/schemas/common.ts
Description
SegmentedControland updates its usage.Closes #410.
Type of change
Testing
./node_modules/.bin/vitest run src/server/services/user-profile.service.test.ts./node_modules/.bin/vitest run src/server/services/user-profile.service.test.ts src/server/repositories/device-brands.repository.test.ts./node_modules/.bin/eslint src/app/games/[id]/components/GameEditForm.tsx src/app/games/new/NewGamePage.tsx src/components/ui/image-selectors/ImageSelectorSwitcher.tsx./node_modules/.bin/eslint ../node_modules/.bin/tsc --noEmitChecklist