fix(cli): resolve flicker at boundaries of list in BaseSelectionList (#23298)

This commit is contained in:
Jack Wotherspoon
2026-03-22 23:10:47 -04:00
committed by GitHub
parent c7d44e339b
commit c67817f1a9
2 changed files with 47 additions and 14 deletions

View File

@@ -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 },

View File

@@ -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 && (
<Text
color={scrollOffset > 0 ? theme.text.primary : theme.text.secondary}
color={
effectiveScrollOffset > 0
? theme.text.primary
: theme.text.secondary
}
>
</Text>
)}
{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 && (
<Text
color={
scrollOffset + maxItemsToShow < items.length
effectiveScrollOffset + maxItemsToShow < items.length
? theme.text.primary
: theme.text.secondary
}