From 0b264a482e3104ae1591fd74a1d4a7666053054c Mon Sep 17 00:00:00 2001 From: hayyaksi <193020925+hayaksi1@users.noreply.github.com> Date: Mon, 10 Aug 2026 17:01:38 +0300 Subject: [PATCH] Reverse a confirmed autocomplete with a single undo (#34635) Confirming a completion produces two model updates for one keystroke: the completion replaces the range and closes the autocomplete, and then the wrapper closes it again. The model reported that second close like any other change, and since it carries no input type and no diff the history manager treats it as bulk input and always pushes, leaving two identical states on the undo stack. The model now ignores a close which has nothing to replace and arrives once the autocomplete is already gone. Dismissing with escape, and the fallback close when there was no selection to confirm, both happen while it is still open and are unaffected. Tests: a single undo restores the typed text after a completion, and the redundant close causes no further render. --- apps/web/src/editor/model.test.ts | 38 +++++++++++++++++++++++++++++++ apps/web/src/editor/model.ts | 6 +++++ 2 files changed, 44 insertions(+) diff --git a/apps/web/src/editor/model.test.ts b/apps/web/src/editor/model.test.ts index 2976a81e84..f391d8d80e 100644 --- a/apps/web/src/editor/model.test.ts +++ b/apps/web/src/editor/model.test.ts @@ -9,6 +9,7 @@ Please see LICENSE files in the repository root for full details. import { describe, it, expect } from "vitest"; import EditorModel from "./model"; +import HistoryManager from "./history"; import { createPartCreator, createRenderer, type MockAutoComplete } from "./__mocks__"; import DocumentOffset from "./offset"; import { type PillPart } from "./parts"; @@ -192,6 +193,43 @@ describe("editor/model", function () { expect(model.parts[1].text).toBe("Alice"); }); + it("reverses a confirmed completion with a single undo", function () { + const history = new HistoryManager(); + const pc = createPartCreator([{ resourceId: "@alice", text: "Alice" } as PillPart]); + // Stands in for the composer, which pushes every update it is told about onto the + // undo stack. + const model: EditorModel = new EditorModel([pc.plain("hello ")], pc, (caret) => + history.tryPush(model, caret), + ); + + model.update("hello @a", "insertText", new DocumentOffset(8, true)); + history.ensureLastChangesPushed(model); + const autoComplete = model.autoComplete as unknown as MockAutoComplete; + autoComplete.tryComplete(); + // Confirming a completion closes the autocomplete a second time, which is what used to + // put an identical second state on the undo stack. + autoComplete.close(); + + const undone = history.undo(model); + expect(undone).toBeTruthy(); + expect(undone!.parts.map((p) => p.text)).toEqual(["hello ", "@a"]); + }); + + it("does not report the redundant close which follows a completion", function () { + const renderer = createRenderer(); + const pc = createPartCreator([{ resourceId: "@alice", text: "Alice" } as PillPart]); + const model = new EditorModel([pc.plain("hello ")], pc, renderer); + + model.update("hello @a", "insertText", new DocumentOffset(8, true)); + const autoComplete = model.autoComplete as unknown as MockAutoComplete; + autoComplete.tryComplete(); + expect(renderer.count).toBe(2); + + autoComplete.close(); + + expect(renderer.count).toBe(2); + }); + it("insert room pill", function () { const renderer = createRenderer(); const pc = createPartCreator([{ resourceId: "#riot-dev" } as PillPart]); diff --git a/apps/web/src/editor/model.ts b/apps/web/src/editor/model.ts index 44b5064d81..b66d58c8c9 100644 --- a/apps/web/src/editor/model.ts +++ b/apps/web/src/editor/model.ts @@ -262,6 +262,12 @@ export default class EditorModel { } private onAutoComplete = ({ replaceParts, close, range }: ICallback): void => { + // Confirming a completion closes the autocomplete twice: once as part of the completion and + // once more by the wrapper afterwards. Reporting the second one would have the history + // manager record a second, identical state, costing the user an extra undo to reverse one + // completion. + if (!replaceParts && close && !this._autoComplete) return; + let pos: DocumentPosition | undefined; if (replaceParts) { const autoCompletePartIdx = this.autoCompletePartIdx || 0;