diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx index 0501667d1f..b873de80d9 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx @@ -447,6 +447,28 @@ describe('BaseSelectionList', () => { unmount(); }); + it('should correctly calculate scroll offset during the initial render phase', async () => { + // Verify that the component correctly calculates the scroll offset during the + // initial render pass when starting with a high activeIndex. + // List length 10, max items 3, activeIndex 9 (last item). + const { unmount } = await renderScrollableList(9); + + const renderedItemValues = mockRenderItem.mock.calls.map( + (call) => call[0].value, + ); + + // Item 1 (index 0) should not be rendered if the scroll offset is correctly + // synchronized with the activeIndex from the start. + expect(renderedItemValues).not.toContain('Item 1'); + + // The items at the end of the list should be rendered. + expect(renderedItemValues).toContain('Item 8'); + expect(renderedItemValues).toContain('Item 9'); + expect(renderedItemValues).toContain('Item 10'); + + unmount(); + }); + it('should handle maxItemsToShow larger than the list length', async () => { const { lastFrame, unmount } = await renderComponent( { items: longList, maxItemsToShow: 15 }, diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx index 1090d4010d..455069f03f 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx @@ -5,7 +5,7 @@ */ import type React from 'react'; -import { useEffect, useState } from 'react'; +import { useState } from 'react'; import { Text, Box } from 'ink'; import { theme } from '../../semantic-colors.js'; import { @@ -84,20 +84,27 @@ export function BaseSelectionList< const [scrollOffset, setScrollOffset] = useState(0); - // Handle scrolling for long lists - useEffect(() => { - const newScrollOffset = Math.max( + // Derive the effective scroll offset during render to avoid "no-selection" flicker. + // This ensures that the visibleItems calculation uses an offset that includes activeIndex. + let effectiveScrollOffset = scrollOffset; + if (activeIndex < effectiveScrollOffset) { + effectiveScrollOffset = activeIndex; + } else if (activeIndex >= effectiveScrollOffset + maxItemsToShow) { + effectiveScrollOffset = Math.max( 0, Math.min(activeIndex - maxItemsToShow + 1, items.length - maxItemsToShow), ); - if (activeIndex < scrollOffset) { - setScrollOffset(activeIndex); - } else if (activeIndex >= scrollOffset + maxItemsToShow) { - setScrollOffset(newScrollOffset); - } - }, [activeIndex, items.length, scrollOffset, maxItemsToShow]); + } - const visibleItems = items.slice(scrollOffset, scrollOffset + maxItemsToShow); + // Synchronize state if it changed during derivation + if (effectiveScrollOffset !== scrollOffset) { + setScrollOffset(effectiveScrollOffset); + } + + const visibleItems = items.slice( + effectiveScrollOffset, + effectiveScrollOffset + maxItemsToShow, + ); const numberColumnWidth = String(items.length).length; return ( @@ -105,14 +112,18 @@ export function BaseSelectionList< {/* Use conditional coloring instead of conditional rendering */} {showScrollArrows && items.length > maxItemsToShow && ( 0 ? theme.text.primary : theme.text.secondary} + color={ + effectiveScrollOffset > 0 + ? theme.text.primary + : theme.text.secondary + } > ▲ )} {visibleItems.map((item, index) => { - const itemIndex = scrollOffset + index; + const itemIndex = effectiveScrollOffset + index; const isSelected = activeIndex === itemIndex; // Determine colors based on selection and disabled state @@ -182,7 +193,7 @@ export function BaseSelectionList< {showScrollArrows && items.length > maxItemsToShow && (