fix(settings): persist the last IME syllable when leaving the display-name field mid-composition (#9517)
RepoSettingsDraftInput holds unconfirmed IME text in local draft state and only persists on compositionend (so the async store echo cannot cancel the composition). If the user types a Hangul syllable and immediately navigates back out of project settings while it is still composing, no compositionend fires — the last syllable stays in the draft and never reaches the store, so the project's display name loses its final character. Users have to click back into the field or press Enter (both force compositionend) to make it stick. Flush the visible draft on blur and on unmount, guarded by lastPersistedRef so it is a no-op when a keystroke or compositionend already persisted that value. The flush closure is published from an effect (post-commit) so a discarded concurrent render cannot leave the cleanup pointing at uncommitted draft state. Adds blur / unmount / no-double-persist / no-op regression tests to the component's existing suite. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
cb19e79504
commit
a90ec540f2
|
|
@ -7,19 +7,39 @@ import { RepoSettingsDraftInput } from './RepositorySettingsDraftInput'
|
|||
|
||||
let container: HTMLDivElement
|
||||
let root: Root
|
||||
let unmounted: boolean
|
||||
|
||||
beforeEach(() => {
|
||||
container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
root = createRoot(container)
|
||||
unmounted = false
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
if (!unmounted) {
|
||||
act(() => {
|
||||
root.unmount()
|
||||
})
|
||||
}
|
||||
container.remove()
|
||||
})
|
||||
|
||||
// Why: some tests observe the flush that runs when the field unmounts
|
||||
// mid-composition; unmount explicitly and let afterEach skip the double-unmount.
|
||||
function unmountNow(): void {
|
||||
act(() => {
|
||||
root.unmount()
|
||||
})
|
||||
container.remove()
|
||||
})
|
||||
unmounted = true
|
||||
}
|
||||
|
||||
// Why: React routes onBlur through the bubbling focusout event.
|
||||
function blurInput(): void {
|
||||
act(() => {
|
||||
getInput().dispatchEvent(new FocusEvent('focusout', { bubbles: true }))
|
||||
})
|
||||
}
|
||||
|
||||
function render(props: {
|
||||
repoId: string
|
||||
|
|
@ -209,4 +229,60 @@ describe('RepoSettingsDraftInput', () => {
|
|||
expect(onTextChange).toHaveBeenCalledTimes(1)
|
||||
expect(onTextChange).toHaveBeenCalledWith('Renamed Repo Two')
|
||||
})
|
||||
|
||||
it('flushes an abandoned IME composition on blur', () => {
|
||||
const onTextChange = vi.fn()
|
||||
render({ repoId: 'repo-1', storeValue: '', onTextChange })
|
||||
|
||||
// Hangul: type a syllable but leave it unconfirmed, then blur the field
|
||||
// (no compositionend) — the last syllable must still be persisted.
|
||||
compositionStart()
|
||||
composingInput('홍')
|
||||
expect(onTextChange).not.toHaveBeenCalled()
|
||||
|
||||
blurInput()
|
||||
|
||||
expect(onTextChange).toHaveBeenCalledTimes(1)
|
||||
expect(onTextChange).toHaveBeenCalledWith('홍')
|
||||
})
|
||||
|
||||
it('flushes an abandoned IME composition when the field unmounts', () => {
|
||||
const onTextChange = vi.fn()
|
||||
render({ repoId: 'repo-1', storeValue: '', onTextChange })
|
||||
|
||||
// The user navigates back out of project settings before confirming the
|
||||
// syllable: the field unmounts with no compositionend.
|
||||
compositionStart()
|
||||
composingInput('홍')
|
||||
expect(onTextChange).not.toHaveBeenCalled()
|
||||
|
||||
unmountNow()
|
||||
|
||||
expect(onTextChange).toHaveBeenCalledTimes(1)
|
||||
expect(onTextChange).toHaveBeenCalledWith('홍')
|
||||
})
|
||||
|
||||
it('does not re-persist a confirmed value on blur or unmount', () => {
|
||||
const onTextChange = vi.fn()
|
||||
render({ repoId: 'repo-1', storeValue: '', onTextChange })
|
||||
|
||||
compositionStart()
|
||||
composingInput('홍')
|
||||
compositionEnd('홍')
|
||||
blurInput()
|
||||
unmountNow()
|
||||
|
||||
expect(onTextChange).toHaveBeenCalledTimes(1)
|
||||
expect(onTextChange).toHaveBeenCalledWith('홍')
|
||||
})
|
||||
|
||||
it('does not persist on blur or unmount when nothing was edited', () => {
|
||||
const onTextChange = vi.fn()
|
||||
render({ repoId: 'repo-1', storeValue: 'seed', onTextChange })
|
||||
|
||||
blurInput()
|
||||
unmountNow()
|
||||
|
||||
expect(onTextChange).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ export function RepoSettingsDraftInput({
|
|||
repoId,
|
||||
storeValue,
|
||||
onTextChange,
|
||||
onBlur,
|
||||
onCompositionStart,
|
||||
onCompositionEnd,
|
||||
...inputProps
|
||||
|
|
@ -32,9 +33,15 @@ export function RepoSettingsDraftInput({
|
|||
// repeats the already-persisted confirmed value; consume that one change so
|
||||
// the value is not persisted twice.
|
||||
const skipNextChangeRef = useRef<string | null>(null)
|
||||
// Why: the blur/unmount flush below must not re-persist text that a keystroke
|
||||
// or compositionend already committed. Track the last value handed to
|
||||
// onTextChange (and the store value we adopt) so the flush is a no-op unless a
|
||||
// composition was abandoned with genuinely unpersisted text.
|
||||
const lastPersistedRef = useRef(storeValue)
|
||||
|
||||
const persist = (text: string): void => {
|
||||
pendingStoreEchoesRef.current.push(text)
|
||||
lastPersistedRef.current = text
|
||||
onTextChange(text)
|
||||
}
|
||||
|
||||
|
|
@ -44,11 +51,13 @@ export function RepoSettingsDraftInput({
|
|||
pendingStoreEchoesRef.current = []
|
||||
composingRef.current = false
|
||||
skipNextChangeRef.current = null
|
||||
lastPersistedRef.current = storeValue
|
||||
return { repoId, text: storeValue }
|
||||
}
|
||||
if (storeValue === current.text) {
|
||||
pendingStoreEchoesRef.current = []
|
||||
skipNextChangeRef.current = null
|
||||
lastPersistedRef.current = storeValue
|
||||
return current
|
||||
}
|
||||
const pendingEchoIndex = pendingStoreEchoesRef.current.indexOf(storeValue)
|
||||
|
|
@ -60,11 +69,36 @@ export function RepoSettingsDraftInput({
|
|||
}
|
||||
pendingStoreEchoesRef.current = []
|
||||
skipNextChangeRef.current = null
|
||||
lastPersistedRef.current = storeValue
|
||||
return { repoId, text: storeValue }
|
||||
})
|
||||
}, [repoId, storeValue])
|
||||
|
||||
const text = draft.repoId === repoId ? draft.text : storeValue
|
||||
|
||||
// Why: onChange keeps `draft` current even mid-composition, but persistence is
|
||||
// deliberately held until compositionend. If the field blurs or unmounts while
|
||||
// a composition is still active — e.g. the user types a Hangul syllable and
|
||||
// immediately navigates back out of project settings — no compositionend fires,
|
||||
// so that last syllable lives only in `draft` and never reaches the store.
|
||||
// Flush the visible draft on blur and on unmount so it is not lost. Guarded to
|
||||
// stay a no-op when the text was already persisted (keystroke/compositionend).
|
||||
const flushRef = useRef<() => void>(() => {})
|
||||
// Why: publish the flush closure from an effect (post-commit) rather than during
|
||||
// render, so a discarded concurrent render can never leave the blur/unmount flush
|
||||
// pointing at uncommitted draft state.
|
||||
useEffect(() => {
|
||||
flushRef.current = (): void => {
|
||||
if (draft.repoId !== repoId || draft.text === lastPersistedRef.current) {
|
||||
return
|
||||
}
|
||||
composingRef.current = false
|
||||
skipNextChangeRef.current = draft.text
|
||||
persist(draft.text)
|
||||
}
|
||||
})
|
||||
useEffect(() => () => flushRef.current(), [])
|
||||
|
||||
return (
|
||||
<Input
|
||||
{...inputProps}
|
||||
|
|
@ -84,6 +118,10 @@ export function RepoSettingsDraftInput({
|
|||
skipNextChangeRef.current = null
|
||||
persist(nextText)
|
||||
}}
|
||||
onBlur={(e) => {
|
||||
flushRef.current()
|
||||
onBlur?.(e)
|
||||
}}
|
||||
onCompositionStart={(e) => {
|
||||
composingRef.current = true
|
||||
skipNextChangeRef.current = null
|
||||
|
|
|
|||
Loading…
Reference in New Issue