fix(conversation): restore the composer caret after the draft insert (#425)
A programmatic setDraft commit resets the controlled textarea's caret (observed landing at the start of the value), so every later insert probed the reset position and the stack drifted (A|B + C + D became |DACB). placeComposerCaretAfterInsert puts the caret right after the inserted text once the value commit lands - the index accounts for the left-side separating space - and is guarded by a value-sync check plus a two-frame retry, so a newer edit or a different composer wins untouched. spliceInsert now reports the caret index together with the spliced draft so the placement math cannot drift from the join logic. Nothing else moves: the pure insertAtCaret surface is unchanged.
This commit is contained in:
懿驹-伯乐
@@ -14,6 +14,15 @@
|
||||
* insert into the middle of a sentence keeps single-space separation like
|
||||
* the append path. An unknown/stale caret falls back to appending at the
|
||||
* end (the pre-fix behavior).
|
||||
*
|
||||
* The caret is also *restored* after the insert: committing a programmatic
|
||||
* draft change resets the controlled textarea's caret (observed landing at
|
||||
* the start of the value), which would make every later insert probe the
|
||||
* reset position and drift the stack (A|B + C + D ended up as |DACB). The
|
||||
* placement (`placeComposerCaretAfterInsert`) puts the caret right after the
|
||||
* inserted text once the value commit lands — the index accounts for the
|
||||
* separating space on the left, so stacked inserts stay at their running
|
||||
* position (A|B + C + D → ACD|B).
|
||||
*/
|
||||
import type { Context, SidebarConversation } from '../context-types.ts'
|
||||
|
||||
@@ -24,24 +33,62 @@ export interface DraftCaret {
|
||||
}
|
||||
|
||||
/**
|
||||
* Splice `text` into `draft` at `caret` (replacing any live selection) with
|
||||
* whitespace-aware joins. `caret === null` (position unknown) appends at the
|
||||
* end, exactly like the original behavior. Pure string math — unit-tested
|
||||
* directly.
|
||||
* The spliced draft plus the caret index (in that draft) right after the
|
||||
* inserted text — the left-side separating space shifts the caret by one,
|
||||
* which naive `start + text.length` misses (it would land before the tail).
|
||||
*/
|
||||
export function insertAtCaret(draft: string, text: string, caret: DraftCaret | null): string {
|
||||
interface SpliceResult {
|
||||
draft: string
|
||||
caretAfter: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Splice `text` into `draft` at `caret` (replacing any live selection) with
|
||||
* whitespace-aware joins and report the caret position right after the
|
||||
* inserted text. `caret === null` (position unknown) appends at the end,
|
||||
* exactly like the original behavior.
|
||||
*/
|
||||
function spliceInsert(draft: string, text: string, caret: DraftCaret | null): SpliceResult {
|
||||
if (caret === null || draft === '') {
|
||||
return draft.trim() === '' ? text : `${draft} ${text}`
|
||||
const next = draft.trim() === '' ? text : `${draft} ${text}`
|
||||
return { draft: next, caretAfter: next.length }
|
||||
}
|
||||
const prefix = draft.slice(0, caret.start)
|
||||
const suffix = draft.slice(caret.end)
|
||||
if (prefix === '' && suffix === '') return text
|
||||
if (prefix === '' && suffix === '') return { draft: text, caretAfter: text.length }
|
||||
// One separating space, but never doubled against adjacent whitespace
|
||||
// (or the string edges) — mirrors how typing in the middle of a sentence
|
||||
// behaves.
|
||||
const left = prefix === '' || /\s$/.test(prefix) ? '' : ' '
|
||||
const right = suffix === '' || /^\s/.test(suffix) ? '' : ' '
|
||||
return `${prefix}${left}${text}${right}${suffix}`
|
||||
return {
|
||||
draft: `${prefix}${left}${text}${right}${suffix}`,
|
||||
caretAfter: prefix.length + left.length + text.length,
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The spliced draft string (see {@link spliceInsert}); pure string math —
|
||||
* unit-tested directly.
|
||||
*/
|
||||
export function insertAtCaret(draft: string, text: string, caret: DraftCaret | null): string {
|
||||
return spliceInsert(draft, text, caret).draft
|
||||
}
|
||||
|
||||
/**
|
||||
* Locate the composer `<textarea>` in the conversation column: prefer the
|
||||
* `data-phase`-tagged textarea (the composer's marker), falling back to any
|
||||
* textarea in the column, then to a bare data-phase textarea (older host
|
||||
* layouts without the column attribute). Null in jsdom-less hosts.
|
||||
*/
|
||||
function findComposerTextarea(): HTMLTextAreaElement | null {
|
||||
if (typeof document === 'undefined') return null
|
||||
const column = document.querySelector('#root [data-slot="conversation"]')
|
||||
const find = (scope: ParentNode): HTMLTextAreaElement | null =>
|
||||
scope.querySelector('textarea[data-phase]') ?? scope.querySelector('textarea')
|
||||
return column !== null
|
||||
? find(column)
|
||||
: document.querySelector<HTMLTextAreaElement>('textarea[data-phase]')
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -56,17 +103,7 @@ export function insertAtCaret(draft: string, text: string, caret: DraftCaret | n
|
||||
* hosts report null selectionStart/End).
|
||||
*/
|
||||
export function probeComposerCaret(draft: string): DraftCaret | null {
|
||||
if (typeof document === 'undefined') return null
|
||||
// The active composer lives in the conversation column; prefer its
|
||||
// `data-phase`-tagged textarea (the composer's marker), falling back to
|
||||
// any textarea in the column, then to a bare data-phase textarea (older
|
||||
// host layouts without the column attribute).
|
||||
const column = document.querySelector('#root [data-slot="conversation"]')
|
||||
const find = (scope: ParentNode): HTMLTextAreaElement | null =>
|
||||
scope.querySelector('textarea[data-phase]') ?? scope.querySelector('textarea')
|
||||
const el = column !== null
|
||||
? find(column)
|
||||
: document.querySelector<HTMLTextAreaElement>('textarea[data-phase]')
|
||||
const el = findComposerTextarea()
|
||||
if (el === null || el.disabled || el.readOnly) return null
|
||||
if (el.value !== draft) return null
|
||||
let start = el.selectionStart
|
||||
@@ -78,6 +115,43 @@ export function probeComposerCaret(draft: string): DraftCaret | null {
|
||||
return { start, end }
|
||||
}
|
||||
|
||||
/**
|
||||
* Restore the composer caret to `caretIndex` after a programmatic
|
||||
* `setDraft` commit. A controlled textarea update resets the caret (React
|
||||
* commits the value asynchronously and the browser moves the caret to the
|
||||
* start/end), so the placement is scheduled and retried across at most two
|
||||
* animation frames (setTimeout fallback for jsdom), and only applied when
|
||||
* the textarea still matches `expectedDraft` — a newer edit or a different
|
||||
* composer wins the race untouched. The caret is clamped into the value
|
||||
* bounds, mirroring how browsers clamp type-in positions.
|
||||
*/
|
||||
export function placeComposerCaretAfterInsert(expectedDraft: string, caretIndex: number): void {
|
||||
let remaining = 2
|
||||
let scheduled = false
|
||||
const schedule = (fn: () => void): void => {
|
||||
if (scheduled) return
|
||||
scheduled = true
|
||||
if (typeof requestAnimationFrame === 'function') requestAnimationFrame(fn)
|
||||
else setTimeout(fn, 0)
|
||||
}
|
||||
const place = (): void => {
|
||||
scheduled = false
|
||||
if (remaining <= 0) return
|
||||
remaining -= 1
|
||||
const el = findComposerTextarea()
|
||||
if (el === null || el.disabled || el.readOnly) return
|
||||
if (el.value !== expectedDraft) {
|
||||
// The commit has not landed yet (or a competing edit won) — one more
|
||||
// frame before giving up.
|
||||
schedule(place)
|
||||
return
|
||||
}
|
||||
const clamped = Math.max(0, Math.min(caretIndex, el.value.length))
|
||||
el.setSelectionRange(clamped, clamped)
|
||||
}
|
||||
schedule(place)
|
||||
}
|
||||
|
||||
/**
|
||||
* Insert `text` into the session's composer draft at the composer's live
|
||||
* caret (see {@link probeComposerCaret}), falling back to appending at the
|
||||
@@ -87,12 +161,24 @@ export function probeComposerCaret(draft: string): DraftCaret | null {
|
||||
export function appendToDraft(ctx: Context, sessionId: string, text: string): boolean {
|
||||
try {
|
||||
const actx = ctx.sessions.scope(sessionId)
|
||||
if (actx === undefined) return false
|
||||
if (actx === undefined) {
|
||||
console.warn('[dsh-better-sidebar] draft insert skipped: no session scope', sessionId)
|
||||
return false
|
||||
}
|
||||
const conversation = ctx.get('conversation') as SidebarConversation | undefined
|
||||
if (conversation === undefined) return false
|
||||
if (conversation === undefined) {
|
||||
console.warn('[dsh-better-sidebar] draft insert skipped: conversation service unavailable')
|
||||
return false
|
||||
}
|
||||
const input = conversation.input.for(actx)
|
||||
const draft = input.state.getSnapshot().draft
|
||||
input.setDraft(insertAtCaret(draft, text, probeComposerCaret(draft)))
|
||||
const caret = probeComposerCaret(draft)
|
||||
const { draft: next, caretAfter } = spliceInsert(draft, text, caret)
|
||||
input.setDraft(next)
|
||||
// Put the caret right after the inserted text once the value commit
|
||||
// lands — see the module doc for why this keeps stacked inserts at the
|
||||
// running position.
|
||||
placeComposerCaretAfterInsert(next, caretAfter)
|
||||
return true
|
||||
} catch (error) {
|
||||
console.warn('[dsh-better-sidebar] draft insert failed:', error)
|
||||
|
||||
@@ -3,11 +3,19 @@
|
||||
* issue #425 ②). `insertAtCaret` is pure string math (splice with
|
||||
* whitespace-aware joins); `probeComposerCaret` reads the live composer
|
||||
* `<textarea>` selection out of the DOM, guarded by a value-sync check so a
|
||||
* stale or wrong-composer caret is never applied.
|
||||
* stale or wrong-composer caret is never applied; `placeComposerCaretAfterInsert`
|
||||
* restores the caret right after the inserted text once the setDraft value
|
||||
* commit lands, keeping stacked inserts at their running position.
|
||||
*/
|
||||
// @vitest-environment jsdom
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { insertAtCaret, probeComposerCaret } from '../src/client/conversation-draft.ts'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { Context } from '../src/context-types.ts'
|
||||
import {
|
||||
appendToDraft,
|
||||
insertAtCaret,
|
||||
placeComposerCaretAfterInsert,
|
||||
probeComposerCaret,
|
||||
} from '../src/client/conversation-draft.ts'
|
||||
|
||||
/**
|
||||
* Mount a fake composer textarea inside the DSH conversation column
|
||||
@@ -32,6 +40,9 @@ afterEach(() => {
|
||||
document.body.innerHTML = ''
|
||||
})
|
||||
|
||||
/** Let the scheduled caret placement (rAF or setTimeout fallback) settle. */
|
||||
const tick = (): Promise<void> => new Promise((resolve) => { setTimeout(resolve, 30) })
|
||||
|
||||
describe('probeComposerCaret', () => {
|
||||
it('returns the textarea selection when the DOM is in sync with the draft', () => {
|
||||
mountComposer('hello world', 5, 5)
|
||||
@@ -119,4 +130,111 @@ describe('insertAtCaret', () => {
|
||||
it('handles an empty draft with a resolved caret', () => {
|
||||
expect(insertAtCaret('', 'CODE', { start: 0, end: 0 })).toBe('CODE')
|
||||
})
|
||||
})
|
||||
|
||||
describe('placeComposerCaretAfterInsert', () => {
|
||||
it('places the caret right after the inserted text once the value commits', async () => {
|
||||
const input = mountComposer('AB', 1, 1) // pre-insert draft still shown
|
||||
placeComposerCaretAfterInsert('A C B', 1 + 1)
|
||||
input.value = 'A C B' // the setDraft commit lands
|
||||
await tick()
|
||||
expect(input.selectionStart).toBe(2)
|
||||
expect(input.selectionEnd).toBe(2)
|
||||
})
|
||||
|
||||
it('places the caret when the value already matches (no commit needed)', async () => {
|
||||
const input = mountComposer('A C B', 0, 0)
|
||||
placeComposerCaretAfterInsert('A C B', 2)
|
||||
await tick()
|
||||
expect(input.selectionStart).toBe(2)
|
||||
})
|
||||
|
||||
it('clamps an out-of-range caret into the composer bounds', async () => {
|
||||
const input = mountComposer('A C B', 0, 0)
|
||||
placeComposerCaretAfterInsert('A C B', 99)
|
||||
await tick()
|
||||
expect(input.selectionStart).toBe(5)
|
||||
})
|
||||
|
||||
it('does not clobber a composer that never matches the expected draft', async () => {
|
||||
const input = mountComposer('OLD', 0, 0)
|
||||
input.value = 'UNRELATED' // a competing update wins the race
|
||||
input.setSelectionRange(1, 1)
|
||||
placeComposerCaretAfterInsert('NEW DRAFT', 2)
|
||||
await tick()
|
||||
expect(input.selectionStart).toBe(1)
|
||||
})
|
||||
|
||||
it('leaves the caret alone when no composer is mounted', async () => {
|
||||
placeComposerCaretAfterInsert('X', 0)
|
||||
await tick() // must not throw
|
||||
expect(document.querySelector('textarea')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('appendToDraft', () => {
|
||||
/** A fake ctx exposing the conversation service face appendToDraft uses. */
|
||||
function fakeCtx(
|
||||
getDraft: () => string,
|
||||
onSet: (next: string) => void,
|
||||
withConversation = true,
|
||||
): Context {
|
||||
const input = {
|
||||
state: { getSnapshot: (): { draft: string } => ({ draft: getDraft() }) },
|
||||
setDraft: (next: string): void => { onSet(next) },
|
||||
}
|
||||
return {
|
||||
sessions: { scope: (): unknown => ({}) },
|
||||
get: (name: string): unknown =>
|
||||
withConversation && name === 'conversation'
|
||||
? { input: { for: (): unknown => input } }
|
||||
: undefined,
|
||||
} as unknown as Context
|
||||
}
|
||||
|
||||
it('splices at the probed caret and restores the caret right after the insert', async () => {
|
||||
const composer = mountComposer('AB', 1, 1)
|
||||
const calls: string[] = []
|
||||
const ctx = fakeCtx(() => 'AB', (next) => { calls.push(next); composer.value = next })
|
||||
expect(appendToDraft(ctx, 's1', 'C')).toBe(true)
|
||||
expect(calls).toEqual(['A C B'])
|
||||
await tick()
|
||||
// 'A C| B' — the caret sits right after the inserted text (the left
|
||||
// separating space shifts it past `start + text.length`).
|
||||
expect(composer.selectionStart).toBe(3)
|
||||
expect(composer.selectionEnd).toBe(3)
|
||||
})
|
||||
|
||||
it('appends and places the caret at the end when the caret is unknown', async () => {
|
||||
const composer = mountComposer('hi', 0, 0)
|
||||
composer.value = 'hi changed' // out of sync → caret unresolved → append
|
||||
const calls: string[] = []
|
||||
const ctx = fakeCtx(() => 'hi', (next) => { calls.push(next); composer.value = next })
|
||||
appendToDraft(ctx, 's1', 'X')
|
||||
expect(calls).toEqual(['hi X'])
|
||||
await tick()
|
||||
expect(composer.selectionStart).toBe(4)
|
||||
})
|
||||
|
||||
it('keeps stacked inserts at the running caret (A|B + C + D → ACD|B)', async () => {
|
||||
const composer = mountComposer('AB', 1, 1)
|
||||
let draft = 'AB'
|
||||
const ctx = fakeCtx(() => draft, (next) => { draft = next; composer.value = next })
|
||||
appendToDraft(ctx, 's1', 'C')
|
||||
await tick()
|
||||
expect(draft).toBe('A C B')
|
||||
expect(composer.selectionStart).toBe(3) // A C| B
|
||||
appendToDraft(ctx, 's1', 'D')
|
||||
await tick()
|
||||
expect(draft).toBe('A C D B')
|
||||
expect(composer.selectionStart).toBe(5) // A C D| B
|
||||
})
|
||||
|
||||
it('returns false and logs when the conversation service is unavailable', () => {
|
||||
const consoleWarn = vi.spyOn(console, 'warn').mockImplementation(() => undefined)
|
||||
const ctx = fakeCtx(() => '', (): void => undefined, false)
|
||||
expect(appendToDraft(ctx, 's1', 'X')).toBe(false)
|
||||
expect(consoleWarn).toHaveBeenCalledTimes(1)
|
||||
consoleWarn.mockRestore()
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user