fix(web): avoid stealing focus when tapping terminal links#606
fix(web): avoid stealing focus when tapping terminal links#606nikolasdehor wants to merge 1 commit intoamantus-ai:mainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Adjusts terminal click-to-focus behavior so that tapping/clicking URL anchors inside terminal output preserves native link handling (notably improving mobile usability), while keeping existing click-to-focus behavior for non-link terminal clicks.
Changes:
- Update terminal root click handler to early-return when the click originates within an anchor (
a[href]). - Add regression tests covering link clicks vs. non-link terminal clicks with respect to paste-input focusing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| web/src/client/components/terminal.ts | Prevents the terminal click handler from focusing the paste input when a link is clicked. |
| web/src/client/components/terminal.test.ts | Adds tests asserting link clicks don’t trigger paste-input focus while normal terminal clicks still do. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const target = e.target as HTMLElement | null; | ||
| if (target?.closest('a[href]')) { |
There was a problem hiding this comment.
e.target is typed/cast as HTMLElement, but click targets can be non-Element (e.g., Text nodes). In that case target?.closest('a[href]') will throw at runtime because closest won’t exist. Guard with target instanceof Element (or use e.composedPath() and check elements) before calling closest.
| const target = e.target as HTMLElement | null; | |
| if (target?.closest('a[href]')) { | |
| const target = e.target; | |
| if (target instanceof Element && target.closest('a[href]')) { |
|
|
||
| expect(focusSpy).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The new link-click regression test only covers e.target being the <a> element. Since handleClick now uses target.closest(...), consider adding a test that dispatches the click from a Text node inside the anchor to prevent regressions where non-Element targets would throw and reintroduce focus-stealing/broken taps.
| it('should not steal focus when clicking text inside a terminal link', async () => { | |
| const terminalRoot = element.querySelector('.terminal-root') as HTMLElement | null; | |
| const pasteInput = element.querySelector( | |
| '.terminal-paste-input' | |
| ) as HTMLTextAreaElement | null; | |
| expect(terminalRoot).toBeTruthy(); | |
| expect(pasteInput).toBeTruthy(); | |
| const focusSpy = vi.spyOn(pasteInput as HTMLTextAreaElement, 'focus'); | |
| const link = document.createElement('a'); | |
| link.href = 'https://example.com'; | |
| link.className = 'terminal-link'; | |
| const textNode = document.createTextNode('example'); | |
| link.appendChild(textNode); | |
| terminalRoot?.appendChild(link); | |
| textNode.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); | |
| expect(focusSpy).not.toHaveBeenCalled(); | |
| }); |
Summary
vibe-terminalroot click handler from forcing paste-input focus when click target is a linkWhy
On mobile, tapping URLs in terminal output can be interrupted when the terminal click handler always redirects focus to the hidden paste input. This change keeps link taps native while preserving existing click-to-focus behavior elsewhere.
Testing
cd web && pnpm exec vitest run src/client/components/terminal.test.tscd web && pnpm exec biome check src/client/components/terminal.ts src/client/components/terminal.test.tsCloses #545