Skip to content

Commit 7f77ae8

Browse files
kevin9327claudedavidmckayv
authored
Confirm a composed character with Enter in the web app's text fields, instead of saving or moving on (#576)
Japanese, Chinese and Korean are typed through an input method, and Enter is how the character being built is confirmed. That press is still a keydown with `key === "Enter"`: Chromium marks it `isComposing`, and WebKit sends it after `compositionend` with key code 229. Three fields acted on it with the text still unconfirmed: - a coworker's name or title, edited in place, was saved; - the new-coworker wizard moved on to its next step. Its handler runs ahead of the questionnaire primitive and prevents default, which also skipped the primitive's own `isComposing || keyCode === 229` check; - a boundary rule was saved into the policy in force. The chat composer already skips this Enter through `prompt-area`. The two checks now live in one helper, `isComposing`, and each of the three handlers asks it. An ordinary Enter acts as before. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: David McKay <david@copilotkit.ai>
1 parent 513383b commit 7f77ae8

6 files changed

Lines changed: 251 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
88

99
## Unreleased
1010

11+
### Enter that confirms a typed character no longer saves a name, a rule or a wizard step
12+
13+
Japanese, Chinese and Korean are typed through an input method, where Enter confirms the character
14+
being built. In three fields that Enter also acted: editing a coworker's name or title saved it with
15+
the character still unconfirmed, the new-coworker wizard moved on to its next step, and a boundary
16+
rule was saved into the policy in force. Those fields now wait for the character, the way the chat
17+
composer already does, and an ordinary Enter works as before.
18+
1119
### A built-in coworker can be edited where the deployment's own Bot is on localhost
1220

1321
A coworker created as Built in is stored pointing at the managed Bot's address. Editing its name,

app/src/components/agents/agent-dialog.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ import {
7070
updateAgentMutationOptions,
7171
} from "@/lib/agents/mutations";
7272
import { type AgentProfile, agentQueryOptions } from "@/lib/agents/queries";
73+
import { isComposing } from "@/lib/composing";
7374
import { agentPluginsQueryOptions } from "@/lib/plugins/queries";
7475
import { readToolName } from "@/lib/plugins/tool-name";
7576

@@ -438,7 +439,9 @@ function EditableTextItem({
438439
autoFocus
439440
onChange={(event) => setDraft(event.target.value)}
440441
onKeyDown={(event) => {
441-
if (event.key === "Enter") {
442+
// Not the Enter that confirms a composed character: that one would save the value
443+
// before the person has finished typing it.
444+
if (event.key === "Enter" && !isComposing(event)) {
442445
event.preventDefault();
443446
void submit();
444447
}

app/src/components/agents/create-agent-dialog.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
type ConnectionVerdict,
3939
testAgentConnection,
4040
} from "@/lib/agents/queries";
41+
import { isComposing } from "@/lib/composing";
4142
import { queryClient } from "@/query-client";
4243

4344
/**
@@ -254,12 +255,15 @@ function CreateAgentWizard({
254255
* questionnaire's own submit path refuses any item it does not consider answered, and it
255256
* cannot see these fields: the identity inputs are this dialog's own, not registered
256257
* answers. Running first and preventing default also keeps the primitive's Enter
257-
* handling out of the way; a textarea keeps Enter for its line breaks.
258+
* handling out of the way; a textarea keeps Enter for its line breaks. The Enter that
259+
* confirms a composed character is left alone, as the primitive leaves it: it finishes a
260+
* character, not the step.
258261
*/
259262
onKeyDown={(event) => {
260263
if (
261264
event.key === "Enter" &&
262265
!event.shiftKey &&
266+
!isComposing(event) &&
263267
event.target instanceof HTMLInputElement
264268
) {
265269
event.preventDefault();

app/src/lib/composing.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import type { KeyboardEvent } from "react";
2+
3+
/**
4+
* Whether a keydown belongs to a character an input method is still composing.
5+
*
6+
* Japanese, Chinese and Korean are typed through an input method, and Enter is how the character
7+
* being built is confirmed. That press still arrives as a keydown with `key === "Enter"`: Chromium
8+
* marks it `isComposing`, and WebKit sends it after `compositionend` with the key code 229 instead.
9+
* A field that acts on Enter without asking this acts on text the person has not finished typing.
10+
*
11+
* The same two checks the libraries under this app already make: `prompt-area`, which draws the chat
12+
* composer, and the questionnaire primitive both skip a keydown either one describes.
13+
*/
14+
export function isComposing(event: KeyboardEvent): boolean {
15+
return event.nativeEvent.isComposing || event.keyCode === 229;
16+
}

app/src/routes/_authed/admin/boundaries.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { useState } from "react";
44
import { PageSection, PageShell } from "@/components/layout/page-shell";
55
import { Button } from "@/components/ui/button";
66
import { Input } from "@/components/ui/input";
7+
import { isComposing } from "@/lib/composing";
78
import { saveActionPolicyMutationOptions } from "@/lib/computers/mutations";
89
import {
910
type ActionPolicy,
@@ -231,7 +232,9 @@ function BoundariesPage() {
231232
setTested(null);
232233
}}
233234
onKeyDown={(event) => {
234-
if (event.key === "Enter") addRule(draft);
235+
// Not the Enter that confirms a composed character, which would put a half-typed
236+
// rule into the policy in force.
237+
if (event.key === "Enter" && !isComposing(event)) addRule(draft);
235238
}}
236239
placeholder='tool.name == "computer_click" && contains(element.name, "submit")'
237240
value={draft}

app/tests/composing-enter.test.tsx

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
import {
2+
afterAll,
3+
afterEach,
4+
beforeAll,
5+
beforeEach,
6+
expect,
7+
test,
8+
} from "bun:test";
9+
import { GlobalRegistrator } from "@happy-dom/global-registrator";
10+
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
11+
import {
12+
createMemoryHistory,
13+
createRootRoute,
14+
createRouter,
15+
RouterProvider,
16+
} from "@tanstack/react-router";
17+
import { act, cleanup, fireEvent, render } from "@testing-library/react";
18+
import userEvent from "@testing-library/user-event";
19+
import type { ComponentType, ReactNode } from "react";
20+
import { AgentDialog } from "@/components/agents/agent-dialog";
21+
import { CreateAgentDialog } from "@/components/agents/create-agent-dialog";
22+
import { type AgentProfile, agentKeys } from "@/lib/agents/queries";
23+
import { computerKeys } from "@/lib/computers/queries";
24+
import { Route as BoundariesRoute } from "@/routes/_authed/admin/boundaries";
25+
26+
/**
27+
* The Enter that confirms a character an input method is composing is not an Enter.
28+
*
29+
* Japanese, Chinese and Korean are typed through an input method (IME), and Enter is how the
30+
* character being built is confirmed. That press still arrives as a keydown with `key === "Enter"`.
31+
* Chromium marks it `isComposing`, and WebKit sends it after `compositionend` with the key code 229.
32+
* The chat composer already skips it: `prompt-area` checks `isComposing` on every Enter it handles.
33+
* Three text fields in the app acted on it instead, each with a text field's text still unconfirmed:
34+
* a coworker's name saved in place, the new-coworker wizard moving on to its next step, and a
35+
* boundary rule saved into the policy in force.
36+
*
37+
* THE HARNESS IS THIS REPOSITORY'S: `GlobalRegistrator` in `beforeAll`/`afterAll`, `cleanup` in
38+
* `afterEach`, queries off `render()`'s own return, and a `QueryClient` with `retry: false`. Each
39+
* screen is its real component, drawn inside a router of one route rather than through its own
40+
* route singleton, so nothing here is left pointing another file's router at a decoy.
41+
*/
42+
43+
beforeAll(() => GlobalRegistrator.register());
44+
afterEach(cleanup);
45+
afterAll(() => GlobalRegistrator.unregister());
46+
47+
const originalFetch = global.fetch;
48+
49+
/** Every write a screen sent, as `METHOD path`, so "nothing was saved" is an assertion. */
50+
let writes: { request: string; body: unknown }[] = [];
51+
52+
const PROFILE: AgentProfile = {
53+
id: "expenses",
54+
name: "Expenses",
55+
title: "Finance Operations",
56+
roleDescription: "Review receipts.",
57+
avatarSeed: "expenses",
58+
visibility: "private",
59+
endpoint: null,
60+
builtIn: true,
61+
hasAuth: false,
62+
hasCallbackToken: false,
63+
hidden: false,
64+
systemOwned: false,
65+
canManage: true,
66+
mine: true,
67+
};
68+
69+
const POLICY = { mode: "enforce", deny: [], allow: [] };
70+
71+
beforeEach(() => {
72+
writes = [];
73+
global.fetch = Object.assign(
74+
async (
75+
path: Parameters<typeof fetch>[0],
76+
init?: Parameters<typeof fetch>[1],
77+
) => {
78+
const method = init?.method ?? "GET";
79+
const body =
80+
typeof init?.body === "string" ? JSON.parse(init.body) : undefined;
81+
if (method !== "GET") writes.push({ request: `${method} ${path}`, body });
82+
const json = (value: unknown) => Response.json(value);
83+
if (path === "/api/computers/policy") {
84+
return json({ policy: method === "PUT" ? body : POLICY });
85+
}
86+
if (path === "/api/agents/capabilities") {
87+
return json({ capabilities: { builtInAvailable: true } });
88+
}
89+
if (path === `/api/agents/${PROFILE.id}`) {
90+
return json({ agent: method === "PATCH" ? PROFILE : PROFILE });
91+
}
92+
return new Response(null, { status: 404 });
93+
},
94+
{ preconnect: originalFetch.preconnect },
95+
);
96+
});
97+
98+
afterEach(() => {
99+
global.fetch = originalFetch;
100+
});
101+
102+
/** A screen drawn inside a router of one route, for the `Link` and `useNavigate` it holds. */
103+
function draw(screen: ReactNode, seed?: (client: QueryClient) => void) {
104+
const queryClient = new QueryClient({
105+
defaultOptions: { queries: { retry: false } },
106+
});
107+
seed?.(queryClient);
108+
const router = createRouter({
109+
history: createMemoryHistory({ initialEntries: ["/"] }),
110+
routeTree: createRootRoute({ component: () => screen }),
111+
});
112+
return render(
113+
<QueryClientProvider client={queryClient}>
114+
<RouterProvider router={router} />
115+
</QueryClientProvider>,
116+
);
117+
}
118+
119+
/** Both shapes the confirming Enter arrives in: Chromium's, then WebKit's. */
120+
async function confirmComposedCharacter(field: Element) {
121+
await act(async () => {
122+
fireEvent.keyDown(field, { key: "Enter", isComposing: true });
123+
fireEvent.keyDown(field, { key: "Enter", keyCode: 229 });
124+
});
125+
}
126+
127+
/** A person replacing what a field holds, one key at a time. */
128+
async function type(field: Element, value: string) {
129+
const user = userEvent.setup({ document: field.ownerDocument });
130+
await user.clear(field);
131+
await user.type(field, value);
132+
}
133+
134+
/** Long enough for a write the keydown started to have reached `fetch`. */
135+
async function settle() {
136+
await act(async () => {
137+
await new Promise((resolve) => setTimeout(resolve, 50));
138+
});
139+
}
140+
141+
test("a coworker's name is not saved by the Enter that confirms a composed character", async () => {
142+
const view = draw(
143+
<AgentDialog agentId={PROFILE.id} onClose={() => {}} open />,
144+
(client) => client.setQueryData(agentKeys.detail(PROFILE.id), PROFILE),
145+
);
146+
147+
fireEvent.click(await view.findByRole("button", { name: "Edit name" }));
148+
const field = view.getByDisplayValue(PROFILE.name);
149+
await type(field, "経費");
150+
151+
await confirmComposedCharacter(field);
152+
await settle();
153+
expect(writes).toEqual([]);
154+
expect(view.getByDisplayValue("経費")).toBeTruthy();
155+
156+
// An ordinary Enter still saves, once.
157+
await act(async () => {
158+
fireEvent.keyDown(field, { key: "Enter", keyCode: 13 });
159+
});
160+
await settle();
161+
expect(writes.map((write) => write.request)).toEqual([
162+
`PATCH /api/agents/${PROFILE.id}`,
163+
]);
164+
expect(writes[0]?.body).toMatchObject({ name: "経費" });
165+
});
166+
167+
test("the new-coworker wizard does not move on from the Enter that confirms a composed character", async () => {
168+
const view = draw(
169+
<CreateAgentDialog onClose={() => {}} onCreated={() => {}} open />,
170+
);
171+
172+
const name = await view.findByLabelText("Name");
173+
await type(name, "経費");
174+
await type(view.getByLabelText("Title"), "Finance Operations");
175+
await type(view.getByLabelText("Role"), "Review receipts.");
176+
177+
await confirmComposedCharacter(name);
178+
await settle();
179+
expect(view.getByText("Step 1 of 3")).toBeTruthy();
180+
181+
// An ordinary Enter still means Continue.
182+
await act(async () => {
183+
fireEvent.keyDown(name, { key: "Enter", keyCode: 13 });
184+
});
185+
expect(await view.findByText("Step 2 of 3")).toBeTruthy();
186+
});
187+
188+
test("a boundary rule is not saved by the Enter that confirms a composed character", async () => {
189+
const Boundaries = BoundariesRoute.options.component as ComponentType;
190+
const view = draw(<Boundaries />, (client) =>
191+
client.setQueryData(computerKeys.policy(), POLICY),
192+
);
193+
194+
const field = await view.findByLabelText("A rule, written in CEL");
195+
const rule = 'contains(element.name, "送信")';
196+
await type(field, rule);
197+
198+
await confirmComposedCharacter(field);
199+
await settle();
200+
expect(writes).toEqual([]);
201+
expect(view.getByDisplayValue(rule)).toBeTruthy();
202+
203+
// An ordinary Enter still adds the rule, once.
204+
await act(async () => {
205+
fireEvent.keyDown(field, { key: "Enter", keyCode: 13 });
206+
});
207+
await settle();
208+
expect(writes).toEqual([
209+
{
210+
request: "PUT /api/computers/policy",
211+
body: { ...POLICY, deny: [rule] },
212+
},
213+
]);
214+
});

0 commit comments

Comments
 (0)