fix: preserve repository view state and stabilize sync interactions#60
Open
HuberttFox wants to merge 4 commits intoAmintaCCCP:mainfrom
Open
fix: preserve repository view state and stabilize sync interactions#60HuberttFox wants to merge 4 commits intoAmintaCCCP:mainfrom
HuberttFox wants to merge 4 commits intoAmintaCCCP:mainfrom
Conversation
📝 WalkthroughWalkthroughThe changes implement centralized category selection state management by migrating from local component state to persistent global store, enhance visual presentation with PNG favicon and image logo, and improve repository list scroll behavior with position restoration when categories or filters change. Changes
Sequence DiagramsequenceDiagram
participant User
participant CategorySidebar
participant App
participant Store as useAppStore
participant RepositoryList
participant ScrollMgmt as Scroll/Animation
User->>CategorySidebar: Select category
CategorySidebar->>Store: setSelectedCategory(category)
Store->>Store: Update selectedCategory<br/>Persist to localStorage
Store->>App: Notify subscribers
App->>App: Re-render with new<br/>selectedCategory from store
App->>RepositoryList: Pass updated selectedCategory
RepositoryList->>RepositoryList: Detect category change<br/>(previousCategoryRef)
RepositoryList->>ScrollMgmt: Save current scroll position
RepositoryList->>RepositoryList: Reset visibleCount to initial
RepositoryList->>ScrollMgmt: Animate scroll to top with<br/>requestAnimationFrame
ScrollMgmt->>RepositoryList: Complete scroll animation
RepositoryList->>User: Display filtered repositories
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
![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 `@src/components/LoginScreen.tsx`:
- Around line 71-74: The img in LoginScreen uses a relative src="./icon.png"
which breaks on non-root routes; update the img src to be base-aware by either
importing the asset into the component (e.g., import icon from '.../icon.png'
and assign that to the src) or by using the public base URL (e.g.,
process.env.PUBLIC_URL + '/icon.png') so the logo resolves correctly regardless
of route; modify the src attribute in the img element inside LoginScreen
accordingly.
In `@src/components/RepositoryList.tsx`:
- Around line 37-39: Ensure scroll restoration only occurs if the category
hasn't changed: when saving scroll state (use
savedScrollYRef/previousCategoryRef), record previousCategoryRef.current =
selectedCategory; when scheduling the restore via restoreScrollFrameRef, check
that previousCategoryRef.current === selectedCategory before applying
window.scrollTo and only then clear savedScrollYRef/restoreScrollFrameRef;
additionally, on category change update previousCategoryRef and cancel any
pending restore by calling cancelAnimationFrame(restoreScrollFrameRef.current)
and clearing savedScrollYRef/restoreScrollFrameRef so a late restore cannot jump
the user back to the old category.
In `@src/store/useAppStore.ts`:
- Around line 116-117: Persisted selectedCategory can be stale; update the
rehydration logic in useAppStore to validate the restored selectedCategory id
against the current categories list and fall back to 'all' when the id is
missing or invalid. Specifically, in the code path that handles store
rehydration (the code that reads/restores selectedCategory), check whether the
restored id exists in the categories array (or map) used by the app and if not
assign 'all' before setting the state; reference the selectedCategory state/key
and the rehydrate/restore handler in useAppStore so RepositoryList and other
consumers always receive a valid category id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3912f464-4aab-46f0-b481-d4f020a3c83b
📒 Files selected for processing (7)
index.htmlnginx.confsrc/App.tsxsrc/components/LoginScreen.tsxsrc/components/RepositoryList.tsxsrc/store/useAppStore.tssrc/types/index.ts
Comment on lines
+71
to
+74
| <img | ||
| src="./icon.png" | ||
| alt="GitHub Stars Manager" | ||
| className="w-full h-full object-cover" |
There was a problem hiding this comment.
Use a base-aware icon path to avoid broken logo URLs.
Line 72 uses a relative path ("./icon.png"). On non-root paths this can resolve incorrectly and 404. Prefer a base-aware absolute asset path.
🔧 Proposed fix
- <img
- src="./icon.png"
+ <img
+ src={`${import.meta.env.BASE_URL}icon.png`}
alt="GitHub Stars Manager"
className="w-full h-full object-cover"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <img | |
| src="./icon.png" | |
| alt="GitHub Stars Manager" | |
| className="w-full h-full object-cover" | |
| <img | |
| src={`${import.meta.env.BASE_URL}icon.png`} | |
| alt="GitHub Stars Manager" | |
| className="w-full h-full object-cover" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/LoginScreen.tsx` around lines 71 - 74, The img in LoginScreen
uses a relative src="./icon.png" which breaks on non-root routes; update the img
src to be base-aware by either importing the asset into the component (e.g.,
import icon from '.../icon.png' and assign that to the src) or by using the
public base URL (e.g., process.env.PUBLIC_URL + '/icon.png') so the logo
resolves correctly regardless of route; modify the src attribute in the img
element inside LoginScreen accordingly.
Comment on lines
+37
to
+39
| const previousCategoryRef = useRef(selectedCategory); | ||
| const savedScrollYRef = useRef<number | null>(null); | ||
| const restoreScrollFrameRef = useRef<number | null>(null); |
There was a problem hiding this comment.
Guard scroll restoration when category changes during sync.
Current logic always restores saved scroll on sync end. If the user switches category while syncing (Line 124 scrolls to top), the later restore can jump back to the old position and break the new-category flow.
🔧 Proposed fix
const previousCategoryRef = useRef(selectedCategory);
const savedScrollYRef = useRef<number | null>(null);
const restoreScrollFrameRef = useRef<number | null>(null);
+ const syncStartCategoryRef = useRef<string | null>(null);
@@
if (isSyncing) {
+ syncStartCategoryRef.current = previousCategoryRef.current;
savedScrollYRef.current = window.scrollY;
if (restoreScrollFrameRef.current !== null) {
cancelAnimationFrame(restoreScrollFrameRef.current);
restoreScrollFrameRef.current = null;
}
return;
}
const targetScrollY = savedScrollYRef.current;
if (targetScrollY === null) return;
+ if (
+ syncStartCategoryRef.current !== null &&
+ syncStartCategoryRef.current !== previousCategoryRef.current
+ ) {
+ savedScrollYRef.current = null;
+ syncStartCategoryRef.current = null;
+ return;
+ }
restoreScrollFrameRef.current = window.requestAnimationFrame(() => {
restoreScrollFrameRef.current = window.requestAnimationFrame(() => {
window.scrollTo({ top: targetScrollY, behavior: 'auto' });
restoreScrollFrameRef.current = null;
savedScrollYRef.current = null;
+ syncStartCategoryRef.current = null;
});
});Also applies to: 163-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/RepositoryList.tsx` around lines 37 - 39, Ensure scroll
restoration only occurs if the category hasn't changed: when saving scroll state
(use savedScrollYRef/previousCategoryRef), record previousCategoryRef.current =
selectedCategory; when scheduling the restore via restoreScrollFrameRef, check
that previousCategoryRef.current === selectedCategory before applying
window.scrollTo and only then clear savedScrollYRef/restoreScrollFrameRef;
additionally, on category change update previousCategoryRef and cancel any
pending restore by calling cancelAnimationFrame(restoreScrollFrameRef.current)
and clearing savedScrollYRef/restoreScrollFrameRef so a late restore cannot jump
the user back to the old category.
Comment on lines
+116
to
+117
| | 'currentView' | ||
| | 'selectedCategory' |
There was a problem hiding this comment.
Validate persisted selectedCategory during rehydration.
Persisting category id is great, but stale/removed custom category ids can be restored and lead to empty repository results (see src/components/RepositoryList.tsx Line 52). Add a fallback to 'all' when persisted id is no longer valid.
🔧 Proposed fix
const normalizePersistedState = (
persisted: PersistedAppState | undefined,
currentState: AppState & AppActions
): Partial<AppState & AppActions> => {
const safePersisted = persisted ?? {};
+ const persistedCustomCategories = Array.isArray(safePersisted.customCategories)
+ ? safePersisted.customCategories
+ : [];
+ const validCategoryIds = new Set([
+ ...defaultCategories.map((cat) => cat.id),
+ ...persistedCustomCategories.map((cat) => cat.id),
+ ]);
+ const normalizedSelectedCategory =
+ typeof safePersisted.selectedCategory === 'string' &&
+ validCategoryIds.has(safePersisted.selectedCategory)
+ ? safePersisted.selectedCategory
+ : 'all';
+
const repositories = Array.isArray(safePersisted.repositories) ? safePersisted.repositories : [];
const releases = Array.isArray(safePersisted.releases) ? safePersisted.releases : [];
@@
- customCategories: Array.isArray(safePersisted.customCategories) ? safePersisted.customCategories : [],
+ customCategories: persistedCustomCategories,
+ selectedCategory: normalizedSelectedCategory,Also applies to: 484-485
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/store/useAppStore.ts` around lines 116 - 117, Persisted selectedCategory
can be stale; update the rehydration logic in useAppStore to validate the
restored selectedCategory id against the current categories list and fall back
to 'all' when the id is missing or invalid. Specifically, in the code path that
handles store rehydration (the code that reads/restores selectedCategory), check
whether the restored id exists in the categories array (or map) used by the app
and if not assign 'all' before setting the state; reference the selectedCategory
state/key and the rehydrate/restore handler in useAppStore so RepositoryList and
other consumers always receive a valid category id.
Owner
|
Added a small guard to avoid redundant scroll restoration after sync. Problem: we always restore scroll even when scrollY never changed, which can cause subtle jitter. Fix: only restore when window.scrollY differs from the saved value, otherwise clear the saved scroll value. |
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
selectedCategory,currentView) so category/view state survives refresh and reload.Changes
src/components/RepositoryList.tsxsrc/App.tsxsrc/store/useAppStore.tssetSelectedCategorycurrentViewandselectedCategorysrc/types/index.tsselectedCategorytoAppStateindex.html,src/components/LoginScreen.tsxnginx.conf/api/proxyingWhy
These changes fix UX regressions where sync could disrupt reading flow (scroll jumps, visible-card reset, lost category context), and make backend proxy routing more robust after container restarts/recreates.
Summary by CodeRabbit
New Features
Improvements
Style