-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(cli): make @ completion category tabs clickable #8395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e71bc6c
93e2a5b
c34dd67
bec2894
28fc002
a287ac2
35f0dfd
342a45a
c62d15e
2f8c0a3
57f4202
4bfe311
499b09f
8b62f58
4677ee0
a87f5fd
c2f3155
f12770d
e19de89
a81d1f2
0e06548
214e60f
10aa640
eb79645
a06c440
7d7cc37
66de09e
efae094
c16e993
749804f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # Completion Category Mouse Selection | ||
|
|
||
| ## Scope | ||
|
|
||
| Verify that the tabbed `@` completion picker can switch to an exact category | ||
| with a mouse click when terminal mouse tracking is enabled. Keyboard category | ||
| navigation and suggestion-row mouse behavior must remain unchanged. | ||
|
|
||
| ## Baseline | ||
|
|
||
| A released global `qwen` executable was not available in this environment, so | ||
| the before-change Warp dry-run could not be repeated locally. The issue's SGR | ||
| mouse reproduction reports that suggestion rows accept clicks while category | ||
| labels do not. Source inspection on `upstream/main` confirms that suggestion | ||
| rows have a mouse controller, while category labels have no refs or click | ||
| handler. | ||
|
|
||
| ## Manual Scenario | ||
|
|
||
| 1. Run Qwen Code in Warp with `ui.mouseTracking` enabled and the terminal | ||
| buffer enabled. | ||
| 2. Open a workspace where `@` completion shows at least three tabs, such as | ||
| **All / Files / Sessions**. | ||
| 3. Click **Files**, then **Sessions**, then **All**. | ||
| 4. Confirm that each click activates the exact tab under the pointer, resets | ||
| the highlighted suggestion to the first visible result, and filters the | ||
| suggestion rows for that category. | ||
| 5. Confirm that clicking outside every tab does not change the active tab. | ||
| 6. While the tab bar is visible, confirm that bare `Left` / `Right` cycles the | ||
| categories. Press `Esc` to dismiss the picker and confirm the arrows return | ||
| to normal input-caret behavior. | ||
| 7. Set `ui.mouseTracking` to `false`, restart Qwen Code, and confirm that tab | ||
| clicks are not intercepted by Qwen Code. | ||
|
|
||
| ## Regression Checks | ||
|
|
||
| - Only a left-button press activates a category; pointer movement and button | ||
| release do not. | ||
| - The category hit area follows the rendered tab bounds in both normal and | ||
| overflow layouts. | ||
| - Suggestion-row hover and click handling still use their existing controller. | ||
| - Export completion, command search, and reverse search do not receive a | ||
| category-selection callback. | ||
|
|
||
| ## Automated Verification | ||
|
|
||
| ```bash | ||
| cd packages/cli | ||
| npx vitest run \ | ||
| src/ui/components/CompletionCategoryMouseController.test.tsx \ | ||
| src/ui/components/SuggestionsDisplay.mouse.test.tsx \ | ||
| src/ui/components/InputPrompt.suggestionMouse.test.tsx \ | ||
| src/ui/hooks/useCompletion.test.ts | ||
|
|
||
| cd ../.. | ||
| npm run lint | ||
| npm run build | ||
| npm run typecheck | ||
| ``` | ||
|
|
||
| ## Results | ||
|
|
||
| - The focused mouse/category regression suite passes: 4 files, 41 tests. | ||
| - Repository lint, build, and typecheck pass. | ||
| - Manual Warp execution remains for reviewer verification because Warp and a | ||
| released global `qwen` executable were not available in this environment. |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,112 @@ | ||||||||||||
| /** | ||||||||||||
| * @license | ||||||||||||
| * Copyright 2025 Qwen | ||||||||||||
| * SPDX-License-Identifier: Apache-2.0 | ||||||||||||
| */ | ||||||||||||
|
|
||||||||||||
| import { type MutableRefObject } from 'react'; | ||||||||||||
| import { type DOMElement } from 'ink'; | ||||||||||||
| import { render } from '@testing-library/react'; | ||||||||||||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||||||||||||
| import { CompletionCategoryMouseController } from './CompletionCategoryMouseController.js'; | ||||||||||||
| import { useMouseEvents } from '../hooks/useMouseEvents.js'; | ||||||||||||
| import { useTerminalSize } from '../hooks/useTerminalSize.js'; | ||||||||||||
| import { type MouseEvent } from '../utils/mouse.js'; | ||||||||||||
| import { findElementAtMouseEvent } from '../utils/mouse-hit.js'; | ||||||||||||
|
|
||||||||||||
| vi.mock('../hooks/useMouseEvents.js', () => ({ useMouseEvents: vi.fn() })); | ||||||||||||
| vi.mock('../hooks/useTerminalSize.js', () => ({ useTerminalSize: vi.fn() })); | ||||||||||||
| vi.mock('../utils/mouse-hit.js', () => ({ findElementAtMouseEvent: vi.fn() })); | ||||||||||||
|
|
||||||||||||
| const ref = <T,>(current: T): MutableRefObject<T> => ({ current }); | ||||||||||||
|
|
||||||||||||
| function makeEvent( | ||||||||||||
| partial: Partial<MouseEvent> & Pick<MouseEvent, 'name'>, | ||||||||||||
| ): MouseEvent { | ||||||||||||
| return { | ||||||||||||
| col: 1, | ||||||||||||
| row: 1, | ||||||||||||
| shift: false, | ||||||||||||
| meta: false, | ||||||||||||
| ctrl: false, | ||||||||||||
| button: 'left', | ||||||||||||
| ...partial, | ||||||||||||
| } as MouseEvent; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| describe('CompletionCategoryMouseController', () => { | ||||||||||||
| const containerNode = { tag: 'container' } as unknown as DOMElement; | ||||||||||||
| const categoryNodes = [ | ||||||||||||
| { tag: 'all' }, | ||||||||||||
| { tag: 'file' }, | ||||||||||||
| { tag: 'session' }, | ||||||||||||
| ] as unknown as DOMElement[]; | ||||||||||||
| const categories = ['all', 'file', 'session'] as const; | ||||||||||||
| let onSelectCategory: ReturnType<typeof vi.fn>; | ||||||||||||
|
|
||||||||||||
| beforeEach(() => { | ||||||||||||
| vi.clearAllMocks(); | ||||||||||||
| onSelectCategory = vi.fn(); | ||||||||||||
| vi.mocked(useTerminalSize).mockReturnValue({ rows: 40, columns: 80 }); | ||||||||||||
| vi.mocked(findElementAtMouseEvent).mockReturnValue(null); | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| function mountAndGetHandler(): (event: MouseEvent) => void { | ||||||||||||
| render( | ||||||||||||
| <CompletionCategoryMouseController | ||||||||||||
| containerRef={ref(containerNode)} | ||||||||||||
| categoryRefs={ref(categoryNodes)} | ||||||||||||
| categories={categories} | ||||||||||||
| onSelectCategory={onSelectCategory} | ||||||||||||
| />, | ||||||||||||
| ); | ||||||||||||
| const call = vi.mocked(useMouseEvents).mock.calls.at(-1)!; | ||||||||||||
| expect(call[1]).toMatchObject({ isActive: true, tracking: 'button' }); | ||||||||||||
| return call[0]; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| it('selects the exact category under a left click', () => { | ||||||||||||
| vi.mocked(findElementAtMouseEvent).mockReturnValue(2); | ||||||||||||
| const handler = mountAndGetHandler(); | ||||||||||||
|
|
||||||||||||
| handler(makeEvent({ name: 'left-press', col: 19, row: 5 })); | ||||||||||||
|
|
||||||||||||
| expect(onSelectCategory).toHaveBeenCalledWith('session'); | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| it('ignores clicks outside category bounds', () => { | ||||||||||||
| const handler = mountAndGetHandler(); | ||||||||||||
|
|
||||||||||||
| handler(makeEvent({ name: 'left-press', col: 18, row: 5 })); | ||||||||||||
| handler(makeEvent({ name: 'left-press', col: 9, row: 5 })); | ||||||||||||
| handler(makeEvent({ name: 'left-press', col: 19, row: 6 })); | ||||||||||||
|
|
||||||||||||
| expect(onSelectCategory).not.toHaveBeenCalled(); | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| it('ignores non-press mouse events', () => { | ||||||||||||
| vi.mocked(findElementAtMouseEvent).mockReturnValue(2); | ||||||||||||
| const handler = mountAndGetHandler(); | ||||||||||||
|
Comment on lines
+87
to
+89
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The test that pins the Make the mock return a hittable index so only the guard can prevent the callback:
Suggested change
中文说明[Suggestion] 本应锁定「仅响应 left-press」守卫的测试实际上无法区分该守卫——已通过变异探测验证: — qwen3.8-max via Qwen Code /review (v0.21.10) |
||||||||||||
|
|
||||||||||||
| handler(makeEvent({ name: 'move', col: 19, row: 5 })); | ||||||||||||
| handler(makeEvent({ name: 'left-release', col: 19, row: 5 })); | ||||||||||||
|
|
||||||||||||
| expect(onSelectCategory).not.toHaveBeenCalled(); | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| it('uses terminal height when the composited frame overflows', () => { | ||||||||||||
| vi.mocked(findElementAtMouseEvent).mockReturnValue(2); | ||||||||||||
| const handler = mountAndGetHandler(); | ||||||||||||
|
|
||||||||||||
| handler(makeEvent({ name: 'left-press', col: 19, row: 5 })); | ||||||||||||
|
|
||||||||||||
| expect(findElementAtMouseEvent).toHaveBeenCalledWith( | ||||||||||||
| containerNode, | ||||||||||||
| categoryNodes, | ||||||||||||
| expect.objectContaining({ col: 19, row: 5 }), | ||||||||||||
| 40, | ||||||||||||
| 'rect', | ||||||||||||
| ); | ||||||||||||
| expect(onSelectCategory).toHaveBeenCalledWith('session'); | ||||||||||||
| }); | ||||||||||||
| }); | ||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2025 Qwen | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { type MutableRefObject, useCallback } from 'react'; | ||
| import { type DOMElement } from 'ink'; | ||
| import { useTerminalSize } from '../hooks/useTerminalSize.js'; | ||
| import { useMouseEvents } from '../hooks/useMouseEvents.js'; | ||
| import { type MouseEvent } from '../utils/mouse.js'; | ||
| import { findElementAtMouseEvent } from '../utils/mouse-hit.js'; | ||
| import { type SuggestionCategory } from '../utils/suggestions.js'; | ||
|
|
||
| type CompletionCategory = SuggestionCategory | 'all'; | ||
|
|
||
| interface CompletionCategoryMouseControllerProps { | ||
| containerRef: MutableRefObject<DOMElement | null>; | ||
| categoryRefs: MutableRefObject<Array<DOMElement | null>>; | ||
| categories: readonly CompletionCategory[]; | ||
| onSelectCategory: (category: CompletionCategory) => void; | ||
| } | ||
|
|
||
| /** | ||
| * Headless click layer for the completion category tabs. | ||
| * | ||
| * Coordinates assume the alternate-screen virtual viewport used by the owning | ||
| * suggestion UI. Ink bottom-pins an overflowing frame, so terminal rows must | ||
| * pass through `layoutRowForEvent` before they are compared with layout-space | ||
| * tab rectangles. Inline mode is intentionally unsupported; mount this only | ||
| * behind the owning surface's `mouseEnabled` gate. | ||
| */ | ||
| export function CompletionCategoryMouseController({ | ||
| containerRef, | ||
| categoryRefs, | ||
| categories, | ||
| onSelectCategory, | ||
| }: CompletionCategoryMouseControllerProps): null { | ||
| const { rows: terminalHeight } = useTerminalSize(); | ||
|
|
||
| const handleMouse = useCallback( | ||
| (event: MouseEvent) => { | ||
| if (event.name !== 'left-press') return; | ||
|
|
||
| const index = findElementAtMouseEvent( | ||
| containerRef.current, | ||
| categoryRefs.current, | ||
| event, | ||
| terminalHeight, | ||
| 'rect', | ||
| ); | ||
| if (index !== null && index < categories.length) { | ||
| onSelectCategory(categories[index]); | ||
| } | ||
|
Comment on lines
+52
to
+54
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The it('ignores a resolved index beyond the category list', () => {
vi.mocked(findElementAtMouseEvent).mockReturnValue(3);
// render with 3 categories, send a left-press, then:
expect(onSelectCategory).not.toHaveBeenCalled();
});中文说明[建议] it('ignores a resolved index beyond the category list', () => {
vi.mocked(findElementAtMouseEvent).mockReturnValue(3);
// 用 3 个分类渲染,发送一次鼠标左键按下,然后断言:
expect(onSelectCategory).not.toHaveBeenCalled();
});— qwen3.8-max via Qwen Code /review (v0.21.11)
Comment on lines
+52
to
+54
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The it('ignores a resolved index beyond the category list', () => {
vi.mocked(findElementAtMouseEvent).mockReturnValue(3);
// render with 3 categories, send a left-press, then:
expect(onSelectCategory).not.toHaveBeenCalled();
});中文说明[建议] it('ignores a resolved index beyond the category list', () => {
vi.mocked(findElementAtMouseEvent).mockReturnValue(3);
// 用 3 个分类渲染,发送一次鼠标左键按下,然后断言:
expect(onSelectCategory).not.toHaveBeenCalled();
});— qwen3.8-max via Qwen Code /review (v0.21.11) |
||
| }, | ||
| [containerRef, categoryRefs, categories, onSelectCategory, terminalHeight], | ||
| ); | ||
|
|
||
| useMouseEvents(handleMouse, { isActive: true, tracking: 'button' }); | ||
|
|
||
| return null; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] The SGR→0-based column offset (
event.col - 1) inCompletionCategoryMouseControlleris not pinned by any test — the mutationevent.col - 1→event.colsurvives all three controller tests. — Concrete cost: every test coordinate is strictly interior/exterior under both mappings (col 19 → 18 or 19 both land inside the mockedsessionrect[18,24); col 9 → 8 or 9 both miss). A click at SGR col 18 (layout col 17, the gap just before the Sessions tab) selects nothing under correct code but selects'session'under the mutant, so a one-column hit-region shift on the column side would ship green.Add a boundary case that flips under the mutation:
中文说明
CompletionCategoryMouseController里的 SGR→0 基列偏移(event.col - 1)没有被任何测试钉住——把event.col - 1突变成event.col后,三个控制器测试仍全部通过。具体代价:所有测试坐标在两种映射下都严格落在命中区内部或外部(col 19 → 18 或 19 都落进 mock 的session矩形[18,24);col 9 → 8 或 9 都错过)。真实输入可以区分二者:在 SGR col 18(布局 col 17,即 Sessions 标签前的空隙)点击时,正确代码什么都不选,而突变体会选中'session',因此列方向上一格的命中区偏移会在测试全绿的情况下溜进去。建议补充一个能在该突变下翻转的边界用例(见上方代码块)。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)