diff --git a/cypress/e2e/keyFormatEditor.cy.ts b/cypress/e2e/keyFormatEditor.cy.ts new file mode 100644 index 00000000..158f5f4c --- /dev/null +++ b/cypress/e2e/keyFormatEditor.cy.ts @@ -0,0 +1,104 @@ +import { DEFAULT_CREDENTIALS } from "@/web/urlConfig"; +import { EditorView } from "@codemirror/view"; +import { visitWithState } from "../common/tools"; + +function openEditor() { + visitWithState({ + config: { + apiUrl: DEFAULT_CREDENTIALS.apiUrl, + apiKey: DEFAULT_CREDENTIALS.apiKey, + language: "en", + namespace: "", + documentInfo: true, + pageInfo: true, + prefillKeyFormat: true, + keyFormat: "", + }, + }); + + cy.iframeBody() + .find('[data-cy="index_settings_button"]') + .should("be.visible") + .click(); + cy.iframeBody() + .find('[data-cy="settings_expandable_strings"]') + .should("be.visible") + .click(); + cy.iframeBody() + .find('[data-cy="global-editor"]') + .should("be.visible") + .click(); + cy.iframeBody().find(".cm-content").as("editorContent"); + cy.iframeBody().find(".cm-tooltip-autocomplete").should("be.visible"); +} + +function getEditorView(): Cypress.Chainable { + return cy.get("@editorContent").then(($content) => { + const view = EditorView.findFromDOM($content[0]); + expect(view).not.to.be.null; + return view!; + }); +} + +function acceptSelectedCompletion(): Cypress.Chainable { + cy.wait(200); + return getEditorView() + .then((view) => { + const frameWindow = view.contentDOM.ownerDocument.defaultView; + expect(frameWindow).not.to.be.null; + view.contentDOM.dispatchEvent( + new frameWindow!.KeyboardEvent("keydown", { + key: "Enter", + code: "Enter", + bubbles: true, + cancelable: true, + }), + ); + return view.state.selection.main.assoc; + }) + .then((assoc) => + cy + .iframeBody() + .find(".cm-tooltip-autocomplete") + .should("not.exist") + .then(() => assoc), + ); +} + +describe("Key format editor", () => { + it("preserves cursor association after completion", () => { + openEditor(); + acceptSelectedCompletion().should("equal", -1); + }); + + it("opens completion when a placeholder badge is clicked", () => { + openEditor(); + acceptSelectedCompletion(); + + cy.iframeBody().find(".placeholder-widget").click(); + cy.iframeBody().find(".cm-tooltip-autocomplete").should("be.visible"); + }); + + it("keeps separators between inserted placeholders", () => { + openEditor(); + acceptSelectedCompletion(); + + getEditorView().then((view) => { + const position = view.state.selection.main.head; + const insert = ".component"; + view.dispatch({ + changes: { from: position, insert }, + selection: { anchor: position + insert.length }, + userEvent: "input.type", + }); + }); + cy.iframeBody().find(".cm-tooltip-autocomplete").should("be.visible"); + acceptSelectedCompletion(); + + getEditorView() + .its("state.doc") + .invoke("toString") + .should("equal", "{artboard}.{component}"); + cy.get("@editorContent").should("have.text", "artboard.component"); + }); +}); diff --git a/docs/superpowers/plans/2026-07-10-key-format-cursor.md b/docs/superpowers/plans/2026-07-10-key-format-cursor.md new file mode 100644 index 00000000..f84ba1eb --- /dev/null +++ b/docs/superpowers/plans/2026-07-10-key-format-cursor.md @@ -0,0 +1,204 @@ +# Key Format Cursor Fix Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Keep separators and the cursor at the intended position when users combine multiple placeholders in the key-format editor. + +**Architecture:** Use CodeMirror's completion lifecycle and content click handler +instead of restarting completion from bubbled events. The visual-caret follow-up +customizes only the completion transaction's cursor association so CodeMirror +draws the caret on the logical side of an atomic placeholder. + +**Tech Stack:** TypeScript, Preact, CodeMirror 6, Cypress, Bun + +## Global Constraints + +- Keep click-to-open completion. +- Do not change placeholder names, formatting, preview generation, or persistence. +- Add no dependencies or new abstractions. +- Use Bun for project commands. +- Do not add or run Cypress coverage for the visual-caret follow-up; the reporter + will verify it manually. + +--- + +### Task 1: Preserve the cursor between key-format placeholders + +**Files:** + +- Create: `cypress/e2e/keyFormatEditor.cy.ts` +- Modify: `src/ui/views/Settings/StringsEditor.tsx:93-108,163-193,227-242` + +**Interfaces:** + +- Consumes: CodeMirror `Completion.apply`, `EditorView.domEventHandlers`, and the existing `StringsEditor` props. +- Produces: unchanged `StringsEditor` props and unchanged key-format strings through `onChange`. + +- [x] **Step 1: Write the failing Cypress regression** + +```typescript +import { DEFAULT_CREDENTIALS } from "@/web/urlConfig"; +import { visitWithState } from "../common/tools"; + +describe("Key format editor", () => { + it("keeps separators between inserted placeholders", () => { + visitWithState({ + config: { + apiUrl: DEFAULT_CREDENTIALS.apiUrl, + apiKey: DEFAULT_CREDENTIALS.apiKey, + language: "en", + namespace: "", + documentInfo: true, + pageInfo: true, + prefillKeyFormat: true, + keyFormat: "", + }, + }); + + cy.iframe().findDcy("index_settings_button").click(); + cy.iframe().findDcy("settings_expandable_strings").click(); + cy.iframe().findDcy("global-editor").click(); + cy.iframe().find(".cm-content").as("editorContent"); + cy.iframe().find(".cm-tooltip-autocomplete").should("be.visible"); + cy.wait(150); + + cy.get("@editorContent").type("{enter}").should("have.text", "artboard"); + cy.iframe().find(".cm-tooltip-autocomplete").should("not.exist"); + cy.get("@editorContent").type(".component"); + cy.iframe().find(".cm-tooltip-autocomplete").should("be.visible"); + cy.wait(150); + cy.get("@editorContent") + .type("{enter}") + .should("have.text", "artboard.component"); + cy.iframe().find(".cm-tooltip-autocomplete").should("not.exist"); + }); +}); +``` + +- [x] **Step 2: Run the focused test and verify RED** + +Run: + +```bash +bunx cypress run --spec cypress/e2e/keyFormatEditor.cy.ts +``` + +Expected: FAIL because `.cm-tooltip-autocomplete` still exists after accepting the first placeholder, demonstrating that completion was restarted by the wrapper event. + +- [x] **Step 3: Delegate insertion and click handling to CodeMirror** + +Replace `createCompletionOption` with CodeMirror's string `apply` contract: + +```typescript +function createCompletionOption( + label: string, + detail: string, + insertText: string, +) { + return { + label, + detail, + type: "keyword" as const, + apply: insertText, + }; +} +``` + +Add a content-scoped click handler to the editor extensions: + +```typescript +EditorView.domEventHandlers({ + click(_event, view) { + startCompletion(view); + return false; + }, +}), +``` + +Remove the Preact `onClick` and `onKeyDown` props from the editor wrapper. This prevents completion-option clicks and ordinary key presses from starting a stale explicit completion. + +- [x] **Step 4: Run focused and static verification** + +Run: + +```bash +bunx cypress run --spec cypress/e2e/keyFormatEditor.cy.ts +bun run tsc +bun run build +``` + +Expected: the Cypress spec passes, TypeScript reports no errors, and the plugin build completes successfully. + +- [x] **Step 5: Commit the bug fix** + +```bash +git add cypress/e2e/keyFormatEditor.cy.ts src/ui/views/Settings/StringsEditor.tsx docs/superpowers/plans/2026-07-10-key-format-cursor.md +git commit -m "fix(settings): preserve key format cursor position" +``` + +### Task 2: Render the caret on the logical side of a placeholder + +**Files:** + +- Modify: `src/ui/views/Settings/StringsEditor.tsx:3,16-21,93-106` +- Modify: `docs/superpowers/specs/2026-07-10-key-format-cursor-design.md:37-53` + +**Interfaces:** + +- Consumes: CodeMirror `Completion`, `EditorSelection.cursor`, + `pickedCompletion`, and the existing placeholder completion range. +- Produces: the existing completion option shape with a custom `apply` function + that places the cursor at `from + insertText.length` with `assoc: -1`. + +- [x] **Step 1: Apply the completion with an explicit cursor association** + +Import the selection and completion APIs: + +```typescript +import { Compartment, EditorSelection, EditorState } from "@codemirror/state"; +import { + autocompletion, + Completion, + CompletionContext, + CompletionResult, + pickedCompletion, + startCompletion, +} from "@codemirror/autocomplete"; +``` + +Replace the string `apply` value in `createCompletionOption` with a typed +function that preserves CodeMirror's completion annotation: + +```typescript +apply(view: EditorView, completion: Completion, from: number, to: number) { + view.dispatch({ + changes: { from, to, insert: insertText }, + selection: EditorSelection.cursor(from + insertText.length, -1), + annotations: pickedCompletion.of(completion), + scrollIntoView: true, + userEvent: "input.complete", + }); +}, +``` + +- [x] **Step 2: Run static verification** + +Run: + +```bash +bun run tsc +bun run build +``` + +Expected: TypeScript reports no errors and the plugin build completes +successfully. Do not run Cypress; the reporter will verify left/right caret +rendering in Figma. + +- [x] **Step 3: Commit the follow-up fix** + +```bash +git add src/ui/views/Settings/StringsEditor.tsx \ + docs/superpowers/specs/2026-07-10-key-format-cursor-design.md \ + docs/superpowers/plans/2026-07-10-key-format-cursor.md +git commit -m "fix(settings): render cursor after placeholder badge" +``` diff --git a/docs/superpowers/specs/2026-07-10-key-format-cursor-design.md b/docs/superpowers/specs/2026-07-10-key-format-cursor-design.md new file mode 100644 index 00000000..4cfdce74 --- /dev/null +++ b/docs/superpowers/specs/2026-07-10-key-format-cursor-design.md @@ -0,0 +1,54 @@ +# Key format cursor fix + +## Problem + +The key-format editor starts completion on every `keydown`, before CodeMirror +has applied the typed character. Its custom completion transaction also bypasses +CodeMirror's built-in completion lifecycle. After inserting a placeholder, the +next separator or placeholder can therefore use a stale completion range and +move the cursor unexpectedly. + +After accepting a placeholder, the logical cursor position is correct, but the +caret is rendered on the left side of the placeholder badge. The same visual +position is shown when the cursor is logically before or after the badge. + +## Design + +- Keep click-to-open completion. +- Remove the editor wrapper's `onKeyDown` completion trigger. CodeMirror already + activates completion for normal typing. +- Represent each placeholder's `apply` value as the inserted string so CodeMirror + owns the document change, selection mapping, and completion closing. +- Do not change placeholder names, formatting, preview generation, or persistence. + +### Placeholder caret association + +The placeholder is rendered as a CodeMirror widget with `side: 1`. CodeMirror +uses a cursor range's `assoc` value to choose the visual side at an ambiguous +widget boundary. A completion currently leaves `assoc` at its default value, so +the caret is drawn on the badge's left edge even though the cursor is logically +after it. + +Apply placeholder completions with a cursor range whose `assoc` is `-1`. This +associates the cursor with the preceding content and renders it on the right +edge of the badge. Preserve CodeMirror's `pickedCompletion` annotation so its +completion lifecycle and events continue to behave normally. + +Arrow-key movement remains owned by CodeMirror. Consider a local cursor overlay +only if manual verification shows that CodeMirror does not preserve the correct +association while crossing the atomic placeholder. + +## Verification + +Add a Cypress regression that opens the key-format editor, inserts a placeholder, +types `.` at the cursor, inserts another placeholder, and verifies the separator +remains between the two placeholders. Run the focused test first to prove the +current implementation fails, then run it again after the fix together with the +TypeScript and build checks. + +The reporter will manually verify the primary caret's position relative to the +placeholder badge; do not add or run Cypress coverage for this follow-up: + +- directly after accepting a placeholder, the caret is at the badge's right edge; +- after moving left across the placeholder, the caret is at its left edge; +- after moving right across the placeholder, the caret is again at its right edge. diff --git a/src/ui/views/Settings/StringsEditor.tsx b/src/ui/views/Settings/StringsEditor.tsx index becc3610..9c198ecf 100644 --- a/src/ui/views/Settings/StringsEditor.tsx +++ b/src/ui/views/Settings/StringsEditor.tsx @@ -1,23 +1,99 @@ import { h, RefObject } from "preact"; import { useEffect, useRef } from "preact/hooks"; import { minimalSetup } from "codemirror"; -import { Compartment, EditorState } from "@codemirror/state"; +import { + EditorSelection, + EditorState, + RangeSetBuilder, + StateField, +} from "@codemirror/state"; import { ViewUpdate, EditorView, KeyBinding, Decoration, + DecorationSet, WidgetType, } from "@codemirror/view"; -import { PlaceholderPlugin } from "@tginternal/editor"; +import { getPlaceholders, Placeholder } from "@tginternal/editor"; import "!./StringsEditor.css"; import { autocompletion, + Completion, CompletionContext, CompletionResult, + pickedCompletion, startCompletion, } from "@codemirror/autocomplete"; +class PlaceholderBadgeWidget extends WidgetType { + constructor(private value: Placeholder) { + super(); + } + + eq(other: PlaceholderBadgeWidget) { + return ( + other.value.name === this.value.name && + other.value.type === this.value.type && + other.value.error === this.value.error + ); + } + + toDOM() { + const outer = document.createElement("span"); + let classes = `placeholder-widget placeholder-${this.value.type}`; + if (this.value.error) { + classes += ` placeholder-error placeholder-error-${this.value.error}`; + } + outer.className = classes; + const inner = document.createElement("span"); + inner.textContent = this.value.name; + outer.appendChild(inner); + return outer; + } + + ignoreEvent() { + return false; + } +} + +// Renders {placeholder} ranges as badges. Replaces PlaceholderPlugin from +// @tginternal/editor, whose Decoration.widget({side: 1}) ranges place both +// boundary cursor positions inside the widget, so the caret is always drawn +// at the badge's left edge. Non-inclusive Decoration.replace keeps the +// positions before/after the badge, so the caret renders on the correct side. +function placeholderBadges() { + const build = (state: EditorState) => { + const builder = new RangeSetBuilder(); + let placeholders: Placeholder[] | null = []; + try { + placeholders = getPlaceholders(state.doc.toString(), true); + } catch { + placeholders = []; + } + for (const placeholder of placeholders ?? []) { + builder.add( + placeholder.position.start, + placeholder.position.end, + Decoration.replace({ + widget: new PlaceholderBadgeWidget(placeholder), + }), + ); + } + return builder.finish(); + }; + const field = StateField.define({ + create: build, + update: (set, transaction) => + transaction.docChanged ? build(transaction.state) : set, + provide: (f) => [ + EditorView.decorations.from(f), + EditorView.atomicRanges.of((view) => view.state.field(f)), + ], + }); + return field; +} + // Custom placeholder widget class PlaceholderWidget extends WidgetType { constructor(private placeholder: string) { @@ -83,7 +159,6 @@ export const StringsEditor = ({ }: EditorProps) => { const ref = useRef(null); const editor = useRef(); - const placeholders = useRef(new Compartment()); const callbacksRef = useRefGroup({ onChange, onFocus, @@ -99,11 +174,20 @@ export const StringsEditor = ({ label, detail, type: "keyword" as const, - apply(view: EditorView, _: any, from: number, to: number) { - const insert = insertText; + apply( + view: EditorView, + completion: Completion, + from: number, + to: number, + ) { view.dispatch({ - changes: { from, to, insert }, - selection: { anchor: from + insert.length }, + changes: { from, to, insert: insertText }, + selection: EditorSelection.create([ + EditorSelection.cursor(from + insertText.length, -1), + ]), + annotations: pickedCompletion.of(completion), + scrollIntoView: true, + userEvent: "input.complete", }); }, }; @@ -174,13 +258,13 @@ export const StringsEditor = ({ callbacksRef.current?.onChange?.(v.state.doc.toString()); } }), - placeholders.current.of( - PlaceholderPlugin({ - examplePluralNum: 1, - nested: true, - tooltips: false, - }), - ), + EditorView.domEventHandlers({ + click(_event, view) { + startCompletion(view); + return false; + }, + }), + placeholderBadges(), autocompletion({ override: [completions], optionClass: () => { @@ -227,12 +311,6 @@ export const StringsEditor = ({ return (
{ - startCompletion(editor.current!); - }} - onKeyDown={() => { - startCompletion(editor.current!); - }} class="strings-editor" data-cy="global-editor" ref={ref}