feat(a11y): keyboard & ARIA pass on the choice and macro GUIs (#1250)

eslint.config.mjs

@@ -61,6 +61,15 @@ export default [
             // (new Set/Map -> assign), which is reactive under $state. SvelteSet/SvelteMap
             // are only needed for in-place mutation, so this rule is a false positive here.
             'svelte/prefer-svelte-reactivity': 'off',
+
+            // eslint-plugin-svelte v3 ships no dedicated a11y-* rules; the Svelte 5
+            // compiler emits the a11y_* warnings instead. valid-compile surfaces those
+            // (plus other compiler warnings, e.g. css_unused_selector) as lint errors,
+            // so a bare interactive <div>/<span> can't be reintroduced without CI
+            // catching it. Baseline is clean after the #1250 a11y pass. Note: this is a
+            // regression ratchet — it does NOT police accessible names on icon-only
+            // buttons or keyboard-operable drag handles, which are covered by tests.
+            'svelte/valid-compile': 'error',
         },
     },
     // Special rules for main.ts to preserve critical import order

src/gui/ChoiceBuilder/FolderList.svelte

@@ -1,5 +1,5 @@
 <script lang="ts">
-    import ObsidianIcon from "../components/ObsidianIcon.svelte";
+    import IconButton from "../components/IconButton.svelte";
     import type { FolderListProps } from "./folderListProps.svelte";
 
     let { folders, deleteFolder }: FolderListProps = $props();
@@ -9,15 +9,11 @@
     {#each folders as folder (folder)}
         <div class="quickAddCommandListItem">
             <span>{folder}</span>
-            <span
-                role="button"
-                tabindex="0"
+            <IconButton
+                iconId="trash-2"
+                label={`Remove folder ${folder}`}
                 onclick={() => deleteFolder(folder)}
-                onkeypress={(e) => (e.key === 'Enter' || e.key === ' ') && deleteFolder(folder)}
-                class="clickable"
-            >
-                <ObsidianIcon iconId="trash-2" size={16} />
-            </span>
+            />
         </div>
     {/each}
 </div>
@@ -41,9 +37,4 @@
     max-width: 50%;
     margin: 12px auto;
 }
-
-
-.clickable {
-    cursor: pointer;
-}
-</style>
+</style>

src/gui/ChoiceBuilder/choiceBuilder.ts

@@ -99,21 +99,28 @@ export abstract class ChoiceBuilder extends Modal {
 		const headerEl: HTMLHeadingElement = this.contentEl.createEl("h2", {
 			cls: "choiceNameHeader",
 		});
-		const textEl = headerEl.createSpan({
+		// Rename affordance is a real <button> (keyboard operable: Enter/Space) inside
+		// the heading, so the <h2> keeps its heading role for screen readers (#1250).
+		const button = headerEl.createEl("button", {
+			cls: "choiceNameHeaderButton qa-rename-title-button",
+			attr: { type: "button", "aria-label": `Rename ${choice.name}` },
+		});
+		const textEl = button.createSpan({
 			text: choice.name,
 			cls: "choiceNameHeaderText",
 		});
-		const iconEl = headerEl.createSpan({
+		const iconEl = button.createSpan({
 			cls: "choiceNameHeaderIcon",
 			attr: { "aria-hidden": "true" },
 		});
 		setIcon(iconEl, "pencil");
 
-		headerEl.addEventListener("click", async (ev) => {
+		button.addEventListener("click", async () => {
 			const newName = await promptRenameChoice(this.app, choice.name);
 			if (!newName) return;
 			choice.name = newName;
 			textEl.setText(newName);
+			button.setAttribute("aria-label", `Rename ${newName}`);
 		});
 	}
 

src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts

@@ -52,13 +52,18 @@ export class AIAssistantCommandSettingsModal extends Modal {
 	}
 
 	private display(): void {
-		const header = this.contentEl.createEl("h2", {
+		const header = this.contentEl.createEl("h2");
+		header.addClass("qa-clickable-modal-title");
+
+		// Rename affordance is a real <button> (keyboard operable: Enter/Space) inside
+		// the heading, so the <h2> keeps its heading role for screen readers (#1250).
+		const renameButton = header.createEl("button", {
+			cls: "qa-rename-title-button",
 			text: `${this.settings.name} Settings`,
+			attr: { type: "button", "aria-label": `Rename ${this.settings.name}` },
 		});
 
-		header.addClass("qa-clickable-modal-title");
-
-		header.addEventListener("click", async () => {
+		renameButton.addEventListener("click", async () => {
 			try {
 				const newName = await GenericInputPrompt.Prompt(
 					this.app,

src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.ts

@@ -47,13 +47,18 @@ export class InfiniteAIAssistantCommandSettingsModal extends Modal {
 
 	private display(): void {
 		this.contentEl.empty();
-		const header = this.contentEl.createEl("h2", {
+		const header = this.contentEl.createEl("h2");
+		header.addClass("qa-clickable-modal-title");
+
+		// Rename affordance is a real <button> (keyboard operable: Enter/Space) inside
+		// the heading, so the <h2> keeps its heading role for screen readers (#1250).
+		const renameButton = header.createEl("button", {
+			cls: "qa-rename-title-button",
 			text: `${this.settings.name} Settings`,
+			attr: { type: "button", "aria-label": `Rename ${this.settings.name}` },
 		});
 
-		header.addClass("qa-clickable-modal-title");
-
-		header.addEventListener("click", async () => {
+		renameButton.addEventListener("click", async () => {
 			try {
 				const newName = await GenericInputPrompt.Prompt(
 					this.app,

src/gui/MacroGUIs/CommandList.conditional.test.ts

@@ -31,7 +31,11 @@ describe("CommandList conditional branch persistence", () => {
 		});
 
 		const { getByLabelText } = render(CommandList, { props });
-		await fireEvent.click(getByLabelText("Edit then branch"));
+		// Edit labels include the condition summary (a11y); a default condition
+		// summarizes as "(missing variable) is truthy".
+		await fireEvent.click(
+			getByLabelText("Edit then branch for (missing variable) is truthy"),
+		);
 
 		await vi.waitFor(() => expect(saveCommands).toHaveBeenCalledTimes(1));
 		const saved = saveCommands.mock.calls[0][0] as Array<{ thenCommands?: unknown[] }>;
@@ -54,7 +58,11 @@ describe("CommandList conditional branch persistence", () => {
 		});
 
 		const { getByLabelText } = render(CommandList, { props });
-		await fireEvent.click(getByLabelText("Edit then branch"));
+		// Edit labels include the condition summary (a11y); a default condition
+		// summarizes as "(missing variable) is truthy".
+		await fireEvent.click(
+			getByLabelText("Edit then branch for (missing variable) is truthy"),
+		);
 		await Promise.resolve();
 
 		expect(saveCommands).not.toHaveBeenCalled();

src/gui/MacroGUIs/CommandList.keyboardReorder.test.ts

@@ -0,0 +1,75 @@
+import { describe, expect, it, vi } from "vitest";
+import { fireEvent, render } from "@testing-library/svelte";
+
+// CommandList transitively imports src/main, which pulls obsidian-dataview's CJS
+// require('obsidian'); mock it as the rest of the suite does.
+vi.mock("obsidian-dataview", () => ({ getAPI: vi.fn() }));
+
+import { App } from "obsidian";
+import CommandList from "./CommandList.svelte";
+import { createCommandListProps } from "./commandListProps.svelte";
+import { ObsidianCommand } from "../../types/macros/ObsidianCommand";
+import type { ICommand } from "../../types/macros/ICommand";
+
+const makeProps = (commands: ICommand[], saveCommands = vi.fn()) =>
+	createCommandListProps({
+		commands,
+		app: new App() as never,
+		plugin: {} as never,
+		deleteCommand: vi.fn(),
+		saveCommands,
+	});
+
+describe("CommandList keyboard reorder", () => {
+	it("ArrowDown on a row's drag handle moves it down and persists the new order", async () => {
+		const a = new ObsidianCommand("Alpha", "a");
+		const b = new ObsidianCommand("Beta", "b");
+		const c = new ObsidianCommand("Gamma", "c");
+		const saveCommands = vi.fn();
+
+		const { getByLabelText } = render(CommandList, {
+			props: makeProps([a, b, c], saveCommands),
+		});
+
+		// Handle labels include the command identity (a11y, #1250).
+		await fireEvent.keyDown(getByLabelText("Reorder Alpha"), { key: "ArrowDown" });
+
+		await vi.waitFor(() => expect(saveCommands).toHaveBeenCalledTimes(1));
+		const saved = saveCommands.mock.calls[0][0] as ICommand[];
+		expect(saved.map((cmd) => cmd.id)).toEqual([b.id, a.id, c.id]);
+	});
+
+	it("ArrowUp on the last row moves it up", async () => {
+		const a = new ObsidianCommand("Alpha", "a");
+		const b = new ObsidianCommand("Beta", "b");
+		const c = new ObsidianCommand("Gamma", "c");
+		const saveCommands = vi.fn();
+
+		const { getByLabelText } = render(CommandList, {
+			props: makeProps([a, b, c], saveCommands),
+		});
+
+		await fireEvent.keyDown(getByLabelText("Reorder Gamma"), { key: "ArrowUp" });
+
+		await vi.waitFor(() => expect(saveCommands).toHaveBeenCalledTimes(1));
+		expect((saveCommands.mock.calls[0][0] as ICommand[]).map((cmd) => cmd.id)).toEqual([
+			a.id,
+			c.id,
+			b.id,
+		]);
+	});
+
+	it("clamps at the ends — ArrowUp on the first row is a no-op", async () => {
+		const a = new ObsidianCommand("Alpha", "a");
+		const b = new ObsidianCommand("Beta", "b");
+		const saveCommands = vi.fn();
+
+		const { getByLabelText } = render(CommandList, {
+			props: makeProps([a, b], saveCommands),
+		});
+
+		await fireEvent.keyDown(getByLabelText("Reorder Alpha"), { key: "ArrowUp" });
+		await Promise.resolve();
+		expect(saveCommands).not.toHaveBeenCalled();
+	});
+});

src/gui/MacroGUIs/CommandList.svelte

@@ -1,5 +1,6 @@
 <script lang="ts">
 import type { ICommand } from "../../types/macros/ICommand";
+import { Platform } from "obsidian";
 import { type DndEvent, dndzone, SOURCES } from "svelte-dnd-action";
 import { replaceById, stripShadow } from "../shared/dndReorder";
 import { snapshot } from "../svelte/persist.svelte";
@@ -40,7 +41,11 @@ let {
 	onEditElseBranch,
 }: CommandListProps = $props();
 
-let dragDisabled = $state(true);
+const isMobile = Platform.isMobile;
+// Desktop: drag is armed by grabbing the handle. Mobile: no handle — the whole row
+// is draggable by long-press (delayTouchStart), so drag stays enabled.
+let dragArmed = $state(false);
+const dragDisabled = $derived(!isMobile && !dragArmed);
 
 // Narrowing helpers: the {#each} discriminates on command.type, so each child
 // receives the matching subtype. Passed one-way — children report edits via the
@@ -66,18 +71,37 @@ function handleConsider(e: CustomEvent<DndEvent>) {
 function handleSort(e: CustomEvent<DndEvent>) {
 	commands = stripShadow(e.detail.items as ICommand[]);
 
+	// Desktop: disarm after a pointer drag so the handle must be grabbed again.
+	// Mobile: dragDisabled ignores dragArmed, so this is a no-op.
 	if (e.detail.info.source === SOURCES.POINTER) {
-		dragDisabled = true;
+		dragArmed = false;
 	}
 
 	persist();
 }
 
-let startDrag = (e: MouseEvent | TouchEvent) => {
-	e.preventDefault();
-	dragDisabled = false;
+// Arm svelte-dnd-action's pointer drag (desktop handle). NO preventDefault here —
+// see DragHandle: preventDefault on pointerdown would suppress the compat mousedown
+// the library starts the drag from.
+let startDrag = () => {
+	dragArmed = true;
 };
 
+// Keyboard reorder (ArrowUp/ArrowDown on a row's drag handle): move the command one
+// step and persist via the same snapshot path as a pointer drag's finalize.
+function moveCommand(id: string, direction: -1 | 1) {
+	const list = stripShadow(commands);
+	const index = list.findIndex((c) => c.id === id);
+	if (index === -1) return;
+	const target = index + direction;
+	if (target < 0 || target >= list.length) return; // clamp at the ends
+	const next = [...list];
+	const [moved] = next.splice(index, 1);
+	next.splice(target, 0, moved);
+	commands = next;
+	persist();
+}
+
 function updateCommand(command: ICommand) {
 	commands = replaceById(commands, command);
 	persist();
@@ -169,6 +193,13 @@ async function configureOpenFile(command: IOpenFileCommand) {
 		dragDisabled,
 		dropTargetStyle: {},
 		type: "command",
+		autoAriaDisabled: true,
+		// Keep the rows out of the tab order — only the action buttons / drag handle
+		// inside each row are focusable (the library defaults this to 0, adding a dead
+		// tab stop per row even with autoAriaDisabled).
+		zoneItemTabIndex: -1,
+		// Mobile: long-press (hold) initiates a row drag; a quick swipe still scrolls.
+		delayTouchStart: 200,
 	}}
 	onconsider={handleConsider}
 	onfinalize={handleSort}
@@ -181,6 +212,8 @@ async function configureOpenFile(command: IOpenFileCommand) {
 				{startDrag}
 				onDeleteCommand={deleteCommand}
 				onUpdateCommand={updateCommand}
+				onMoveUp={() => moveCommand(command.id, -1)}
+				onMoveDown={() => moveCommand(command.id, 1)}
 			/>
 		{:else if command.type === CommandType.NestedChoice}
 			<NestedChoiceCommand
@@ -189,6 +222,8 @@ async function configureOpenFile(command: IOpenFileCommand) {
 				{startDrag}
 				onDeleteCommand={deleteCommand}
 				onConfigureChoice={configureChoice}
+				onMoveUp={() => moveCommand(command.id, -1)}
+				onMoveDown={() => moveCommand(command.id, 1)}
 			/>
 		{:else if command.type === CommandType.UserScript}
 			<UserScriptCommand
@@ -197,6 +232,8 @@ async function configureOpenFile(command: IOpenFileCommand) {
 				{startDrag}
 				onDeleteCommand={deleteCommand}
 				onConfigureScript={configureScript}
+				onMoveUp={() => moveCommand(command.id, -1)}
+				onMoveDown={() => moveCommand(command.id, 1)}
 			/>
 		{:else if command.type === CommandType.AIAssistant}
 			<AIAssistantCommand
@@ -205,6 +242,8 @@ async function configureOpenFile(command: IOpenFileCommand) {
 				{startDrag}
 				onDeleteCommand={deleteCommand}
 				onConfigureAssistant={configureAssistant}
+				onMoveUp={() => moveCommand(command.id, -1)}
+				onMoveDown={() => moveCommand(command.id, 1)}
 			/>
 		{:else if command.type === CommandType.OpenFile}
 			<OpenFileCommand
@@ -213,6 +252,8 @@ async function configureOpenFile(command: IOpenFileCommand) {
 				{startDrag}
 				onDeleteCommand={deleteCommand}
 				onConfigureOpenFile={configureOpenFile}
+				onMoveUp={() => moveCommand(command.id, -1)}
+				onMoveDown={() => moveCommand(command.id, 1)}
 			/>
 		{:else if command.type === CommandType.Conditional}
 			<ConditionalCommand
@@ -223,13 +264,17 @@ async function configureOpenFile(command: IOpenFileCommand) {
 				onConfigureCondition={configureConditionalCommand}
 				onEditThenBranch={editConditionalThen}
 				onEditElseBranch={editConditionalElse}
+				onMoveUp={() => moveCommand(command.id, -1)}
+				onMoveDown={() => moveCommand(command.id, 1)}
 			/>
 		{:else}
 			<StandardCommand
 				{command}
 				{dragDisabled}
 				{startDrag}
 				onDeleteCommand={deleteCommand}
+				onMoveUp={() => moveCommand(command.id, -1)}
+				onMoveDown={() => moveCommand(command.id, 1)}
 			/>
 		{/if}
 	{/each}