feat(repositories): switch to infinite scroll with consistent card widths #25
Conversation
…dths - Replace pagination with on-demand infinite scrolling (load 50 per batch) - Use IntersectionObserver and a bottom sentinel to trigger loading - Update stats to show current range “X–Y / N repositories” - Switch from CSS columns to fixed grid to ensure consistent card widths - Remove pagination state and controls - Clean up unused variables and resolve lint warnings UX: smoother scrolling, stable card widths, more natural loading behavior.
WalkthroughIntroduces infinite scroll to RepositoryList.tsx using IntersectionObserver and a visibleCount slice. Resets visibleCount on data/filter changes, computes visible ranges, and renders only the visible subset. Updates text to show range. Removes an unused state and adapts analyzeRepository calls to drop the index parameter. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant RL as RepositoryList (Component)
participant IO as IntersectionObserver
participant SRV as analyzeRepository (service)
Note over RL: Initial render with filters applied<br/>visibleCount = LOAD_BATCH
U->>RL: Scrolls repository list
RL->>IO: Observe sentinel (rootMargin: 200px)
IO-->>RL: Sentinel enters viewport
RL->>RL: Increment visibleCount (bounded by filtered length)
RL-->>U: Render visibleRepositories[startIndex..endIndex]
rect rgba(200, 240, 255, 0.3)
Note right of RL: On filters/data change<br/>reset visibleCount to LOAD_BATCH
end
alt When analysis is triggered for visible items
RL->>SRV: analyzeRepository(repo) for each visible repo (Promise.all batches)
SRV-->>RL: Analysis results
RL-->>U: Update cards with analysis
else No more items
IO-->>RL: No further intersections
RL-->>U: Sentinel hidden/no-op
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/RepositoryList.tsx (1)
175-185: Pause/resume won’t respond due to stale state in async loop; use a ref.
isPausedinsidewhile (isPaused && ...)is captured at function creation, so changes viasetIsPausedaren’t seen by running tasks.Apply this diff:
- const isAnalyzingRef = useRef(false); + const isAnalyzingRef = useRef(false); + const isPausedRef = useRef(false); + + useEffect(() => { + isPausedRef.current = isPaused; + }, [isPaused]);And update the loop:
- while (isPaused && !shouldStopRef.current) { + while (isPausedRef.current && !shouldStopRef.current) { await new Promise(resolve => setTimeout(resolve, 1000)); }
🧹 Nitpick comments (4)
src/components/RepositoryList.tsx (4)
87-92: Tighten reset effect dependencies; avoid disabling exhaustive-deps.
Resetting on bothrepositoriesandfilteredRepositories.lengthcan cause redundant resets and the ESLint disable masks real deps. Prefer derivingfilteredRepositorieswithuseMemo([...deps])and then:
- Reset on
[selectedCategory, repositories](or[selectedCategory, repositories.length]if identity is unstable).Apply this diff to simplify the effect:
- useEffect(() => { - setVisibleCount(LOAD_BATCH); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [selectedCategory, repositories, filteredRepositories.length]); + useEffect(() => { + setVisibleCount(LOAD_BATCH); + }, [selectedCategory, repositories]);
93-114: Observer cleanup can leak when the sentinel disappears; include visibleCount and unobserve.
WhenvisibleCountreaches the total, the sentinel unmounts but this effect won’t rerun, leaving the observer alive until unmount. AddvisibleCountto deps and unobserve the specific node.Apply this diff:
- useEffect(() => { + useEffect(() => { const node = sentinelRef.current; if (!node) return; const observer = new IntersectionObserver( (entries) => { const entry = entries[0]; if (entry.isIntersecting) { setVisibleCount((count) => { if (count >= filteredRepositories.length) return count; return Math.min(count + LOAD_BATCH, filteredRepositories.length); }); } }, { root: null, rootMargin: '200px', threshold: 0 } ); observer.observe(node); - return () => observer.disconnect(); - }, [filteredRepositories.length]); + return () => { + try { observer.unobserve(node); } catch {} + observer.disconnect(); + }; + }, [filteredRepositories.length, visibleCount]);
514-517: Minor a11y: hide the sentinel from assistive tech.
Mark the sentinel as presentational to avoid confusing announcements.Apply this diff:
- <div ref={sentinelRef} className="h-8" /> + <div ref={sentinelRef} className="h-8" aria-hidden="true" role="presentation" />
502-512: Consider virtualization for large lists.
Infinite scroll still renders O(visibleCount) cards; with thousands of repos this can jank. Consider react-window/react-virtual for windowed rendering; sentinel logic can coexist for batch fetching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/RepositoryList.tsx(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/RepositoryList.tsx (2)
src/components/RepositoryCard.tsx (1)
RepositoryCard(16-561)src/store/useAppStore.ts (1)
useAppStore(162-429)
🔇 Additional comments (4)
src/components/RepositoryList.tsx (4)
1-1: LGTM: added useEffect import as required for new effects.
78-86: Infinite scroll state/setup looks good.
- Batch size and visible slice are straightforward.
- 1-based startIndex for display is fine.
240-241: Signature change handled correctly.
Mapping with(repo) => analyzeRepository(repo)matches the updated signature.
471-474: Range text looks correct and localized.
Shows 1-based range with total; good.
| {/* Repository Grid with consistent card widths */} | ||
| <div className="grid grid-cols-1 lg:grid-cols-2 xl:grid-cols-3 gap-6"> | ||
| {filteredRepositories.map(repo => ( | ||
| {visibleRepositories.map(repo => ( | ||
| <RepositoryCard | ||
| key={repo.id} | ||
| key={repo.id} | ||
| repository={repo} | ||
| showAISummary={showAISummary} | ||
| searchQuery={useAppStore.getState().searchFilters.query} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
Fix stale store read and hook rule violation related to searchFilters.
- Passing
searchQuery={useAppStore.getState().searchFilters.query}won’t update on store changes. - Elsewhere (Line 298)
useAppStore()is called conditionally inside anifblock, violating the Rules of Hooks and risking runtime errors.
Apply these diffs:
Add searchFilters to the top-level selector so it subscribes and is safe to use everywhere:
const {
githubToken,
aiConfigs,
activeAIConfig,
isLoading,
setLoading,
updateRepository,
language,
customCategories,
analysisProgress,
- setAnalysisProgress
+ setAnalysisProgress,
+ searchFilters
} = useAppStore();Use the subscribed value when rendering cards:
- searchQuery={useAppStore.getState().searchFilters.query}
+ searchQuery={searchFilters.query}Remove the conditional hook usage in the empty-state branch and reuse the subscribed searchFilters:
- const { searchFilters } = useAppStore();📝 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.
| {/* Repository Grid with consistent card widths */} | |
| <div className="grid grid-cols-1 lg:grid-cols-2 xl:grid-cols-3 gap-6"> | |
| {filteredRepositories.map(repo => ( | |
| {visibleRepositories.map(repo => ( | |
| <RepositoryCard | |
| key={repo.id} | |
| key={repo.id} | |
| repository={repo} | |
| showAISummary={showAISummary} | |
| searchQuery={useAppStore.getState().searchFilters.query} | |
| /> | |
| ))} | |
| {/* Repository Grid with consistent card widths */} | |
| <div className="grid grid-cols-1 lg:grid-cols-2 xl:grid-cols-3 gap-6"> | |
| {visibleRepositories.map(repo => ( | |
| <RepositoryCard | |
| key={repo.id} | |
| repository={repo} | |
| showAISummary={showAISummary} | |
| searchQuery={searchFilters.query} | |
| /> | |
| ))} |
🤖 Prompt for AI Agents
In src/components/RepositoryList.tsx around lines 298 and 502-511, fix a stale
store read and a Rules of Hooks violation: add searchFilters to the top-level
useAppStore selector so the component subscribes to searchFilters (e.g., include
searchFilters in the selector return), replace direct useAppStore.getState()
reads when rendering RepositoryCard with the subscribed searchFilters value, and
remove the conditional call to useAppStore() inside the empty-state branch by
reusing the top-level subscribed searchFilters variable so hooks are only called
unconditionally at the component top.
2 participants
UX: smoother scrolling, stable card widths, more natural loading behavior.
Summary by CodeRabbit
New Features
Refactor
Chores