diff --git a/src/App.test.tsx b/src/App.test.tsx index 96f72843..90e6671c 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -31,7 +31,7 @@ describe("Switchify PC shell", () => { function localSwitches(): switchHooks.SwitchController { const settings: switchHooks.SwitchSettings = { schemaVersion: 1, holdIntervalMs: 1000, bindings: [{ id: "space", name: "Space switch", key: "Space", pressAction: "select", holdActions: [] }] }; - return { settings, state: { settings, capture: { active: false, key: null, error: null }, supported: true, error: null, escapeHoldMs: 4000, unavailableKeys: [] }, pending: 0, error: null, capturing: false, unsaved: false, update: vi.fn(), capture: vi.fn(), cancelCapture: vi.fn().mockResolvedValue(undefined), setKeyboardEntry: vi.fn().mockResolvedValue(true), retry: vi.fn() }; + return { settings, state: { settings, capture: { active: false, key: null, error: null }, supported: true, error: null, escapeHoldMs: 4000, unavailableKeys: [] }, pending: 0, error: null, capturing: false, unsaved: false, update: vi.fn(), remove: vi.fn(), capture: vi.fn(), cancelCapture: vi.fn().mockResolvedValue(undefined), setKeyboardEntry: vi.fn().mockResolvedValue(true), retry: vi.fn() }; } it("shows local readiness without Bluetooth and explains Android suspension", async () => { diff --git a/src/Switches.test.tsx b/src/Switches.test.tsx index 3b121ff8..7312a3fd 100644 --- a/src/Switches.test.tsx +++ b/src/Switches.test.tsx @@ -1,3 +1,4 @@ +import { useLayoutEffect, type ReactNode } from "react"; import { act, fireEvent, @@ -159,6 +160,7 @@ it("cancelling a new switch also cancels its capture", async () => { event({ ...current, capture: { active: false, key: "Enter", error: null } }); await screen.findByText("Enter"); fireEvent.click(screen.getByRole("button", { name: "Cancel new switch" })); + fireEvent.click(screen.getByRole("button", { name: "Discard switch" })); expect(screen.queryByLabelText("New switch name")).toBeNull(); expect(current.settings.bindings).toHaveLength(1); }); @@ -345,8 +347,9 @@ it("keeps focus in the draft when another switch is removed while adding", async event(current); await screen.findByText("Enter"); fireEvent.click(screen.getByRole("button", { name: "Remove Head switch" })); + fireEvent.click(screen.getByRole("button", { name: "Remove switch" })); await waitFor(() => expect(current.settings.bindings).toHaveLength(0)); - expect(document.activeElement).toBe(screen.getByLabelText("New switch name")); + await waitFor(() => expect(document.activeElement).toBe(screen.getByLabelText("New switch name"))); }); it("lists remote switches with local ones and edits them in place", async () => { render(); @@ -397,9 +400,11 @@ it("removes a remote switch and surfaces a failed remote save with retry", async throw "Remote save failed"; }); fireEvent.click(screen.getByRole("button", { name: "Remove Remote switch 1" })); - await screen.findByText("Remote save failed"); + fireEvent.click(screen.getByRole("button", { name: "Remove switch" })); + await screen.findByText(/Could not remove the switch/); expect(remote.slots[0].pressAction).toBe("select"); - fireEvent.click(screen.getByRole("button", { name: "Retry save" })); + expect(screen.getByRole("heading", { name: "Remote switch 1" })).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Remove switch" })); await waitFor(() => expect(remote.slots[0].pressAction).toBeNull()); expect(screen.queryByRole("heading", { name: "Remote switch 1" })).toBeNull(); }); @@ -460,6 +465,8 @@ it("preserves a newly learned key while typing and releases capture ownership on fireEvent.change(screen.getByRole("textbox", { name: "New switch name" }), { target: { value: "Second switch" } }); expect(screen.getByText("Enter", { selector: "kbd" })).toBeTruthy(); fireEvent.click(screen.getByRole("button", { name: "Cancel new switch" })); + expect(current.keyboardEntry).toBe(true); + fireEvent.click(screen.getByRole("button", { name: "Discard switch" })); await waitFor(() => expect(current.keyboardEntry).toBe(false)); expect(current.settings.bindings).toHaveLength(1); }); @@ -472,15 +479,125 @@ it("offers recovery when returning to Switches with keyboard typing still active }); -it.each([0, 1])("releases keyboard entry when the edited remote switch is removed using control %s", async (index) => { +it.each([["Remote switch 1", 0], ["Remote switch 1", 1], ["Head switch", 0], ["Head switch", 1]] as const)("releases keyboard entry when %s is removed using control %s", async (name, index) => { render(); - fireEvent.click(await screen.findByRole("button", { name: "Edit Remote switch 1" })); + fireEvent.click(await screen.findByRole("button", { name: `Edit ${name}` })); fireEvent.click(screen.getByRole("button", { name: "Type with keyboard" })); await screen.findByRole("button", { name: "Resume switch control" }); - fireEvent.click(screen.getAllByRole("button", { name: "Remove Remote switch 1" })[index]); + fireEvent.click(screen.getAllByRole("button", { name: `Remove ${name}` })[index]); + expect(current.keyboardEntry).toBe(true); + fireEvent.click(screen.getByRole("button", { name: "Remove switch" })); await waitFor(() => expect(current.keyboardEntry).toBe(false)); expect(mocks.invoke).toHaveBeenCalledWith("set_switch_keyboard_entry", { active: false }); - expect(screen.queryByRole("heading", { name: "Remote switch 1" })).not.toBeInTheDocument(); + expect(screen.queryByRole("heading", { name })).not.toBeInTheDocument(); +}); +it("keeps an edited local draft and restores focus after cancelling discard", async () => { + render(); + await screen.findByRole("heading", { name: "Head switch" }); + fireEvent.click(screen.getByRole("button", { name: "Add switch" })); + await screen.findByRole("button", { name: "Cancel capture" }); + event({ ...current, capture: { active: false, key: "Enter", error: null } }); + fireEvent.change(await screen.findByLabelText("New switch name"), { target: { value: "Foot" } }); + const cancel = screen.getByRole("button", { name: "Cancel new switch" }); + cancel.focus(); + fireEvent.click(cancel); + const dialog = screen.getByRole("alertdialog", { name: "Discard this new switch?" }); + const keep = within(dialog).getByRole("button", { name: "Keep switch" }); + expect(keep).toHaveFocus(); + fireEvent.keyDown(keep, { key: "Tab", shiftKey: true }); + expect(within(dialog).getByRole("button", { name: "Discard switch" })).toHaveFocus(); + fireEvent.keyDown(dialog, { key: "Escape" }); + await waitFor(() => expect(cancel).toHaveFocus()); + expect(screen.getByLabelText("New switch name")).toHaveValue("Foot"); +}); +it("requires discard after changing a remote draft action, while untouched cancellation is immediate", async () => { + render(); + await screen.findByRole("heading", { name: "Remote switch 1" }); + fireEvent.click(screen.getByRole("button", { name: "Add remote switch" })); + fireEvent.change(screen.getByLabelText("New switch action"), { target: { value: "next" } }); + fireEvent.click(screen.getByRole("button", { name: "Cancel new switch" })); + expect(screen.getByRole("alertdialog")).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Discard switch" })); + expect(remote.slots[1].pressAction).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Add remote switch" })); + fireEvent.click(screen.getByRole("button", { name: "Cancel new switch" })); + expect(screen.queryByRole("alertdialog")).not.toBeInTheDocument(); + expect(screen.queryByLabelText("New switch name")).not.toBeInTheDocument(); +}); +it.each(["Head switch", "Remote switch 1"])("protects both removal controls for %s", async (name) => { + render(); + await screen.findByRole("heading", { name }); + const remove = screen.getByRole("button", { name: `Remove ${name}` }); + remove.focus(); + fireEvent.click(remove); + expect(screen.getByRole("alertdialog", { name: `Remove ${name}?` })).toBeInTheDocument(); + expect(mocks.invoke.mock.calls.some(([command]) => command.startsWith("save_"))).toBe(false); + fireEvent.click(screen.getByRole("button", { name: "Keep switch" })); + await waitFor(() => expect(remove).toHaveFocus()); + open(name); + fireEvent.click(screen.getAllByRole("button", { name: `Remove ${name}` })[1]); + fireEvent.click(screen.getByRole("button", { name: "Remove switch" })); + await waitFor(() => expect(screen.queryByRole("heading", { name })).not.toBeInTheDocument()); +}); +it("retains a local assignment when deletion fails and allows keeping it", async () => { + render(); + await screen.findByRole("heading", { name: "Head switch" }); + mocks.invoke.mockImplementationOnce(async () => { throw "Disk full"; }); + fireEvent.click(screen.getByRole("button", { name: "Remove Head switch" })); + fireEvent.click(screen.getByRole("button", { name: "Remove switch" })); + await screen.findByText(/Could not remove the switch/); + fireEvent.click(screen.getByRole("button", { name: "Keep switch" })); + expect(screen.getByRole("heading", { name: "Head switch" })).toBeInTheDocument(); + expect(current.settings.bindings[0]).toEqual(initial.settings.bindings[0]); +}); + +it("hands remote draft focus to the committed input before passive work can run", async () => { + const committedFocus: (Element | null)[] = []; + function CommitProbe({ children, revision }: { children: ReactNode; revision: number }) { + useLayoutEffect(() => { if (revision) committedFocus.push(document.activeElement); }, [revision]); + return children; + } + const view = render(); + await screen.findByRole("heading", { name: "Remote switch 1" }); + const add = screen.getByRole("button", { name: "Add remote switch" }); + add.focus(); + act(() => { + fireEvent.click(add); + // A parent update can commit alongside asynchronous backend state. Observe + // focus at that commit, before any pending passive effects get a turn. + view.rerender(); + }); + const name = screen.getByRole("textbox", { name: "New switch name" }); + expect(committedFocus).toEqual([name]); + act(() => { + fireEvent.click(screen.getByRole("button", { name: "Cancel new switch" })); + view.rerender(); + }); + expect(committedFocus[1]).toBe(screen.getByRole("button", { name: "Add switch" })); +}); + +it.each(["remove", "discard"])("restores usable focus after confirmed %s with browser inert behavior", async (operation) => { + const focus = HTMLElement.prototype.focus; + const spy = vi.spyOn(HTMLElement.prototype, "focus").mockImplementation(function (this: HTMLElement, options?: FocusOptions) { + // jsdom does not enforce inert; browsers refuse focus inside an inert root. + if (!this.closest("[inert]")) focus.call(this, options); + }); + try { + render(); + await screen.findByRole("heading", { name: "Remote switch 1" }); + if (operation === "remove") { + fireEvent.click(screen.getByRole("button", { name: "Remove Head switch" })); + } else { + fireEvent.click(screen.getByRole("button", { name: "Add remote switch" })); + fireEvent.change(screen.getByRole("textbox", { name: "New switch name" }), { target: { value: "Foot" } }); + fireEvent.click(screen.getByRole("button", { name: "Cancel new switch" })); + } + const dialog = screen.getByRole("alertdialog"); + expect(within(dialog).getByRole("button", { name: "Keep switch" })).toHaveFocus(); + fireEvent.click(within(dialog).getByRole("button", { name: operation === "remove" ? "Remove switch" : "Discard switch" })); + await waitFor(() => expect(screen.queryByRole("alertdialog")).not.toBeInTheDocument()); + expect(screen.getByRole("button", { name: "Add switch" })).toHaveFocus(); + } finally { spy.mockRestore(); } }); it("separates an empty local configuration from the six remote presets", async () => { diff --git a/src/scanning/useRemoteSwitches.ts b/src/scanning/useRemoteSwitches.ts index 3fad7687..c1a6eac8 100644 --- a/src/scanning/useRemoteSwitches.ts +++ b/src/scanning/useRemoteSwitches.ts @@ -76,11 +76,32 @@ export function useRemoteSwitches() { setConfig(next); save(m.revision, next); }; + const remove = async (index: number) => { + const m = model.current; + if (!m.config || m.pending || m.revision !== m.saved) return false; + const next = { ...m.config, slots: m.config.slots.map((slot, i) => i === index ? { pressAction: null, holdActions: [] } : slot) }; + m.pending++; + setPending(m.pending); + setError(null); + try { + const result = await invoke("save_remote_switches", { config: next }); + m.config = result; + setConfig(result); + return true; + } catch (e) { + setError(String(e)); + return false; + } finally { + m.pending--; + setPending(m.pending); + } + }; return { config, pending, error, update, + remove, retry: () => { if (model.current.config) save(model.current.revision, model.current.config); diff --git a/src/scanning/useSwitches.ts b/src/scanning/useSwitches.ts index e6af74a6..d4772f79 100644 --- a/src/scanning/useSwitches.ts +++ b/src/scanning/useSwitches.ts @@ -126,6 +126,29 @@ export function useSwitches() { setSettings(next); save(next, m.revision); }; + // Destructive saves keep the original assignment visible until persistence succeeds. + const remove = async (id: string) => { + const m = model.current; + if (m.pending || m.revision !== m.saved || m.state?.capture.active || !m.state?.supported) return false; + const next = { ...m.settings, bindings: m.settings.bindings.filter((binding) => binding.id !== id) }; + m.pending++; + setPending(m.pending); + setError(null); + try { + const stamp = runtime.current; + const result = await invoke("save_switches", { settings: next }); + m.settings = result.settings; + setSettings(result.settings); + if (stamp === runtime.current) receive(result); + return true; + } catch (e) { + setError(String(e)); + return false; + } finally { + m.pending--; + setPending(m.pending); + } + }; const capture = async () => { const request = ++captureRequest.current; setCapturing(true); @@ -195,6 +218,7 @@ export function useSwitches() { error, capturing, update, + remove, capture, cancelCapture, setKeyboardEntry, diff --git a/src/settings/SwitchesSection.tsx b/src/settings/SwitchesSection.tsx index 8ee00ebb..03be13e6 100644 --- a/src/settings/SwitchesSection.tsx +++ b/src/settings/SwitchesSection.tsx @@ -1,4 +1,5 @@ -import { useEffect, useId, useRef, useState } from "react"; +import { createPortal } from "react-dom"; +import { useEffect, useId, useLayoutEffect, useRef, useState } from "react"; import { ChevronDown, ChevronUp, Keyboard, Smartphone, Trash2, X } from "lucide-react"; import { useRemoteSwitches, type RemoteSlot } from "../scanning/useRemoteSwitches"; import { @@ -350,6 +351,34 @@ function CaptureDialog({ name, onCancel }: { name: string; onCancel: () => void ); } +function SwitchConfirmation({ title, action, busy, error, cancel, confirm }: { title: string; action: string; busy: boolean; error: string | null; cancel: () => void; confirm: () => void }) { + const ref = useRef(null); + const keepRef = useRef(null); + const titleId = useId(); + useLayoutEffect(() => { + const background = [...document.body.children].filter((element) => !element.contains(ref.current)); + const previous = background.map((element) => element.hasAttribute("inert")); + background.forEach((element) => element.setAttribute("inert", "")); + keepRef.current?.focus(); + return () => background.forEach((element, index) => { if (!previous[index]) element.removeAttribute("inert"); }); + }, []); + return createPortal(
{ + event.stopPropagation(); + if (event.key === "Escape") { event.preventDefault(); if (!busy) cancel(); } + if (event.key === "Tab") { + const buttons = [...(ref.current?.querySelectorAll("button:not(:disabled)") ?? [])]; + const first = buttons[0]; const last = buttons.at(-1); + if (!first) { event.preventDefault(); return; } + if (event.shiftKey && document.activeElement === first) { event.preventDefault(); last?.focus(); } + else if (!event.shiftKey && document.activeElement === last) { event.preventDefault(); first.focus(); } + } + }}> +

{title}

+ {error &&

{error} Your switch is still available. Try again or keep it.

} +
+
, document.body); +} + export function SwitchesSection({ controller, onDraftChange, suspended = false, androidConnected }: { controller: SwitchController; onDraftChange?: (draft: boolean) => void; suspended?: boolean; androidConnected?: boolean }) { const { settings, state, pending, unsaved } = controller; // A refused capture sets both the general error and the capture error; the @@ -360,6 +389,16 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, : null; const [draft, setDraft] = useState(null); // A draft with a slot is a remote switch; it never learns a key. + const [confirmation, setConfirmation] = useState<{ kind: "local" | "remote" | "draft"; id: string; index?: number; name: string } | null>(null); + const [confirmBusy, setConfirmBusy] = useState(false); + const [confirmError, setConfirmError] = useState(null); + const opener = useRef(null); + const initialDraft = useRef(null); + const requestRemoval = (kind: "local" | "remote" | "draft", binding: Binding, index?: number) => { + opener.current = document.activeElement instanceof HTMLElement ? document.activeElement : null; + setConfirmError(null); + setConfirmation({ kind, id: binding.id, index, name: binding.name || (kind === "remote" ? `Remote switch ${(index ?? 0) + 1}` : "this switch") }); + }; const [draftSlot, setDraftSlot] = useState(null); useEffect(() => { onDraftChange?.(draft !== null); }, [draft, onDraftChange]); const remote = useRemoteSwitches(); @@ -446,7 +485,10 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, const addRef = useRef(null); const addRemoteRef = useRef(null); const focusAfter = useRef(null); - useEffect(() => { + // Consume the handoff only after its DOM commit. A pending passive effect + // from the previous render can otherwise focus an Add button just before it + // unmounts, consuming the handoff and leaving focus on the document body. + useLayoutEffect(() => { if (!focusAfter.current) return; const id = focusAfter.current; focusAfter.current = null; @@ -516,15 +558,6 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, b.id === binding.id ? binding : b, ), }); - const remove = (id: string) => { - releaseEntry(); - if (expanded === id) setExpanded(null); - focusAfter.current = newId; - controller.update({ - ...settings, - bindings: settings.bindings.filter((b) => b.id !== id), - }); - }; const learn = (id: string) => { releaseEntry(); setRowError(null); @@ -533,7 +566,8 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, }; const startAdd = () => { setDraftSlot(null); - setDraft({ id: newId, name: "", key: "", pressAction: "select", holdActions: [] }); + initialDraft.current = { id: newId, name: "", key: "", pressAction: "select", holdActions: [] }; + setDraft(initialDraft.current); setExpanded(newId); learn(newId); }; @@ -541,7 +575,8 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, const slot = freeSlots[0]; if (slot === undefined) return; setDraftSlot(slot); - setDraft({ id: newId, name: "", key: `Remote ${slot + 1}`, pressAction: "select", holdActions: [] }); + initialDraft.current = { id: newId, name: "", key: `Remote ${slot + 1}`, pressAction: "select", holdActions: [] }; + setDraft(initialDraft.current); setExpanded(newId); // The Add buttons unmount while drafting; the name field takes focus. focusAfter.current = newId; @@ -557,6 +592,26 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, void controller.cancelCapture(); } }; + const cancelConfirmation = () => { + setConfirmation(null); + // The portal restores the background before focus returns to its opener. + requestAnimationFrame(() => { if (opener.current?.isConnected) opener.current.focus(); }); + }; + const confirmRemoval = async () => { + if (!confirmation || confirmBusy) return; + if (confirmation.kind === "draft") { setConfirmation(null); cancelAdd(); return; } + setConfirmBusy(true); + setConfirmError(null); + const removed = confirmation.kind === "local" ? await controller.remove(confirmation.id) : await remote.remove(confirmation.index!); + setConfirmBusy(false); + if (removed) { + releaseEntry(); + if (expanded === confirmation.id) setExpanded(null); + setConfirmation(null); + focusAfter.current = newId; + } + else setConfirmError("Could not remove the switch."); + }; const add = () => { if (!draft) return; if (draftSlot !== null) { @@ -603,6 +658,7 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, Keyboard typing is on. Switch scanning is paused. } + {confirmation && !suspended && void confirmRemoval()} />} {capturing && target && !suspended && ( remove(binding.id)} + disabled={disabled || !!pending || unsaved} + onClick={() => requestRemoval("local", binding)} > @@ -760,7 +816,7 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, focusAfter.current = binding.id; setExpanded(null); }} - onRemove={() => remove(binding.id)} + onRemove={() => requestRemoval("local", binding)} nameRef={nameRef} /> )} @@ -804,12 +860,8 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, type="button" className="icon-button danger-icon" aria-label={`Remove ${name}`} - onClick={() => { - releaseEntry(); - if (expanded === binding.id) setExpanded(null); - focusAfter.current = newId; - setSlot(index, null); - }} + disabled={!!remote.pending || remote.unsaved} + onClick={() => requestRemoval("remote", binding, index)} > @@ -830,12 +882,7 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, focusAfter.current = binding.id; setExpanded(null); }} - onRemove={() => { - releaseEntry(); - if (expanded === binding.id) setExpanded(null); - focusAfter.current = newId; - setSlot(index, null); - }} + onRemove={() => requestRemoval("remote", binding, index)} nameRef={nameRef} remote={{ slot: index, free: freeSlots, onSlot: (to) => moveSlot(index, to) }} /> @@ -869,7 +916,10 @@ export function SwitchesSection({ controller, onDraftChange, suspended = false, onChange={setDraft} onLearn={() => learn(newId)} onDone={add} - onRemove={cancelAdd} + onRemove={() => { + if (JSON.stringify(draft) === JSON.stringify(initialDraft.current)) cancelAdd(); + else requestRemoval("draft", draft); + }} nameRef={nameRef} remote={ draftSlot === null