mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-10 22:21:22 -07:00
feat(ui): refine focus highlight for selection lists and settings items (fixes #21493)
This commit is contained in:
@@ -12,10 +12,22 @@ import {
|
||||
type RenderItemContext,
|
||||
} from './BaseSelectionList.js';
|
||||
import { useSelectionList } from '../../hooks/useSelectionList.js';
|
||||
import { Text } from 'ink';
|
||||
import { Text, getBoundingBox } from 'ink';
|
||||
import type { theme } from '../../semantic-colors.js';
|
||||
import { useTerminalSize } from '../../hooks/useTerminalSize.js';
|
||||
import { useUIState, type UIState } from '../../contexts/UIStateContext.js';
|
||||
|
||||
vi.mock('../../hooks/useSelectionList.js');
|
||||
vi.mock('../../hooks/useTerminalSize.js');
|
||||
vi.mock('../../contexts/UIStateContext.js');
|
||||
|
||||
vi.mock('ink', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('ink')>();
|
||||
return {
|
||||
...actual,
|
||||
getBoundingBox: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
const mockTheme = {
|
||||
text: { primary: 'COLOR_PRIMARY', secondary: 'COLOR_SECONDARY' },
|
||||
@@ -57,6 +69,18 @@ describe('BaseSelectionList', () => {
|
||||
setActiveIndex: vi.fn(),
|
||||
});
|
||||
|
||||
vi.mocked(useTerminalSize).mockReturnValue({
|
||||
columns: 100,
|
||||
rows: 40,
|
||||
});
|
||||
|
||||
vi.mocked(getBoundingBox).mockReturnValue({
|
||||
width: 100,
|
||||
height: 10,
|
||||
x: 0,
|
||||
y: 0,
|
||||
});
|
||||
|
||||
mockRenderItem.mockImplementation(
|
||||
(
|
||||
item: { value: string; label: string; disabled?: boolean; key: string },
|
||||
@@ -82,6 +106,9 @@ describe('BaseSelectionList', () => {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(useUIState).mockReturnValue({
|
||||
mainAreaWidth: 100,
|
||||
} as unknown as UIState);
|
||||
});
|
||||
|
||||
describe('Rendering and Structure', () => {
|
||||
@@ -92,7 +119,8 @@ describe('BaseSelectionList', () => {
|
||||
expect(lastFrame()).toContain('Item B');
|
||||
expect(lastFrame()).toContain('Item C');
|
||||
|
||||
expect(mockRenderItem).toHaveBeenCalledTimes(3);
|
||||
// 3 items. Render count might be higher due to useLayoutEffect for offset.
|
||||
expect(mockRenderItem).toHaveBeenCalledTimes(items.length);
|
||||
expect(mockRenderItem).toHaveBeenCalledWith(items[0], expect.any(Object));
|
||||
unmount();
|
||||
});
|
||||
@@ -496,6 +524,37 @@ describe('BaseSelectionList', () => {
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('should apply full-width highlight and offset when horizontally shifted', async () => {
|
||||
const horizontalOffset = 10;
|
||||
const terminalWidth = 100;
|
||||
|
||||
vi.mocked(getBoundingBox).mockReturnValue({
|
||||
width: 80,
|
||||
height: 10,
|
||||
x: horizontalOffset,
|
||||
y: 0,
|
||||
});
|
||||
|
||||
vi.mocked(useTerminalSize).mockReturnValue({
|
||||
columns: terminalWidth,
|
||||
rows: 40,
|
||||
});
|
||||
|
||||
const { lastFrame, unmount, waitUntilReady } = await renderComponent(
|
||||
{},
|
||||
0,
|
||||
); // Item A selected
|
||||
await waitUntilReady();
|
||||
|
||||
const output = lastFrame();
|
||||
expect(output).toContain('Item A');
|
||||
|
||||
// Since we can't easily inspect Box props from lastFrame(),
|
||||
// this test confirms it doesn't crash and renders correctly.
|
||||
// In a real scenario, we'd use a custom renderer or inspect the tree if possible.
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('should show arrows and correct items when scrolled to the middle', async () => {
|
||||
const { lastFrame, unmount } = await renderComponent(
|
||||
{ items: longList, maxItemsToShow: MAX_ITEMS, showScrollArrows: true },
|
||||
|
||||
@@ -5,10 +5,12 @@
|
||||
*/
|
||||
|
||||
import type React from 'react';
|
||||
import { useEffect, useState } from 'react';
|
||||
import { Text, Box } from 'ink';
|
||||
import { useEffect, useState, useLayoutEffect, useRef } from 'react';
|
||||
import { Text, Box, getBoundingBox, type DOMElement } from 'ink';
|
||||
import { theme } from '../../semantic-colors.js';
|
||||
import { useSelectionList } from '../../hooks/useSelectionList.js';
|
||||
import { useTerminalSize } from '../../hooks/useTerminalSize.js';
|
||||
import { useUIState } from '../../contexts/UIStateContext.js';
|
||||
|
||||
import type { SelectionListItem } from '../../hooks/useSelectionList.js';
|
||||
|
||||
@@ -80,6 +82,25 @@ export function BaseSelectionList<
|
||||
});
|
||||
|
||||
const [scrollOffset, setScrollOffset] = useState(0);
|
||||
const containerRef = useRef<DOMElement>(null);
|
||||
const [horizontalOffset, setHorizontalOffset] = useState(0);
|
||||
const { columns: terminalWidth } = useTerminalSize();
|
||||
const uiState = useUIState();
|
||||
const mainAreaWidth = uiState?.mainAreaWidth;
|
||||
const effectiveTerminalWidth = mainAreaWidth ?? terminalWidth;
|
||||
|
||||
// Measure horizontal offset to allow full-width highlight
|
||||
useLayoutEffect(() => {
|
||||
if (containerRef.current) {
|
||||
const { x } = getBoundingBox(containerRef.current);
|
||||
// We want to track the "true" offset relative to the viewport.
|
||||
// Since we apply -breakoutAmount as a margin to the SELECTED item,
|
||||
// it should not affect the parent container's x coordinate in a standard layout.
|
||||
if (x !== horizontalOffset) {
|
||||
setHorizontalOffset(x);
|
||||
}
|
||||
}
|
||||
}, [terminalWidth, mainAreaWidth, horizontalOffset]);
|
||||
|
||||
// Handle scrolling for long lists
|
||||
useEffect(() => {
|
||||
@@ -98,7 +119,7 @@ export function BaseSelectionList<
|
||||
const numberColumnWidth = String(items.length).length;
|
||||
|
||||
return (
|
||||
<Box flexDirection="column">
|
||||
<Box flexDirection="column" ref={containerRef}>
|
||||
{/* Use conditional coloring instead of conditional rendering */}
|
||||
{showScrollArrows && items.length > maxItemsToShow && (
|
||||
<Text
|
||||
@@ -136,11 +157,20 @@ export function BaseSelectionList<
|
||||
numberColumnWidth,
|
||||
)}.`;
|
||||
|
||||
const breakoutAmount = isSelected
|
||||
? Math.max(0, horizontalOffset - 2)
|
||||
: 0;
|
||||
|
||||
return (
|
||||
<Box
|
||||
key={item.key}
|
||||
alignItems="flex-start"
|
||||
backgroundColor={isSelected ? theme.background.focus : undefined}
|
||||
marginLeft={-breakoutAmount}
|
||||
paddingLeft={breakoutAmount}
|
||||
width={
|
||||
isSelected ? Math.max(1, effectiveTerminalWidth - 4) : '100%'
|
||||
}
|
||||
>
|
||||
{/* Radio button indicator */}
|
||||
<Box minWidth={2} flexShrink={0}>
|
||||
|
||||
@@ -524,12 +524,13 @@ export function BaseSettingsDialog({
|
||||
return (
|
||||
<React.Fragment key={item.key}>
|
||||
<Box
|
||||
marginX={1}
|
||||
marginX={isActive ? 0 : 1}
|
||||
flexDirection="row"
|
||||
alignItems="flex-start"
|
||||
backgroundColor={
|
||||
isActive ? theme.background.focus : undefined
|
||||
}
|
||||
paddingX={isActive ? 1 : 0}
|
||||
>
|
||||
<Box minWidth={2} flexShrink={0}>
|
||||
<Text
|
||||
@@ -560,7 +561,12 @@ export function BaseSettingsDialog({
|
||||
</Text>
|
||||
)}
|
||||
</Text>
|
||||
<Text color={theme.text.secondary} wrap="truncate">
|
||||
<Text
|
||||
color={
|
||||
isActive ? theme.text.primary : theme.text.secondary
|
||||
}
|
||||
wrap="truncate"
|
||||
>
|
||||
{item.description ?? ''}
|
||||
</Text>
|
||||
</Box>
|
||||
|
||||
Reference in New Issue
Block a user