Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds collection and season shortcut interactions plus preview-vs-select semantics across collected-stats components; extends DisplaySeason and header metric types with Changes
Sequence DiagramsequenceDiagram
participant User
participant Header as CollectedStatsHeader
participant Season as CollectedStatsSeasonTile
participant Stats as UserPageCollectedStats
participant Parent as UserPageCollected
participant Router as Router
User->>Header: hover metric
Header->>Stats: onPreview(collection?)
Stats->>Parent: set preferredSeasonPreview / preview state
User->>Header: click metric
Header->>Parent: onCollectionShortcut(collection)
Parent->>Parent: getCollectionUpdate(...)
Parent->>Router: router.replace / updateFields
User->>Season: hover tile
Season->>Stats: onPreview()
Stats->>Parent: preview season
User->>Season: click tile
Season->>Stats: onPreview()
Season->>Stats: onSelect()
Stats->>Parent: onSeasonShortcut(seasonNumber)
Parent->>Parent: getCollectionUpdate(...)
Parent->>Router: router.replace / updateFields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/user/collected/stats/subcomponents/CollectedStatsSeasonTile.tsx`:
- Around line 122-125: The click handler handleClick should not call onPreview
because previewing happens on hover/focus; change handleClick to only invoke
onSelect (i.e., remove the onPreview() call) so clicks only trigger selection.
Apply the same change to the other similar click handler in this component (the
second occurrence around the other tile handler) so no click path invokes
onPreview; keep onPreview only on hover/focus handlers.
In `@components/user/collected/UserPageCollected.tsx`:
- Around line 590-591: The active tile should use the optimistic season state so
UI updates immediately when setSzn() changes filters.szn; change the prop usage
in UserPageCollected so activeSeasonNumber reads the optimistic value
(filters.szn) falling back to filters.initialSznId if empty/undefined — update
the activeSeasonNumber prop passed to the tile/component to use filters.szn ||
filters.initialSznId and ensure the receiving component (activeSeasonNumber)
still accepts the same type.
In `@components/user/collected/UserPageCollectedStats.tsx`:
- Around line 109-139: The component needs to reset its local
tile-preview/detail state whenever the active season filter changes so a
previously-expanded tile (kept by preferredActiveSeasonId) doesn't remain open
for a newly selected season; add a useEffect that watches activeSeasonFilterId
and selectedActiveSeasonId (or activeSeasonNumber) and calls the existing local
preview reset setter (e.g., clearPreview, setExpandedTileId, or equivalent state
used to track the detail/preview UI) to null/false; ensure you import useEffect
(and useState/useId if you add state) at top if not already present so the
preview state is cleared whenever active season filter or selectedActiveSeasonId
changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97a269f4-4a90-422d-88a7-e984a603c669
📒 Files selected for processing (10)
__tests__/components/user/collected/CollectedStatsSeasonTile.test.tsx__tests__/components/user/collected/UserPageCollected.test.tsx__tests__/components/user/collected/UserPageCollectedStats.test.tsxcomponents/user/collected/UserPageCollected.tsxcomponents/user/collected/UserPageCollectedStats.tsxcomponents/user/collected/stats/helpers.tscomponents/user/collected/stats/subcomponents/CollectedStatsHeader.tsxcomponents/user/collected/stats/subcomponents/CollectedStatsSeasonTile.tsxcomponents/user/collected/stats/subcomponents/CollectedStatsSeasons.tsxcomponents/user/collected/stats/types.ts
Signed-off-by: ragnep <ragneinfo@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/user/collected/UserPageCollected.tsx`:
- Around line 304-325: setCollection and setCollectionShortcut only update
URL/search params, causing local filters (used by UserPageCollectedStats) to
remain stale; before calling updateFields(...) call the local state setter
(setFilters) to mirror the new collection and also clear filters.szn when the
collection is being reset (i.e., collection === null), then proceed to call
updateFields(getCollectionUpdateItems({ collection, allowToggle: true })); apply
this change in both setCollection and setCollectionShortcut so UI reads the
updated filters immediately.
- Around line 327-343: The season shortcut currently checks only
filters.initialSznId so it ignores any locally staged selection; in
setSeasonShortcut replace the comparison with the optimistic id (filters.szn?.id
?? filters.initialSznId) to decide isActiveSeasonShortcut, and when applying the
shortcut also stage the chosen season into the optimistic filters state so the
UI updates immediately (i.e., update the same filters.szn or equivalent staged
filter alongside calling updateFields); reference setSeasonShortcut and
filters.szn to locate where to change the comparison and where to stage the
optimistic value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ac6cc4c-68b0-4b4c-89e6-883fa82df25d
📒 Files selected for processing (6)
__tests__/components/user/collected/CollectedStatsSeasonTile.test.tsx__tests__/components/user/collected/UserPageCollected.test.tsx__tests__/components/user/collected/UserPageCollectedStats.test.tsxcomponents/user/collected/UserPageCollected.tsxcomponents/user/collected/UserPageCollectedStats.tsxcomponents/user/collected/stats/subcomponents/CollectedStatsSeasonTile.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
components/user/collected/UserPageCollected.tsx (2)
403-442:⚠️ Potential issue | 🟠 MajorShortcut handlers still skip the optimistic
filtersstate.
UserPageCollectedStatsnow renders pressed state from localfilters, butsetCollection*()andsetSeasonShortcut()only mutate the URL. Until the search-param round trip completes, the header state and cards query still read stale local values, andsetSeasonShortcut()still decides “already active” from URL state instead of the staged local season. Mirror the collection/szn changes intosetFilters()first, including clearing any staged season when the collection is reset, then callupdateFields(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/collected/UserPageCollected.tsx` around lines 403 - 442, setCollection, setCollectionShortcut and setSeasonShortcut currently only call updateFields and leave the local filters state stale; before calling updateFields, call setFilters(...) to mirror the intended changes into the local filters so UI/press states read the staged values. Specifically, in setCollection and setCollectionShortcut update filters.collection to the new collection (and clear filters.initialSznId when collection is set to null), and in setSeasonShortcut compute the intended collection and season using the existing filters plus the new season, update filters.collection and filters.initialSznId accordingly (clearing staged season when collection is reset), then call updateFields(getCollectionUpdateItems(...)) as before. Make sure to use the same allowToggle/value logic used to build getCollectionUpdateItems so the optimistic UI matches the URL update.
689-690:⚠️ Potential issue | 🟡 Minor
??masks an optimistic clear.After
setSzn(null),filters.sznis intentionally cleared, but this expression immediately falls back to the oldinitialSznIduntil the router update lands. The cards query clears right away while the stats shortcut stays pressed. This needs a staged season id/sentinel in local state instead of deriving the active value with??.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/collected/UserPageCollected.tsx` around lines 689 - 690, The activeSeasonNumber prop currently uses "filters.szn?.id ?? filters.initialSznId", which masks the intended optimistic clear; introduce a local staged season sentinel in UserPageCollected (e.g., stagedSznId via useState) and use that to drive activeSeasonNumber (activeSeasonNumber={stagedSznId ?? filters.initialSznId}). Update the handlers that call setSzn so that when clearing you set stagedSznId to null immediately (so the UI reflects the cleared state) and when selecting a season you set stagedSznId to the selected id (and let the router update filters.szn asynchronously); keep filters.szn only as the canonical source from the router and remove the "?? filters.szn" pattern.components/user/collected/stats/subcomponents/CollectedStatsSeasonTile.tsx (1)
128-130:⚠️ Potential issue | 🟡 MinorPointer clicks still preview before selecting.
When
onSelectis present, a normal mouse click focuses the button beforeonClick, soonFocus={onPreview}still fires on the select path. That breaks the new preview-on-hover / select-on-click split in real browsers. Gate preview to keyboard focus, or only fall back to preview on click whenonSelectis absent. Please verify with a real browser oruserEvent.click, sincefireEvent.click()will miss this sequence here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/collected/stats/subcomponents/CollectedStatsSeasonTile.tsx` around lines 128 - 130, The preview is still triggered on mouse click because onFocus={onPreview} runs before onClick when onSelect is present; change the focus logic so preview only runs for keyboard focus or when no onSelect exists: implement a short-lived pointer interaction flag (set onPointerDown/onTouchStart and cleared after focus) and update the onFocus handler in CollectedStatsSeasonTile to call onPreview only if onSelect is undefined OR the focus was not preceded by a pointer interaction (i.e., keyboard-initiated focus). Keep existing onMouseEnter/hasTouchScreen behavior unchanged and ensure onClick/onSelect behavior remains the select path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/user/collected/UserPageCollected.tsx`:
- Line 212: getFilters() currently sets page using raw
Number.parseInt(pageParam, 10) which can produce NaN or non-positive values;
update getFilters() to normalize the parsed page: parse pageParam into a number,
if it's NaN or less than 1 set page to 1, otherwise use the parsed value. Ensure
this normalization mirrors the clamping logic in createQueryString() so the
initial query key/API request never uses NaN or 0; refer to getFilters(),
createQueryString(), and the pageParam variable when making the change.
---
Duplicate comments:
In `@components/user/collected/stats/subcomponents/CollectedStatsSeasonTile.tsx`:
- Around line 128-130: The preview is still triggered on mouse click because
onFocus={onPreview} runs before onClick when onSelect is present; change the
focus logic so preview only runs for keyboard focus or when no onSelect exists:
implement a short-lived pointer interaction flag (set onPointerDown/onTouchStart
and cleared after focus) and update the onFocus handler in
CollectedStatsSeasonTile to call onPreview only if onSelect is undefined OR the
focus was not preceded by a pointer interaction (i.e., keyboard-initiated
focus). Keep existing onMouseEnter/hasTouchScreen behavior unchanged and ensure
onClick/onSelect behavior remains the select path.
In `@components/user/collected/UserPageCollected.tsx`:
- Around line 403-442: setCollection, setCollectionShortcut and
setSeasonShortcut currently only call updateFields and leave the local filters
state stale; before calling updateFields, call setFilters(...) to mirror the
intended changes into the local filters so UI/press states read the staged
values. Specifically, in setCollection and setCollectionShortcut update
filters.collection to the new collection (and clear filters.initialSznId when
collection is set to null), and in setSeasonShortcut compute the intended
collection and season using the existing filters plus the new season, update
filters.collection and filters.initialSznId accordingly (clearing staged season
when collection is reset), then call updateFields(getCollectionUpdateItems(...))
as before. Make sure to use the same allowToggle/value logic used to build
getCollectionUpdateItems so the optimistic UI matches the URL update.
- Around line 689-690: The activeSeasonNumber prop currently uses
"filters.szn?.id ?? filters.initialSznId", which masks the intended optimistic
clear; introduce a local staged season sentinel in UserPageCollected (e.g.,
stagedSznId via useState) and use that to drive activeSeasonNumber
(activeSeasonNumber={stagedSznId ?? filters.initialSznId}). Update the handlers
that call setSzn so that when clearing you set stagedSznId to null immediately
(so the UI reflects the cleared state) and when selecting a season you set
stagedSznId to the selected id (and let the router update filters.szn
asynchronously); keep filters.szn only as the canonical source from the router
and remove the "?? filters.szn" pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf72e2c0-6718-4c9a-8614-a62fa12e9d5c
📒 Files selected for processing (7)
__tests__/components/user/collected/CollectedStatsSeasonTile.test.tsx__tests__/components/user/collected/UserPageCollected.test.tsx__tests__/components/user/collected/UserPageCollectedStats.test.tsxcomponents/user/collected/UserPageCollected.tsxcomponents/user/collected/stats/subcomponents/CollectedStatsSeasonTile.tsxcomponents/user/collected/stats/subcomponents/CollectedStatsSeasons.tsxcomponents/utils/select/dropdown/SeasonsGridDropdown.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/user/collected/UserPageCollected.tsx (1)
212-212:⚠️ Potential issue | 🟡 MinorNormalize non-positive
pagevalues ingetFilters().This line still allows non-positive page values through. A URL like
?page=0or?page=-1would store these values in filters, potentially causing invalid API requests before the URL rewrite occurs.🛡️ Proposed fix
- page: pageParam ? Number.parseInt(pageParam, 10) : 1, + page: Math.max(1, pageParam ? Number.parseInt(pageParam, 10) || 1 : 1),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/collected/UserPageCollected.tsx` at line 212, The getFilters() function currently sets page using pageParam ? Number.parseInt(pageParam, 10) : 1 which allows 0 or negative pages; change the logic to parse pageParam into an integer (use Number.parseInt or Number.parseInt(pageParam, 10)), then normalize by checking isNaN(parsed) || parsed < 1 and defaulting to 1; assign the normalized value to the page field so page is always a positive integer before being stored in filters or used in API calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/user/collected/UserPageCollected.tsx`:
- Around line 252-254: The page value is normalized for NaN but not clamped to a
minimum of 1 in createQueryString(); update createQueryString() so any parsed
page (from pageParam / parsedPage) is coerced to at least 1 (e.g., use
Math.max(1, parsedPage) or equivalent) before serializing, mirroring the
normalization used in getFilters(); reference the pageParam, parsedPage and page
variables and ensure the final page written into the query string is >= 1.
---
Duplicate comments:
In `@components/user/collected/UserPageCollected.tsx`:
- Line 212: The getFilters() function currently sets page using pageParam ?
Number.parseInt(pageParam, 10) : 1 which allows 0 or negative pages; change the
logic to parse pageParam into an integer (use Number.parseInt or
Number.parseInt(pageParam, 10)), then normalize by checking isNaN(parsed) ||
parsed < 1 and defaulting to 1; assign the normalized value to the page field so
page is always a positive integer before being stored in filters or used in API
calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bdb8a708-a329-4c56-b614-3c268237b8a0
📒 Files selected for processing (2)
__tests__/components/user/collected/UserPageCollected.test.tsxcomponents/user/collected/UserPageCollected.tsx
|
simo6529
approved these changes
Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests
Style