Skip to content

Commit 43d1ab3

Browse files
Fix for focus grabbing when follow-up returns + Unit Test (#2263)
1 parent 3f5f8bb commit 43d1ab3

File tree

2 files changed

+119
-6
lines changed

2 files changed

+119
-6
lines changed

webview-ui/src/components/chat/ChatView.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,13 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie
140140
case "followup":
141141
setTextAreaDisabled(isPartial)
142142
setClineAsk("followup")
143-
setEnableButtons(isPartial)
144-
// setPrimaryButtonText(undefined)
145-
// setSecondaryButtonText(undefined)
143+
// setting enable buttons to `false` would trigger a focus grab when
144+
// the text area is enabled which is undesirable.
145+
// We have no buttons for this tool, so no problem having them "enabled"
146+
// to workaround this issue. See #1358.
147+
setEnableButtons(true)
148+
setPrimaryButtonText(undefined)
149+
setSecondaryButtonText(undefined)
146150
break
147151
case "tool":
148152
if (!isAutoApproved(lastMessage)) {

webview-ui/src/components/chat/__tests__/ChatView.test.tsx

Lines changed: 112 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from "react"
2-
import { render, waitFor } from "@testing-library/react"
2+
import { render, waitFor, act } from "@testing-library/react"
33
import ChatView from "../ChatView"
44
import { ExtensionStateContextProvider } from "../../../context/ExtensionStateContext"
55
import { vscode } from "../../../utils/vscode"
@@ -60,17 +60,25 @@ interface ChatTextAreaProps {
6060
shouldDisableImages?: boolean
6161
}
6262

63+
const mockInputRef = React.createRef<HTMLInputElement>()
64+
const mockFocus = jest.fn()
65+
6366
jest.mock("../ChatTextArea", () => {
6467
const mockReact = require("react")
6568
return {
6669
__esModule: true,
6770
default: mockReact.forwardRef(function MockChatTextArea(
6871
props: ChatTextAreaProps,
69-
ref: React.ForwardedRef<HTMLInputElement>,
72+
ref: React.ForwardedRef<{ focus: () => void }>,
7073
) {
74+
// Use useImperativeHandle to expose the mock focus method
75+
React.useImperativeHandle(ref, () => ({
76+
focus: mockFocus,
77+
}))
78+
7179
return (
7280
<div data-testid="chat-textarea">
73-
<input ref={ref} type="text" onChange={(e) => props.onSend(e.target.value)} />
81+
<input ref={mockInputRef} type="text" onChange={(e) => props.onSend(e.target.value)} />
7482
</div>
7583
)
7684
}),
@@ -1087,3 +1095,104 @@ describe("ChatView - Sound Playing Tests", () => {
10871095
})
10881096
})
10891097
})
1098+
1099+
describe("ChatView - Focus Grabbing Tests", () => {
1100+
beforeEach(() => {
1101+
jest.clearAllMocks()
1102+
})
1103+
1104+
it("does not grab focus when follow-up question presented", async () => {
1105+
const sleep = async (timeout: number) => {
1106+
await act(async () => {
1107+
await new Promise((resolve) => setTimeout(resolve, timeout))
1108+
})
1109+
}
1110+
1111+
render(
1112+
<ExtensionStateContextProvider>
1113+
<ChatView
1114+
isHidden={false}
1115+
showAnnouncement={false}
1116+
hideAnnouncement={() => {}}
1117+
showHistoryView={() => {}}
1118+
/>
1119+
</ExtensionStateContextProvider>,
1120+
)
1121+
1122+
// First hydrate state with initial task and streaming
1123+
mockPostMessage({
1124+
autoApprovalEnabled: true,
1125+
alwaysAllowBrowser: true,
1126+
clineMessages: [
1127+
{
1128+
type: "say",
1129+
say: "task",
1130+
ts: Date.now(),
1131+
text: "Initial task",
1132+
},
1133+
{
1134+
type: "say",
1135+
say: "api_req_started",
1136+
ts: Date.now(),
1137+
text: JSON.stringify({}),
1138+
partial: true,
1139+
},
1140+
],
1141+
})
1142+
1143+
// process messages
1144+
await sleep(0)
1145+
// wait for focus updates (can take 50msecs)
1146+
await sleep(100)
1147+
1148+
const FOCUS_CALLS_ON_INIT = 2
1149+
expect(mockFocus).toHaveBeenCalledTimes(FOCUS_CALLS_ON_INIT)
1150+
1151+
// Finish task, and send the followup ask message (streaming unfinished)
1152+
mockPostMessage({
1153+
autoApprovalEnabled: true,
1154+
alwaysAllowBrowser: true,
1155+
clineMessages: [
1156+
{
1157+
type: "say",
1158+
say: "task",
1159+
ts: Date.now(),
1160+
text: "Initial task",
1161+
},
1162+
{
1163+
type: "ask",
1164+
ask: "followup",
1165+
ts: Date.now(),
1166+
text: JSON.stringify({}),
1167+
partial: true,
1168+
},
1169+
],
1170+
})
1171+
1172+
// allow messages to be processed
1173+
await sleep(0)
1174+
1175+
// Finish the followup ask message (streaming finished)
1176+
mockPostMessage({
1177+
autoApprovalEnabled: true,
1178+
alwaysAllowBrowser: true,
1179+
clineMessages: [
1180+
{
1181+
type: "ask",
1182+
ask: "followup",
1183+
ts: Date.now(),
1184+
text: JSON.stringify({}),
1185+
},
1186+
],
1187+
})
1188+
1189+
// allow messages to be processed
1190+
await sleep(0)
1191+
1192+
// wait for focus updates (can take 50msecs)
1193+
await sleep(100)
1194+
1195+
// focus() should not have been called again
1196+
expect(mockFocus).toHaveBeenCalledTimes(FOCUS_CALLS_ON_INIT)
1197+
})
1198+
})

0 commit comments

Comments
 (0)