fix: Release 页面空状态图标居中 & 未读筛选标记已读后不立即消失#178
Merged
Merged
Conversation
…ure removal 1. Fix Package icon not centered in empty state — missing space between dark:text-text-secondary and mx-auto caused both classes to be invalid 2. In unread-only filter mode, marking a release as read no longer instantly removes it from the list. A snapshot of unread IDs is taken when entering unread mode and on refresh; items stay visible until the next page load Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
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:
📝 WalkthroughWalkthroughReleaseTimeline now reads ChangesUnread snapshot filtering & store wiring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/ReleaseTimeline.tsx (1)
388-399: 💤 Low valueConsider consolidating snapshot update logic.
This inline snapshot creation duplicates the logic in
updateUnreadSnapshot. While reading directly fromgetState()is more robust here (avoids stale closures), having two implementations increases maintenance burden.After relocating
updateUnreadSnapshot(per the earlier fix), consider whether you want to:
- Keep the inline approach here (more explicit about fresh state)
- Refactor
updateUnreadSnapshotto also read fromgetState()and call it hereEither approach works; this is just a consistency consideration.
🤖 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/ReleaseTimeline.tsx` around lines 388 - 399, The inline snapshot creation in the ReleaseTimeline component duplicates logic in updateUnreadSnapshot and should be consolidated: remove the local block that reads useAppStore.getState(), builds snapshot, and assigns unreadSnapshotRef.current, and instead call updateUnreadSnapshot() here; updateUnreadSnapshot should be modified to read fresh state via useAppStore.getState(), compute the Set of unread release IDs using releaseSubscriptions and state.readReleases, and assign unreadSnapshotRef.current so this call preserves the same fresh-state behavior as the original inline code (referencing releaseShowMode, unreadSnapshotRef, updateUnreadSnapshot, useAppStore, and releaseSubscriptions).
🤖 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/ReleaseTimeline.tsx`:
- Around line 60-68: updateUnreadSnapshot (the useCallback that reads
subscribedReleases and readReleases and assigns unreadSnapshotRef.current) is
declared before subscribedReleases is defined, causing a TDZ ReferenceError;
relocate the entire declaration of unreadSnapshotRef and updateUnreadSnapshot
(the useRef and the useCallback that references subscribedReleases and
readReleases) to after the subscribedReleases useMemo/definition (i.e., after
the subscribedReleases symbol is created) so the dependency array and the
callback closure reference existing bindings; keep the same logic and
dependencies (subscribedReleases, readReleases) when moving.
---
Nitpick comments:
In `@src/components/ReleaseTimeline.tsx`:
- Around line 388-399: The inline snapshot creation in the ReleaseTimeline
component duplicates logic in updateUnreadSnapshot and should be consolidated:
remove the local block that reads useAppStore.getState(), builds snapshot, and
assigns unreadSnapshotRef.current, and instead call updateUnreadSnapshot() here;
updateUnreadSnapshot should be modified to read fresh state via
useAppStore.getState(), compute the Set of unread release IDs using
releaseSubscriptions and state.readReleases, and assign
unreadSnapshotRef.current so this call preserves the same fresh-state behavior
as the original inline code (referencing releaseShowMode, unreadSnapshotRef,
updateUnreadSnapshot, useAppStore, and releaseSubscriptions).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f72d7f4d-3005-48d0-9cb1-e4458da7453b
📒 Files selected for processing (1)
src/components/ReleaseTimeline.tsx
…ogic 1. Move unreadSnapshotRef/updateUnreadSnapshot after subscribedReleases definition to avoid temporal dead zone ReferenceError 2. Refactor updateUnreadSnapshot to read readReleases via useAppStore.getState() for fresh state access, removing stale closure dependency 3. Replace inline snapshot code in handleRefresh with updateUnreadSnapshot() call Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The snapshot update used a useCallback with stale closure — subscribedReleases didn't include newly added releases yet when updateUnreadSnapshot() was called from handleRefresh. Replace the useCallback + manual call pattern with a useEffect that automatically rebuilds the snapshot whenever releases, releaseSubscriptions, includePreRelease, or readReleases change. This ensures the snapshot is always in sync with the current state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move releaseShowMode from local useState to Zustand store with IndexedDB persistence, matching the pattern used by releaseViewMode and other release page settings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
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 (2)
src/components/ReleaseTimeline.tsx (2)
578-578:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix dark-mode token on the empty-state icon.
tailwind.configdefines the color ascolors.text-quaternary, which corresponds to the Tailwind utilitytext-text-quaternary;dark:text-quaternarywon’t resolve to that token. Update the icon to usedark:text-text-quaternary.🎨 Proposed fix
- <Package className="w-16 h-16 text-gray-500 dark:text-quaternary mx-auto mb-4" /> + <Package className="w-16 h-16 text-gray-500 dark:text-text-quaternary mx-auto mb-4" />🤖 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/ReleaseTimeline.tsx` at line 578, The empty-state icon's dark-mode token is incorrect: update the Package element's className in ReleaseTimeline (the <Package ... /> JSX) to replace the "dark:text-quaternary" token with the correct Tailwind token "dark:text-text-quaternary" so it resolves to colors.text-quaternary in tailwind.config; keep the existing base color ("text-gray-500") and other classes unchanged.
199-266:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFix unread-mode snapshot so
filteredReleasesupdates after refresh (ReleaseTimeline)
unreadSnapshotRefis written in auseEffect, butfilteredReleasesis auseMemothat readsunreadSnapshotRef.currentwhile its dependency list excludes that snapshot/ref—ref writes won’t trigger recomputation, so refreshed/new unread releases can remain hidden in'unread'mode until another unrelated dependency changes.- The effect depends on
readReleases, which conflicts with the snapshot’s intended behavior (“stay visible until refresh/mode-switch”); keep the snapshot driven by refresh-relevant inputs only, and let render updates come from state.🐛 Proposed fix: store snapshot in state and drop `readReleases` from the deps
- const unreadSnapshotRef = useRef<Set<number>>(new Set()); - useEffect(() => { - const state = useAppStore.getState(); - const ids = new Set<number>(); - releases.forEach(r => { - if (releaseSubscriptions.has(r.repository.id) && - (includePreRelease || !r.prerelease) && - !state.readReleases.has(r.id)) { - ids.add(r.id); - } - }); - unreadSnapshotRef.current = ids; - }, [releases, releaseSubscriptions, includePreRelease, readReleases]); + const [unreadSnapshot, setUnreadSnapshot] = useState<Set<number>>(new Set()); + useEffect(() => { + const { readReleases: currentRead } = useAppStore.getState(); + const ids = new Set<number>(); + releases.forEach(r => { + if (releaseSubscriptions.has(r.repository.id) && + (includePreRelease || !r.prerelease) && + !currentRead.has(r.id)) { + ids.add(r.id); + } + }); + setUnreadSnapshot(ids); + }, [releases, releaseSubscriptions, includePreRelease]);const filteredReleases = useMemo(() => { if (releaseShowMode === 'unread') { - return preUnreadFilteredReleases.filter(({ release }) => unreadSnapshotRef.current.has(release.id)); + return preUnreadFilteredReleases.filter(({ release }) => unreadSnapshot.has(release.id)); } return preUnreadFilteredReleases; - }, [preUnreadFilteredReleases, releaseShowMode]); + }, [preUnreadFilteredReleases, releaseShowMode, unreadSnapshot]);
- Minor: the empty-state icon uses
dark:text-quaternary, but other usages use the configured tokendark:text-text-quaternary; align it todark:text-text-quaternary.🤖 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/ReleaseTimeline.tsx` around lines 199 - 266, The unread snapshot currently stored in unreadSnapshotRef is updated inside a useEffect that depends on readReleases, but filteredReleases (which reads unreadSnapshotRef.current) won't recompute when the ref changes; fix by storing the snapshot in React state (e.g. const [unreadSnapshot, setUnreadSnapshot] = useState<Set<number>>(new Set())) and update it in the effect driven only by refresh-relevant inputs (releases, releaseSubscriptions, includePreRelease) — drop readReleases from that effect's dependency list so the snapshot persists until a refresh/mode switch — then change filteredReleases to read unreadSnapshot (and include unreadSnapshot in its useMemo deps) instead of unreadSnapshotRef.current; also normalize the empty-state icon class from dark:text-quaternary to dark:text-text-quaternary.
🤖 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/ReleaseTimeline.tsx`:
- Line 578: The empty-state icon's dark-mode token is incorrect: update the
Package element's className in ReleaseTimeline (the <Package ... /> JSX) to
replace the "dark:text-quaternary" token with the correct Tailwind token
"dark:text-text-quaternary" so it resolves to colors.text-quaternary in
tailwind.config; keep the existing base color ("text-gray-500") and other
classes unchanged.
- Around line 199-266: The unread snapshot currently stored in unreadSnapshotRef
is updated inside a useEffect that depends on readReleases, but filteredReleases
(which reads unreadSnapshotRef.current) won't recompute when the ref changes;
fix by storing the snapshot in React state (e.g. const [unreadSnapshot,
setUnreadSnapshot] = useState<Set<number>>(new Set())) and update it in the
effect driven only by refresh-relevant inputs (releases, releaseSubscriptions,
includePreRelease) — drop readReleases from that effect's dependency list so the
snapshot persists until a refresh/mode switch — then change filteredReleases to
read unreadSnapshot (and include unreadSnapshot in its useMemo deps) instead of
unreadSnapshotRef.current; also normalize the empty-state icon class from
dark:text-quaternary to dark:text-text-quaternary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9657870-2e62-4e1a-9ac4-26776bbf25d8
📒 Files selected for processing (1)
src/components/ReleaseTimeline.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/ReleaseTimeline.tsx (1)
198-212: 🏗️ Heavy liftSnapshot timing may cause new releases to be missing after refresh in "unread" mode.
The
useEffectruns after render, butfilteredReleases(line 262) runs during render using the snapshot. Whenreleaseschanges from a refresh:
filteredReleasesmemo executes with the old snapshot- New unread releases are excluded (not in old snapshot)
- Effect updates snapshot post-render, but memo won't re-run
Consider using
useMemo(withoutreadReleasesin deps) to build the snapshot synchronously during render, and include it infilteredReleasesdependencies. This ensures new releases appear immediately while still preserving the "don't disappear on mark-as-read" behavior.♻️ Suggested refactor using useMemo
- const unreadSnapshotRef = useRef<Set<number>>(new Set()); - useEffect(() => { - const state = useAppStore.getState(); - const ids = new Set<number>(); - releases.forEach(r => { - if (releaseSubscriptions.has(r.repository.id) && - (includePreRelease || !r.prerelease) && - !state.readReleases.has(r.id)) { - ids.add(r.id); - } - }); - unreadSnapshotRef.current = ids; - }, [releases, releaseSubscriptions, includePreRelease, readReleases]); + // Snapshot rebuilds only on releases/subscription/prerelease changes, NOT on readReleases + const unreadSnapshot = useMemo(() => { + const state = useAppStore.getState(); + const ids = new Set<number>(); + releases.forEach(r => { + if (releaseSubscriptions.has(r.repository.id) && + (includePreRelease || !r.prerelease) && + !state.readReleases.has(r.id)) { + ids.add(r.id); + } + }); + return ids; + }, [releases, releaseSubscriptions, includePreRelease]);Then update
filteredReleasesto depend onunreadSnapshot:const filteredReleases = useMemo(() => { if (releaseShowMode === 'unread') { - return preUnreadFilteredReleases.filter(({ release }) => unreadSnapshotRef.current.has(release.id)); + return preUnreadFilteredReleases.filter(({ release }) => unreadSnapshot.has(release.id)); } return preUnreadFilteredReleases; - }, [preUnreadFilteredReleases, releaseShowMode]); + }, [preUnreadFilteredReleases, releaseShowMode, unreadSnapshot]);🤖 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/ReleaseTimeline.tsx` around lines 198 - 212, The unread snapshot is built in a useEffect (unreadSnapshotRef) which runs after render causing filteredReleases to run with a stale snapshot and miss new releases; replace that effect with a useMemo (e.g., const unreadSnapshot = useMemo(() => { ... }, [releases, releaseSubscriptions, includePreRelease])) that synchronously builds the Set of unread IDs during render (read from useAppStore.getState() inside the memo) and do NOT include readReleases in the memo deps so marking-as-read doesn't immediately remove items; then update filteredReleases to depend on unreadSnapshot instead of unreadSnapshotRef.current and remove the post-render useEffect.
🤖 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/store/useAppStore.ts`:
- Line 1271: Add the missing declaration for setReleaseShowMode to the
AppActions interface: declare setReleaseShowMode: (mode: 'all' | 'unread') =>
void; so the implemented action in useAppStore.ts (setReleaseShowMode) matches
the interface and TypeScript consumers like ReleaseTimeline can type-check
against AppActions; update the AppActions interface to include this method
signature.
---
Nitpick comments:
In `@src/components/ReleaseTimeline.tsx`:
- Around line 198-212: The unread snapshot is built in a useEffect
(unreadSnapshotRef) which runs after render causing filteredReleases to run with
a stale snapshot and miss new releases; replace that effect with a useMemo
(e.g., const unreadSnapshot = useMemo(() => { ... }, [releases,
releaseSubscriptions, includePreRelease])) that synchronously builds the Set of
unread IDs during render (read from useAppStore.getState() inside the memo) and
do NOT include readReleases in the memo deps so marking-as-read doesn't
immediately remove items; then update filteredReleases to depend on
unreadSnapshot instead of unreadSnapshotRef.current and remove the post-render
useEffect.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d182d5c-812a-4ca0-bf4a-57d3be995e5a
📒 Files selected for processing (3)
src/components/ReleaseTimeline.tsxsrc/store/useAppStore.tssrc/types/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/store/useAppStore.ts (1)
1271-1271:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing
setReleaseShowModetoAppActionsinterface.The action is implemented but not declared in the
AppActionsinterface (lines 82-217). Add the declaration for TypeScript type checking:setReleaseShowMode: (mode: 'all' | 'unread') => void;This should be added near line 184, alongside other Release Timeline View actions.
🤖 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/store/useAppStore.ts` at line 1271, The AppActions interface is missing the declaration for the implemented action setReleaseShowMode; add a method signature to the AppActions interface: declare setReleaseShowMode(mode: 'all' | 'unread'): void; — place this alongside the other Release Timeline View actions (near the existing related actions in the AppActions block) so TypeScript recognizes the implemented setReleaseShowMode function.
🤖 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/ReleaseTimeline.tsx`:
- Around line 198-212: The useEffect that builds unreadSnapshotRef currently
lists readReleases in its dependency array causing snapshots to rebuild on
mark-as-read; update the dependency array for the effect that sets
unreadSnapshotRef (the effect referencing releases, releaseSubscriptions,
includePreRelease, unreadSnapshotRef) by removing readReleases and instead
include releaseShowMode so the snapshot is rebuilt when switching display modes
but not when individual releases are marked read.
---
Duplicate comments:
In `@src/store/useAppStore.ts`:
- Line 1271: The AppActions interface is missing the declaration for the
implemented action setReleaseShowMode; add a method signature to the AppActions
interface: declare setReleaseShowMode(mode: 'all' | 'unread'): void; — place
this alongside the other Release Timeline View actions (near the existing
related actions in the AppActions block) so TypeScript recognizes the
implemented setReleaseShowMode function.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c92cad2a-e20a-4449-9a3b-a56887c98f8c
📒 Files selected for processing (3)
src/components/ReleaseTimeline.tsxsrc/store/useAppStore.tssrc/types/index.ts
1. Replace readReleases with releaseShowMode in useEffect dependency array to avoid rebuilding snapshot on mark-as-read (which would defeat the purpose of the snapshot); rebuild on mode switch instead 2. Add setReleaseShowMode to AppActions interface for type completeness Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The useEffect updates unreadSnapshotRef but refs don't trigger useMemo recalculation. Add snapshotVersion state as a bridge: the effect increments it after updating the ref, and filteredReleases depends on it to recompute. The effect deliberately excludes readReleases from its dependency array so that marking an item as read does NOT rebuild the snapshot (preserving the 'items stay visible until next refresh' behavior). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
修复内容
1. 空状态图标未居中
图标的 className 中 缺少空格，导致 无法生效，图标贴到了容器左侧。
修复: 补上空格 →
2. 仅未读模式标记已读后列表项立即消失
在未读筛选模式下，点击列表项标记已读后，该项会瞬间从列表中消失，用户无法继续操作。
修复: 引入 快照机制：
Summary by CodeRabbit
New Features
Bug Fixes