mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-19 16:28:28 +00:00
fix(cli): preserve esc-to-cancel on narrow terminals + boundary tests
Address review feedback on the narrow-terminal rendering changes: - Composer: when the full LoadingIndicator is suppressed on ≤30-col terminals during Responding, render a minimal "(esc to cancel)" text fallback so users retain the cancel affordance. Suppressing the full indicator still avoids layout breakage, but the affordance now stays visible. - TableRenderer: clarify that `borderOverhead` is reused by the horizontal-vs-vertical layout threshold so a future change to the border-width formula does not silently shift the threshold. - TableRenderer tests: add equality boundary cases at `ABSOLUTE_MIN_HORIZONTAL_TABLE_WIDTH` (24) and at the 5-column column-budget threshold (35), plus one-below cases, so a future `<` → `<=` regression on the strict comparator is caught. - Composer tests: assert the fallback string is rendered when (and only when) the full indicator is suppressed. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
parent
0779de37a4
commit
169031d9cd
4 changed files with 86 additions and 2 deletions
|
|
@ -229,6 +229,36 @@ describe('Composer', () => {
|
||||||
expect(lastFrame()).not.toContain('LoadingIndicator');
|
expect(lastFrame()).not.toContain('LoadingIndicator');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('preserves "esc to cancel" fallback when LoadingIndicator is suppressed', () => {
|
||||||
|
// Even when the full LoadingIndicator is hidden on ultra-narrow
|
||||||
|
// terminals, the cancel affordance must remain so users can abort.
|
||||||
|
const uiState = createMockUIState({
|
||||||
|
streamingState: StreamingState.Responding,
|
||||||
|
terminalWidth: 25,
|
||||||
|
});
|
||||||
|
|
||||||
|
const { lastFrame } = renderComposer(uiState);
|
||||||
|
|
||||||
|
const output = lastFrame();
|
||||||
|
expect(output).not.toContain('LoadingIndicator');
|
||||||
|
expect(output).toContain('esc to cancel');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not render the esc fallback once the full indicator is visible', () => {
|
||||||
|
const uiState = createMockUIState({
|
||||||
|
streamingState: StreamingState.Responding,
|
||||||
|
terminalWidth: 31,
|
||||||
|
});
|
||||||
|
|
||||||
|
const { lastFrame } = renderComposer(uiState);
|
||||||
|
|
||||||
|
const output = lastFrame();
|
||||||
|
expect(output).toContain('LoadingIndicator');
|
||||||
|
// The minimal fallback string only appears when the full indicator is
|
||||||
|
// suppressed — when LoadingIndicator renders, it owns the cancel hint.
|
||||||
|
expect(output).not.toContain('esc to cancel');
|
||||||
|
});
|
||||||
|
|
||||||
it('shows LoadingIndicator when Responding on a 31-col terminal', () => {
|
it('shows LoadingIndicator when Responding on a 31-col terminal', () => {
|
||||||
const uiState = createMockUIState({
|
const uiState = createMockUIState({
|
||||||
streamingState: StreamingState.Responding,
|
streamingState: StreamingState.Responding,
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,7 @@
|
||||||
* SPDX-License-Identifier: Apache-2.0
|
* SPDX-License-Identifier: Apache-2.0
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { Box, useIsScreenReaderEnabled } from 'ink';
|
import { Box, Text, useIsScreenReaderEnabled } from 'ink';
|
||||||
import { useCallback, useState } from 'react';
|
import { useCallback, useState } from 'react';
|
||||||
import { LoadingIndicator } from './LoadingIndicator.js';
|
import { LoadingIndicator } from './LoadingIndicator.js';
|
||||||
import { InputPrompt } from './InputPrompt.js';
|
import { InputPrompt } from './InputPrompt.js';
|
||||||
|
|
@ -15,6 +15,7 @@ import { useUIState } from '../contexts/UIStateContext.js';
|
||||||
import { useUIActions } from '../contexts/UIActionsContext.js';
|
import { useUIActions } from '../contexts/UIActionsContext.js';
|
||||||
import { useVimMode } from '../contexts/VimModeContext.js';
|
import { useVimMode } from '../contexts/VimModeContext.js';
|
||||||
import { useConfig } from '../contexts/ConfigContext.js';
|
import { useConfig } from '../contexts/ConfigContext.js';
|
||||||
|
import { theme } from '../semantic-colors.js';
|
||||||
import { StreamingState, type HistoryItemToolGroup } from '../types.js';
|
import { StreamingState, type HistoryItemToolGroup } from '../types.js';
|
||||||
import { FeedbackDialog } from '../FeedbackDialog.js';
|
import { FeedbackDialog } from '../FeedbackDialog.js';
|
||||||
import { t } from '../../i18n/index.js';
|
import { t } from '../../i18n/index.js';
|
||||||
|
|
@ -109,6 +110,18 @@ export const Composer = () => {
|
||||||
isReceivingContent={isReceivingContent}
|
isReceivingContent={isReceivingContent}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
|
{/*
|
||||||
|
* Narrow-terminal fallback: when the full LoadingIndicator is suppressed
|
||||||
|
* (≤30 cols, actively Responding) we still surface a minimal `esc to
|
||||||
|
* cancel` hint so users on ultra-narrow terminals retain the cancel
|
||||||
|
* affordance during long-running calls. The full timer/spinner/phrase
|
||||||
|
* UI is still suppressed to avoid layout breakage.
|
||||||
|
*/}
|
||||||
|
{!uiState.embeddedShellFocused && suppressBottomLoadingIndicator && (
|
||||||
|
<Box paddingLeft={2}>
|
||||||
|
<Text color={theme.text.secondary}>{t('(esc to cancel)')}</Text>
|
||||||
|
</Box>
|
||||||
|
)}
|
||||||
|
|
||||||
<QueuedMessageDisplay messageQueue={uiState.messageQueue} />
|
<QueuedMessageDisplay messageQueue={uiState.messageQueue} />
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -496,6 +496,44 @@ describe('<TableRenderer />', () => {
|
||||||
expect(output).toContain('┌');
|
expect(output).toContain('┌');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Boundary equality tests: the comparator is strict `<`, so the threshold
|
||||||
|
// value itself must still render horizontally. Without these, a future
|
||||||
|
// off-by-one change from `<` to `<=` would slip through the < / > pair.
|
||||||
|
it('renders horizontal at exact absolute floor (2 cols, contentWidth=24)', () => {
|
||||||
|
// ABSOLUTE_MIN_HORIZONTAL_TABLE_WIDTH is 24. With strict `<`, equality
|
||||||
|
// means horizontal mode is selected.
|
||||||
|
const output = renderTable(['A', 'B'], [['x', 'y']], 24);
|
||||||
|
expect(output).toContain('┌');
|
||||||
|
expect(output).toContain('└');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to vertical one below absolute floor (2 cols, contentWidth=23)', () => {
|
||||||
|
const output = renderTable(['A', 'B'], [['x', 'y']], 23);
|
||||||
|
expect(output).not.toContain('┌');
|
||||||
|
expect(output).toContain('A:');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('renders horizontal at exact column-budget threshold (5 cols, contentWidth=35)', () => {
|
||||||
|
// 5 cols → minHorizontal = 5*3 + (1+5*3) + 4 = 35. Equality must still
|
||||||
|
// render horizontally under the strict `<` comparator.
|
||||||
|
const output = renderTable(
|
||||||
|
['A', 'B', 'C', 'D', 'E'],
|
||||||
|
[['1', '2', '3', '4', '5']],
|
||||||
|
35,
|
||||||
|
);
|
||||||
|
expect(output).toContain('┌');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to vertical one below column-budget threshold (5 cols, contentWidth=34)', () => {
|
||||||
|
const output = renderTable(
|
||||||
|
['A', 'B', 'C', 'D', 'E'],
|
||||||
|
[['1', '2', '3', '4', '5']],
|
||||||
|
34,
|
||||||
|
);
|
||||||
|
expect(output).not.toContain('┌');
|
||||||
|
expect(output).toContain('A:');
|
||||||
|
});
|
||||||
|
|
||||||
it('forces vertical for many-column tables on narrow terminals', () => {
|
it('forces vertical for many-column tables on narrow terminals', () => {
|
||||||
// 5 cols → minHorizontal = 5*3 + (1+5*3) + 4 = 35; 30 cols is below that.
|
// 5 cols → minHorizontal = 5*3 + (1+5*3) + 4 = 35; 30 cols is below that.
|
||||||
const output = renderTable(
|
const output = renderTable(
|
||||||
|
|
|
||||||
|
|
@ -331,7 +331,10 @@ export const TableRenderer: React.FC<TableRendererProps> = ({
|
||||||
});
|
});
|
||||||
|
|
||||||
// ── Step 2: Calculate available space ──
|
// ── Step 2: Calculate available space ──
|
||||||
// Border overhead: │ content │ content │ = 1 + (width + 3) per column
|
// Border overhead: │ content │ content │ = 1 + (width + 3) per column.
|
||||||
|
// NOTE: this value is reused below in the horizontal-vs-vertical threshold
|
||||||
|
// (`minHorizontalTableWidth`). Any change to this formula will silently
|
||||||
|
// shift the layout threshold — adjust both call sites together.
|
||||||
const borderOverhead = 1 + colCount * 3;
|
const borderOverhead = 1 + colCount * 3;
|
||||||
const availableWidth = Math.max(
|
const availableWidth = Math.max(
|
||||||
contentWidth - borderOverhead - SAFETY_MARGIN,
|
contentWidth - borderOverhead - SAFETY_MARGIN,
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue