fix: add viewport scrolling to category selector in CLI#1076
fix: add viewport scrolling to category selector in CLI#1076SpaceKittie wants to merge 2 commits intof:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds viewport-aware category rendering to PromptList: computes reserved lines and list height, calculates category scroll offset and visible slice, conditionally shows "All Categories" header, and adjusts selection indices to align highlighting with the visible viewport. (49 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 approve the review once all CodeRabbit's comments are resolved.Enable the |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/prompts.chat/src/cli/components/PromptList.tsx (1)
308-315: Consider extracting viewport slice logic for clarity.The slice bounds calculation is complex with multiple conditional paths. While the logic appears correct, extracting these calculations into named variables would improve readability and make future maintenance easier.
♻️ Suggested refactor for readability
+ // Calculate slice bounds based on search state and scroll position + const sliceStart = categorySearchQuery + ? categoryScrollOffset + : Math.max(0, categoryScrollOffset - 1); + const sliceEnd = categorySearchQuery + ? categoryScrollOffset + categoryListHeight + : (categoryScrollOffset === 0 ? categoryListHeight - 1 : categoryScrollOffset - 1 + categoryListHeight); + {filteredCategories.slice( - categorySearchQuery ? categoryScrollOffset : Math.max(0, categoryScrollOffset - 1), - categorySearchQuery ? categoryScrollOffset + categoryListHeight : (categoryScrollOffset === 0 ? categoryListHeight - 1 : categoryScrollOffset - 1 + categoryListHeight) + sliceStart, + sliceEnd ).map((cat, index) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/prompts.chat/src/cli/components/PromptList.tsx` around lines 308 - 315, The slice start/end calculation for filteredCategories in PromptList.tsx is hard to read; extract the conditional expressions into clearly named variables (e.g., sliceStart, sliceEnd) computed using categorySearchQuery, categoryScrollOffset, and categoryListHeight, then call filteredCategories.slice(sliceStart, sliceEnd). Also compute actualIndex and adjustedIndex using those same variables (keeping the existing logic that adds 1 when categoryScrollOffset === 0 and no categorySearchQuery) so the rendering map remains unchanged but the bounds logic is isolated and readable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/prompts.chat/src/cli/components/PromptList.tsx`:
- Around line 308-315: The slice start/end calculation for filteredCategories in
PromptList.tsx is hard to read; extract the conditional expressions into clearly
named variables (e.g., sliceStart, sliceEnd) computed using categorySearchQuery,
categoryScrollOffset, and categoryListHeight, then call
filteredCategories.slice(sliceStart, sliceEnd). Also compute actualIndex and
adjustedIndex using those same variables (keeping the existing logic that adds 1
when categoryScrollOffset === 0 and no categorySearchQuery) so the rendering map
remains unchanged but the bounds logic is isolated and readable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0cf66a07-30a4-4b7f-a129-1659ff94c0f6
📒 Files selected for processing (1)
packages/prompts.chat/src/cli/components/PromptList.tsx
|
Addressed CodeRabbit feedback: added JSDoc for PromptList and refactored category slice/index calculations into named variables without changing behavior. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/prompts.chat/src/cli/components/PromptList.tsx (1)
228-235: Consider extracting viewport calculation logic for readability.The viewport scrolling calculations are correct but have multiple conditional branches that make the logic harder to follow. The different handling for search mode vs. non-search mode, and scrolled vs. non-scrolled states, creates complexity.
♻️ Suggested refactor to clarify intent
// Category viewport scrolling - calculate which items to show const isCategorySearchActive = Boolean(categorySearchQuery); const categoryTotalItems = categorySearchQuery ? filteredCategories.length : filteredCategories.length + 1; - const categoryScrollOffset = Math.max(0, Math.min(categoryIndex - categoryListHeight + 2, Math.max(0, categoryTotalItems - categoryListHeight))); - const sliceStart = isCategorySearchActive ? categoryScrollOffset : Math.max(0, categoryScrollOffset - 1); - const sliceEnd = isCategorySearchActive - ? categoryScrollOffset + categoryListHeight - : (categoryScrollOffset === 0 ? categoryListHeight - 1 : categoryScrollOffset - 1 + categoryListHeight); + + // Scroll offset keeps selected item visible with 1-line buffer at bottom + const maxScrollOffset = Math.max(0, categoryTotalItems - categoryListHeight); + const categoryScrollOffset = Math.max(0, Math.min(categoryIndex - categoryListHeight + 2, maxScrollOffset)); + + // When not searching, "All Categories" occupies virtual index 0, so array indices are offset by 1 + const showAllCategoriesHeader = !isCategorySearchActive && categoryScrollOffset === 0; + const sliceStart = isCategorySearchActive ? categoryScrollOffset : Math.max(0, categoryScrollOffset - 1); + const visibleCategorySlots = showAllCategoriesHeader ? categoryListHeight - 1 : categoryListHeight; + const sliceEnd = sliceStart + visibleCategorySlots;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/prompts.chat/src/cli/components/PromptList.tsx` around lines 228 - 235, Extract the viewport calculation block (the logic computing isCategorySearchActive, categoryTotalItems, categoryScrollOffset, sliceStart, sliceEnd) into a named helper function (e.g., computeCategoryViewport) that accepts the inputs categoryIndex, categoryListHeight, categorySearchQuery, and filteredCategoriesLength and returns { sliceStart, sliceEnd, categoryScrollOffset }; replace the inline calculations in PromptList.tsx with a single call to that helper to preserve current behavior while improving readability and centralizing the conditional logic for search vs non-search modes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/prompts.chat/src/cli/components/PromptList.tsx`:
- Around line 228-235: Extract the viewport calculation block (the logic
computing isCategorySearchActive, categoryTotalItems, categoryScrollOffset,
sliceStart, sliceEnd) into a named helper function (e.g.,
computeCategoryViewport) that accepts the inputs categoryIndex,
categoryListHeight, categorySearchQuery, and filteredCategoriesLength and
returns { sliceStart, sliceEnd, categoryScrollOffset }; replace the inline
calculations in PromptList.tsx with a single call to that helper to preserve
current behavior while improving readability and centralizing the conditional
logic for search vs non-search modes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c96a5dff-559c-4c89-b057-928cce48e9f5
📒 Files selected for processing (1)
packages/prompts.chat/src/cli/components/PromptList.tsx
Description
Fixes an issue in the CLI where the category selector list would overflow the viewport if there were more categories than visible lines in the terminal.
Type of Change
[x] Bug fix
Summary by CodeRabbit
New Features
Documentation