diff --git a/src/renderer/src/components/settings/RepositoryPane.tsx b/src/renderer/src/components/settings/RepositoryPane.tsx index 9999b74ea..cf4199749 100644 --- a/src/renderer/src/components/settings/RepositoryPane.tsx +++ b/src/renderer/src/components/settings/RepositoryPane.tsx @@ -1,4 +1,4 @@ -import { useCallback, useRef, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import type { OrcaHooks, Repo, RepoHookSettings } from '../../../../shared/types' import { getRepoKindLabel, isFolderRepo } from '../../../../shared/repo-kind' import { Button } from '../ui/button' @@ -31,6 +31,61 @@ type RepositoryPaneProps = { removeProject: (repoId: string) => void } +type RepoTextDraft = { repoId: string; text: string } + +// Why: updateRepo persists via async IPC before the store value updates, so a +// store-controlled input resets mid-IME-composition (Hangul decomposes into +// jamo). Keep keystrokes in local draft state; persist stays per-keystroke. +export function RepoSettingsDraftInput({ + repoId, + storeValue, + onTextChange, + ...inputProps +}: { + repoId: string + storeValue: string + onTextChange: (text: string) => void +} & Omit, 'value' | 'onChange'>): React.JSX.Element { + const [draft, setDraft] = useState({ repoId, text: storeValue }) + const pendingStoreEchoesRef = useRef([]) + + useEffect(() => { + setDraft((current) => { + if (current.repoId !== repoId) { + pendingStoreEchoesRef.current = [] + return { repoId, text: storeValue } + } + if (storeValue === current.text) { + pendingStoreEchoesRef.current = [] + return current + } + const pendingEchoIndex = pendingStoreEchoesRef.current.indexOf(storeValue) + if (pendingEchoIndex !== -1) { + // Why: queued updateRepo calls can echo older input text after newer + // keystrokes; accepting that echo re-cancels active IME composition. + pendingStoreEchoesRef.current.splice(0, pendingEchoIndex + 1) + return current + } + pendingStoreEchoesRef.current = [] + return { repoId, text: storeValue } + }) + }, [repoId, storeValue]) + + const text = draft.repoId === repoId ? draft.text : storeValue + return ( + { + const nextText = e.target.value + pendingStoreEchoesRef.current.push(nextText) + setDraft({ repoId, text: nextText }) + onTextChange(nextText) + }} + /> + ) +} + export function matchesRepositoryIdentitySearch(query: string, repo: Repo): boolean { const normalizedQuery = normalizeSettingsSearchQuery(query) if (!normalizedQuery) { @@ -215,14 +270,14 @@ export function RepositoryPane({ className="space-y-2" forceVisible={forceFullPaneForRepoMatch} > - - - updateRepo(repo.id, { - displayName: e.target.value - }) - } + + updateRepo(repo.id, { displayName: text })} className="h-9 text-sm" /> @@ -292,13 +347,12 @@ export function RepositoryPane({ ) : null} - - updateRepo(repo.id, { - worktreeBasePath: e.target.value.trim() ? e.target.value : undefined - }) + onTextChange={(text) => + updateRepo(repo.id, { worktreeBasePath: text.trim() ? text : undefined }) } className="h-9 text-sm" /> diff --git a/src/renderer/src/components/settings/RepositoryPaneDraftInput.test.tsx b/src/renderer/src/components/settings/RepositoryPaneDraftInput.test.tsx new file mode 100644 index 000000000..e97134a7d --- /dev/null +++ b/src/renderer/src/components/settings/RepositoryPaneDraftInput.test.tsx @@ -0,0 +1,112 @@ +// @vitest-environment happy-dom + +import React, { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { RepoSettingsDraftInput } from './RepositoryPane' + +let container: HTMLDivElement +let root: Root + +beforeEach(() => { + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => { + root.unmount() + }) + container.remove() +}) + +function render(props: { + repoId: string + storeValue: string + onTextChange: (text: string) => void +}): void { + act(() => { + root.render(React.createElement(RepoSettingsDraftInput, props)) + }) +} + +function getInput(): HTMLInputElement { + const input = container.querySelector('input') + if (!input) { + throw new Error('input not rendered') + } + return input +} + +function typeText(text: string): void { + act(() => { + const input = getInput() + // Why: React reads controlled-input changes via the native value setter; + // assigning input.value directly is swallowed by React's value tracking. + const setValue = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set + setValue?.call(input, text) + input.dispatchEvent(new Event('input', { bubbles: true })) + }) +} + +describe('RepoSettingsDraftInput', () => { + it('keeps draft text while the store still holds the previous value (IME regression)', () => { + const onTextChange = vi.fn() + render({ repoId: 'repo-1', storeValue: '가', onTextChange }) + + typeText('가나') + + // Why: updateRepo persists via async IPC, so the store re-renders the pane + // with the stale value first. Reverting the input here is what aborted the + // Hangul IME composition (가나다 → ㄱㅏㄴㅏㄷㅏ). + render({ repoId: 'repo-1', storeValue: '가', onTextChange }) + + expect(getInput().value).toBe('가나') + expect(onTextChange).toHaveBeenCalledWith('가나') + }) + + it('keeps draft text when a stale store echo arrives after newer keystrokes', () => { + const onTextChange = vi.fn() + render({ repoId: 'repo-1', storeValue: '', onTextChange }) + + typeText('가') + typeText('가나') + + // Stale repos:changed echo of the first keystroke. + render({ repoId: 'repo-1', storeValue: '가', onTextChange }) + + expect(getInput().value).toBe('가나') + }) + + it('accepts same-repo store changes that did not come from the input draft', () => { + const onTextChange = vi.fn() + render({ repoId: 'repo-1', storeValue: '../custom-worktrees', onTextChange }) + + render({ repoId: 'repo-1', storeValue: '', onTextChange }) + + expect(getInput().value).toBe('') + }) + + it('resets the draft when the pane switches repos', () => { + const onTextChange = vi.fn() + render({ repoId: 'repo-1', storeValue: 'Repo One', onTextChange }) + + typeText('Renamed') + + render({ repoId: 'repo-2', storeValue: 'Repo Two', onTextChange }) + + expect(getInput().value).toBe('Repo Two') + }) + + it('persists every keystroke through onTextChange', () => { + const onTextChange = vi.fn() + render({ repoId: 'repo-1', storeValue: '', onTextChange }) + + typeText('a') + typeText('ab') + + expect(onTextChange).toHaveBeenNthCalledWith(1, 'a') + expect(onTextChange).toHaveBeenNthCalledWith(2, 'ab') + }) +}) diff --git a/tests/e2e/settings-display-name-ime.spec.ts b/tests/e2e/settings-display-name-ime.spec.ts new file mode 100644 index 000000000..d00579109 --- /dev/null +++ b/tests/e2e/settings-display-name-ime.spec.ts @@ -0,0 +1,155 @@ +/** + * Regression test for Hangul IME composition in the repository Display Name + * setting (jamo decomposition: typing 가나다 produced ㄱㅏㄴㅏㄷㅏ). + * + * Why CDP: Playwright's keyboard API cannot drive IME composition. The CDP + * `Input.imeSetComposition` / `Input.insertText` commands go through Blink's + * real composition pipeline, so a controlled-input value reset mid-composition + * cancels the composition exactly like a real OS IME session. + */ +import type { CDPSession, Locator, Page } from '@stablyai/playwright-test' +import { test, expect } from './helpers/orca-app' +import { getStoreState, waitForSessionReady } from './helpers/store' +import type { Repo } from '../../src/shared/types' + +async function openRepoSettings(page: Page, repoId: string): Promise { + await page.evaluate((repoId) => { + const state = window.__store!.getState() + state.openSettingsTarget({ pane: 'repo', repoId }) + state.openSettingsPage() + }, repoId) + await expect(page.getByPlaceholder('Search settings')).toBeVisible({ timeout: 10_000 }) + // Why: first-run announcements can cover the settings pane on fresh profiles. + const maybeLaterButton = page.getByRole('button', { name: 'Maybe Later' }) + if (await maybeLaterButton.isVisible({ timeout: 1_000 }).catch(() => false)) { + await maybeLaterButton.click() + } +} + +/** + * Minimal 2-set Korean IME combination table for the key sequence ㄱㅏㄴㅏㄷㅏ. + * Returns the next composition text, plus the syllable to commit when the new + * key cannot join the pending composition (간 + ㅏ → commit 가, compose 나). + */ +function combineJamo(pending: string, key: string): { commit?: string; compose: string } { + const joins: Record = { + 'ㄱ+ㅏ': { compose: '가' }, + '가+ㄴ': { compose: '간' }, + '간+ㅏ': { commit: '가', compose: '나' }, + '나+ㄷ': { compose: '낟' }, + '낟+ㅏ': { commit: '나', compose: '다' } + } + if (!pending) { + return { compose: key } + } + // Why: like a real IME, a non-joinable key commits the pending text and + // starts a fresh composition with just the new key. + return joins[`${pending}+${key}`] ?? { commit: pending, compose: key } +} + +/** + * Emulates a real 2-set Korean IME typing 가나다 slowly (keys: ㄱㅏㄴㅏㄷㅏ). + * + * Adaptive on purpose: a real OS IME is cancelled (ImeCancelComposition) when + * the page rewrites the text backing its composition — exactly what a + * controlled React input does when its store echo is async. That browser→IME + * channel is not observable from page JS (no compositionend fires), so the + * emulator detects the same condition directly: an input event whose value the + * page clobbered right afterwards. After a clobber the IME restarts from an + * empty state on the next key, which is what turned 가나다 into ㄱㅏㄴㅏㄷㅏ. + * + * The per-key delay models slow human typing: it gives the async updateRepo + * store echo time to land between keystrokes, which is exactly the condition + * that produced the full jamo decomposition. + */ +async function typeHangulGanadaSlowly( + session: CDPSession, + page: Page, + input: Locator +): Promise { + await input.evaluate((el) => { + const w = window as unknown as { __imeClobbered?: boolean } + w.__imeClobbered = false + el.addEventListener('input', () => { + const seen = (el as HTMLInputElement).value + // Why: React restores the controlled value after the input event's + // dispatch but later than its microtasks, so poll on a macrotask. + setTimeout(() => { + if ((el as HTMLInputElement).value !== seen) { + w.__imeClobbered = true + } + }, 0) + }) + }) + const takeClobbered = (): Promise => + page.evaluate(() => { + const w = window as unknown as { __imeClobbered?: boolean } + const clobbered = w.__imeClobbered === true + w.__imeClobbered = false + return clobbered + }) + + let pending = '' + for (const key of ['ㄱ', 'ㅏ', 'ㄴ', 'ㅏ', 'ㄷ', 'ㅏ']) { + if (await takeClobbered()) { + pending = '' + } + const { commit, compose } = combineJamo(pending, key) + if (commit) { + await session.send('Input.insertText', { text: commit }) + } + await session.send('Input.imeSetComposition', { + text: compose, + selectionStart: compose.length, + selectionEnd: compose.length + }) + pending = compose + // Slow typing: let the async store echo land before the next key. + await page.waitForTimeout(200) + } + // A clobbered final composition was already committed by the page's own + // echo; only a still-live composition needs an explicit IME commit. + if (!(await takeClobbered())) { + await session.send('Input.insertText', { text: pending }) + } +} + +test.describe('Repository Display Name IME composition', () => { + test('keeps Hangul syllables composed while typing slowly', async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + + const repos = await getStoreState(orcaPage, 'repos') + expect(repos.length).toBeGreaterThan(0) + const repo = repos[0] + + await openRepoSettings(orcaPage, repo.id) + + const repoSection = orcaPage.locator(`[data-settings-section="repo-${repo.id}"]`) + const displayNameInput = repoSection.getByLabel('Display Name') + await expect(displayNameInput).toHaveValue(repo.displayName) + + // Clear and wait for the async store echo to settle before composing. + await displayNameInput.click() + await displayNameInput.fill('') + await expect(displayNameInput).toHaveValue('') + + const session = await orcaPage.context().newCDPSession(orcaPage) + await typeHangulGanadaSlowly(session, orcaPage, displayNameInput) + + // Why: with the store-bound controlled input, the async updateRepo echo + // reset the field mid-composition, aborting the IME session per keystroke + // and committing bare jamo (ㄱㅏㄴㅏㄷㅏ) instead of syllables. + await expect(displayNameInput).toHaveValue('가나다') + + // The per-keystroke persist still reaches the store. + await expect + .poll( + async () => { + const current = await getStoreState(orcaPage, 'repos') + return current.find((entry) => entry.id === repo.id)?.displayName + }, + { timeout: 5_000, message: 'display name did not persist to the store' } + ) + .toBe('가나다') + }) +})