From 2d14325a034d0347afbb4c1871dc6b956c18c942 Mon Sep 17 00:00:00 2001 From: Amp Date: Sat, 26 Sep 2026 20:45:20 +0000 Subject: [PATCH] fix(builders): open every choice builder with focus on Done Obsidian's Modal.open() focuses the first focusable element. The Macro builder builds its content before open(), so its title rename button got focus and a focus ring; the Template/Capture forms mount after open(), so their footer Done got it. addAutosaveFooter now focuses Done and both builder hosts call it after open(). Amp-Thread-ID: https://ampcode.com/threads/T-01a0df6a-959f-74bb-a96a-d523ad2e20e9 Co-authored-by: Christian Bager Bach Houmann --- .../ChoiceBuilder/builderInitialFocus.test.ts | 56 +++++++++++++++++++ src/gui/ChoiceBuilder/choiceBuilder.ts | 3 +- .../components/autosaveFooter.ts | 7 +++ src/gui/MacroGUIs/MacroBuilder.ts | 3 +- 4 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 src/gui/ChoiceBuilder/builderInitialFocus.test.ts diff --git a/src/gui/ChoiceBuilder/builderInitialFocus.test.ts b/src/gui/ChoiceBuilder/builderInitialFocus.test.ts new file mode 100644 index 000000000..2f88f5659 --- /dev/null +++ b/src/gui/ChoiceBuilder/builderInitialFocus.test.ts @@ -0,0 +1,56 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// FormatPreviewField -> formatter graph pulls obsidian-dataview's CJS require. +vi.mock("obsidian-dataview", () => ({ getAPI: vi.fn() })); + +import { App, Modal } from "obsidian"; +import type QuickAdd from "../../main"; +import { CaptureChoice } from "../../types/choices/CaptureChoice"; +import { MacroChoice } from "../../types/choices/MacroChoice"; +import { TemplateChoice } from "../../types/choices/TemplateChoice"; +import { MacroBuilder } from "../MacroGUIs/MacroBuilder"; +import { CaptureChoiceBuilder } from "./captureChoiceBuilder"; +import { TemplateChoiceBuilder } from "./templateChoiceBuilder"; + +const plugin = { + getTemplateFiles: () => [], + settings: { choices: [] }, +} as unknown as QuickAdd; + +// The builders opened with focus on different elements: the Macro builder's +// title rename button (a focus ring around the title), the Template/Capture +// builders' footer Done. Cause: Obsidian's Modal.open() focuses the first +// focusable element in modalEl, and only the Macro builder builds its content +// before open(). The stub's open() does not autofocus, so emulate it here; +// otherwise a fix that focuses Done before open() would pass the test but lose +// to open() in Obsidian. +describe("choice builder initial focus", () => { + beforeEach(() => { + const open = Modal.prototype.open; + vi.spyOn(Modal.prototype, "open").mockImplementation(function ( + this: InstanceType, + ) { + open.call(this); + this.modalEl + .querySelector("button, input, select, textarea, [tabindex]") + ?.focus(); + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + document.body.replaceChildren(); + }); + + it.each([ + ["Macro", () => new MacroBuilder(new App(), plugin, new MacroChoice("Macro"), [])], + ["Template", () => new TemplateChoiceBuilder(new App(), new TemplateChoice("Template"), plugin)], + ["Capture", () => new CaptureChoiceBuilder(new App(), new CaptureChoice("Capture"), plugin)], + ])("%s builder opens with focus on the footer's Done", (_type, openBuilder) => { + const modal = openBuilder(); + + const done = modal.modalEl.querySelector(".qa-builder-footer button.mod-cta"); + expect(done?.textContent).toBe("Done"); + expect(document.activeElement).toBe(done); + }); +}); diff --git a/src/gui/ChoiceBuilder/choiceBuilder.ts b/src/gui/ChoiceBuilder/choiceBuilder.ts index d31ee5829..b0b6457cb 100644 --- a/src/gui/ChoiceBuilder/choiceBuilder.ts +++ b/src/gui/ChoiceBuilder/choiceBuilder.ts @@ -29,10 +29,11 @@ export abstract class ChoiceBuilder extends Modal { }); this.containerEl.addClass("quickAddModal"); + this.open(); // Installed here, not in display(): display() runs from the subclass // constructor and is re-run by builders that rebuild their content. + // After open() so the footer's Done keeps the initial focus. addAutosaveFooter(this, "choice"); - this.open(); } /** diff --git a/src/gui/ChoiceBuilder/components/autosaveFooter.ts b/src/gui/ChoiceBuilder/components/autosaveFooter.ts index fc11eebc0..09db0a382 100644 --- a/src/gui/ChoiceBuilder/components/autosaveFooter.ts +++ b/src/gui/ChoiceBuilder/components/autosaveFooter.ts @@ -18,6 +18,12 @@ import type { Modal } from "obsidian"; * content cannot drop it — MacroBuilder.reload() empties contentEl. Idempotent * for the same reason. * + * Also gives Done the initial focus, so every builder opens the same way. Call + * it AFTER `modal.open()`: open() autofocuses the first focusable element in the + * modal, which was the title's rename button in the Macro builder (built before + * open) but Done in the Template/Capture builders (their forms mount after open). + * Done is the safe target: Enter only closes, and closing saves. + * * @param subject What the modal edits, e.g. "choice" or "macro". */ export function addAutosaveFooter(modal: Modal, subject: string): void { @@ -33,4 +39,5 @@ export function addAutosaveFooter(modal: Modal, subject: string): void { const done = footer.createEl("button", { type: "button", cls: "mod-cta", text: "Done" }); done.addEventListener("click", () => modal.close()); + done.focus(); } diff --git a/src/gui/MacroGUIs/MacroBuilder.ts b/src/gui/MacroGUIs/MacroBuilder.ts index fe503ea6e..c574d1710 100644 --- a/src/gui/MacroGUIs/MacroBuilder.ts +++ b/src/gui/MacroGUIs/MacroBuilder.ts @@ -94,10 +94,11 @@ export class MacroBuilder extends Modal { ); this.display(); + this.open(); // Installed here, not in display(): reload() re-runs display(), which // empties contentEl. The footer lives on modalEl and survives that. + // After open() so the footer's Done, not the title, gets initial focus. addAutosaveFooter(this, "macro"); - this.open(); } onClose() {