diff --git a/CHANGELOG.md b/CHANGELOG.md index 34331fb86..044b02100 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Consolidated Bandit, dependency audits, supplemental secret checks, and Trivy into one trusted-branch security backstop, delegated CodeQL to GitHub default setup, and removed duplicate local PR security and release-preflight runs. - Pinned npm `10.9.9` as the approved lockfile generator, activated it through Node-bundled Corepack before dependency consumption, and fail closed unless its bundled `tar` is at least `7.5.19`; primary CI still consumes the committed lock only through frozen `npm ci` validation, rejects mutable npm resolution in the lock gate, requires integrity evidence for public-registry lock entries, and preserves generator-sensitive root `@esbuild/*` peer metadata. +- Keep unavailable Add/Open/Remove score actions and PDF pagination controls keyboard-focusable, expose their unavailable state and recovery copy to assistive technology, and prevent project-missing, pagination-boundary, or repeated in-flight attach activation at the action boundary. ### Fixed diff --git a/apps/desktop/src/features/score/ScoreView.disabled-action-accessibility.test.tsx b/apps/desktop/src/features/score/ScoreView.disabled-action-accessibility.test.tsx new file mode 100644 index 000000000..dbe9250f5 --- /dev/null +++ b/apps/desktop/src/features/score/ScoreView.disabled-action-accessibility.test.tsx @@ -0,0 +1,57 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { RehearsalSong } from "@bandscope/shared-types"; +import { invoke } from "@tauri-apps/api/core"; +import { ScoreView } from "./ScoreView"; + +vi.mock("@tauri-apps/api/core", () => ({ + invoke: vi.fn() +})); + +vi.mock("./ScoreViewer", () => ({ + ScoreViewer: () =>
+})); + +vi.mock("../../i18n", () => ({ + createTranslator: () => (key: string) => + ({ + scoreViewTitle: "Score", + scoreViewSubtitle: "Attach validated PDF scores to the current song.", + scoreListTitle: "Attached scores", + scoreAttach: "Add score", + scoreRemove: "Remove", + scoreOpen: "Open score", + scoreRequiresProject: "Scores attach to the active analysis project.", + scoreNavDisabledHint: "Analyze or open a song first" + })[key] ?? key, + detectPreferredLocale: () => "en" +})); + +const song = { + id: "song-1", + title: "Late Night Set", + scoreAttachments: [{ id: "score-1", fileName: "opener.pdf" }] +} as RehearsalSong; + +describe("ScoreView unavailable action accessibility", () => { + it("links focusable unavailable actions to localized recovery copy and blocks activation", () => { + render(); + + const requirement = screen.getByText("Scores attach to the active analysis project."); + const addButton = screen.getByRole("button", { name: "Add score" }); + const openButton = screen.getByRole("button", { name: "Open score: opener.pdf" }); + const removeButton = screen.getByRole("button", { name: "Remove: opener.pdf" }); + + for (const button of [addButton, openButton, removeButton]) { + expect(button).toHaveAttribute("aria-disabled", "true"); + expect(button).toHaveAttribute("aria-describedby", requirement.id); + expect(button).toHaveAttribute("title", "Analyze or open a song first"); + expect(button).not.toBeDisabled(); + } + + fireEvent.click(addButton); + fireEvent.click(openButton); + fireEvent.click(removeButton); + expect(vi.mocked(invoke)).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/desktop/src/features/score/ScoreView.enabled-action-tooltips.test.tsx b/apps/desktop/src/features/score/ScoreView.enabled-action-tooltips.test.tsx new file mode 100644 index 000000000..e7a32dd43 --- /dev/null +++ b/apps/desktop/src/features/score/ScoreView.enabled-action-tooltips.test.tsx @@ -0,0 +1,39 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { RehearsalSong } from "@bandscope/shared-types"; +import { ScoreView } from "./ScoreView"; + +vi.mock("@tauri-apps/api/core", () => ({ + invoke: vi.fn() +})); + +vi.mock("./ScoreViewer", () => ({ + ScoreViewer: () =>
Mock Viewer
+})); + +vi.mock("../../i18n", () => ({ + createTranslator: () => (key: string) => + ({ + scoreOpen: "Open score", + scoreRemove: "Remove" + })[key] ?? key, + detectPreferredLocale: () => "en" +})); + +describe("ScoreView enabled action tooltips", () => { + it("exposes localized pointer tooltips for enabled open and remove actions", () => { + const song = { + id: "song-1", + title: "Test", + scoreAttachments: [{ id: "doc1", fileName: "opener.pdf" }] + } as RehearsalSong; + + render(); + + expect(screen.getByRole("button", { name: "Open score: opener.pdf" })).toHaveAttribute("title", "Open score: opener.pdf"); + expect(screen.getByRole("button", { name: "Remove: opener.pdf" })).toHaveAttribute( + "title", + "Remove: opener.pdf" + ); + }); +}); diff --git a/apps/desktop/src/features/score/ScoreView.error-privacy.test.tsx b/apps/desktop/src/features/score/ScoreView.error-privacy.test.tsx new file mode 100644 index 000000000..5e37ec7d9 --- /dev/null +++ b/apps/desktop/src/features/score/ScoreView.error-privacy.test.tsx @@ -0,0 +1,68 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import type { RehearsalSong } from "@bandscope/shared-types"; +import { beforeEach, expect, it, vi } from "vitest"; + +import { ScoreView } from "./ScoreView"; +import { attachScorePdf } from "./scoreStorage"; + +vi.mock("./scoreStorage", () => ({ + attachScorePdf: vi.fn(), + readScorePdf: vi.fn(), + removeScorePdf: vi.fn() +})); + +vi.mock("./ScoreViewer", () => ({ + ScoreViewer: () =>
+})); + +vi.mock("../../i18n", () => ({ + createTranslator: () => (key: string) => + ({ + scoreViewTitle: "Score", + scoreViewSubtitle: "Attach validated PDF scores to the current song.", + scoreListTitle: "Attached scores", + scoreListEmpty: "No scores attached to this song yet.", + scoreAttach: "Add score", + scoreAttaching: "Attaching...", + scoreRemove: "Remove", + scoreRemoveConfirm: "Remove {fileName} from this song?", + scoreOpen: "Open score", + scoreOpening: "Opening score PDF...", + scoreAttachFailed: "Could not attach the score PDF.", + scoreReadFailed: "Could not open the score PDF.", + scoreRemoveFailed: "Could not remove the score PDF.", + scoreRequiresProject: "Scores attach to the active analysis project.", + scoreNavDisabledHint: "Open an active project first." + })[key] ?? key, + detectPreferredLocale: () => "en" +})); + +const mockAttachScorePdf = vi.mocked(attachScorePdf); + +function makeSong(): RehearsalSong { + return { + id: "song-1", + title: "Late Night Set", + sections: [], + exportSummary: { format: "cue-sheet", headline: "", focusSections: [] } + } as RehearsalSong; +} + +beforeEach(() => { + mockAttachScorePdf.mockReset(); +}); + +it("does not render dependency-controlled score bridge secrets or local paths", async () => { + mockAttachScorePdf.mockRejectedValueOnce( + new Error("Failed to open /Users/Alice/private-score.pdf token=super-secret") + ); + + render(); + fireEvent.click(screen.getByRole("button", { name: "Add score" })); + + const alert = await screen.findByRole("alert"); + expect(alert).toHaveTextContent("Could not attach the score PDF."); + expect(alert).not.toHaveTextContent("/Users/Alice"); + expect(alert).not.toHaveTextContent("private-score.pdf"); + expect(alert).not.toHaveTextContent("token=super-secret"); +}); diff --git a/apps/desktop/src/features/score/ScoreView.test.tsx b/apps/desktop/src/features/score/ScoreView.test.tsx index de4ccb95c..5f292158d 100644 --- a/apps/desktop/src/features/score/ScoreView.test.tsx +++ b/apps/desktop/src/features/score/ScoreView.test.tsx @@ -1,4 +1,4 @@ -import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, createEvent, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { RehearsalSong, ScoreAttachment } from "@bandscope/shared-types"; import { invoke } from "@tauri-apps/api/core"; @@ -90,17 +90,103 @@ describe("ScoreView", () => { expect(mockInvoke).not.toHaveBeenCalled(); }); - it("disables score storage actions when no project workspace is active", () => { + it("ignores clicks while attaching", async () => { + const song = makeSong([]); + render(); + const addBtn = screen.getByRole("button", { name: "Add score" }); + + // In our mock, attachScorePdf returns a promise. + // We can mock it to not resolve immediately, simulating a pending attach + let resolveAttach: (val: unknown) => void; + mockInvoke.mockReturnValueOnce(new Promise((resolve) => { + resolveAttach = resolve; + })); + + await act(async () => { + fireEvent.click(addBtn); + }); + + // Second click should hit `!isAttaching` branch and do nothing + await act(async () => { + fireEvent.click(addBtn); + }); + + expect(mockInvoke).toHaveBeenCalledTimes(1); + + // Resolve the promise to cleanup + await act(async () => { + resolveAttach({ id: "new-score", fileName: "new.pdf" }); + }); + }); + + it("keeps unavailable score storage actions focusable when no project workspace is active", () => { const song = makeSong([{ id: SCORE_ID, fileName: "opener.pdf" }]); render(); expect(screen.getByText("Scores attach to the active analysis project.")).toBeInTheDocument(); - expect(screen.getByRole("button", { name: "Add score" })).toBeDisabled(); - expect(screen.getByRole("button", { name: "Open score: opener.pdf" })).toBeDisabled(); - expect(screen.getByRole("button", { name: "Remove: opener.pdf" })).toBeDisabled(); - fireEvent.click(screen.getByRole("button", { name: "Open score: opener.pdf" })); + const addBtn = screen.getByRole("button", { name: "Add score" }); + expect(addBtn).toHaveAttribute("aria-disabled", "true"); + expect(addBtn).toHaveAttribute("aria-describedby"); + expect(addBtn).toHaveClass("aria-disabled:cursor-not-allowed", "aria-disabled:opacity-60"); + expect(addBtn).toHaveAttribute("title", "scoreNavDisabledHint"); + expect(addBtn).not.toBeDisabled(); + + const openBtn = screen.getByRole("button", { name: "Open score: opener.pdf" }); + expect(openBtn).toHaveAttribute("aria-disabled", "true"); + expect(openBtn).toHaveAttribute("aria-describedby"); + expect(openBtn).toHaveClass("aria-disabled:cursor-not-allowed", "aria-disabled:opacity-60"); + expect(openBtn).toHaveAttribute("title", "scoreNavDisabledHint"); + + const removeBtn = screen.getByRole("button", { name: "Remove: opener.pdf" }); + expect(removeBtn).toHaveAttribute("aria-disabled", "true"); + expect(removeBtn).toHaveAttribute("aria-describedby"); + expect(removeBtn).toHaveClass("aria-disabled:cursor-not-allowed", "aria-disabled:opacity-60"); + expect(removeBtn).toHaveAttribute("title", "scoreNavDisabledHint"); + + const addClickEvent = createEvent.click(addBtn); + fireEvent(addBtn, addClickEvent); + expect(addClickEvent.defaultPrevented).toBe(true); + expect(mockInvoke).not.toHaveBeenCalled(); + + const openClickEvent = createEvent.click(openBtn); + fireEvent(openBtn, openClickEvent); + expect(openClickEvent.defaultPrevented).toBe(true); expect(mockInvoke).not.toHaveBeenCalled(); + + const clickEvent = createEvent.click(removeBtn); + fireEvent(removeBtn, clickEvent); + expect(clickEvent.defaultPrevented).toBe(true); + }); + + it("blocks repeated attach activation while an attach is already pending", async () => { + let resolveAttach!: (value: unknown) => void; + mockInvoke + .mockImplementationOnce(() => new Promise((resolve) => { resolveAttach = resolve; })) + .mockResolvedValueOnce([1, 2, 3]); + const onSongUpdate = vi.fn(); + + render(); + + const addBtn = screen.getByRole("button", { name: "Add score" }); + fireEvent.click(addBtn); + + await waitFor(() => { + expect(addBtn).toHaveAttribute("aria-disabled", "true"); + }); + + const repeatedClick = createEvent.click(addBtn); + fireEvent(addBtn, repeatedClick); + expect(repeatedClick.defaultPrevented).toBe(true); + expect(mockInvoke).toHaveBeenCalledTimes(1); + + resolveAttach(attachResponse()); + + await waitFor(() => { + expect(screen.getByTestId("score-viewer")).toHaveTextContent("bytes:3:opener.pdf"); + }); + expect(mockInvoke).toHaveBeenCalledTimes(2); + expect(onSongUpdate).toHaveBeenCalledTimes(1); }); it("attaches a score, persists the metadata, and opens the new PDF", async () => { diff --git a/apps/desktop/src/features/score/ScoreView.tsx b/apps/desktop/src/features/score/ScoreView.tsx index 72732450f..c3d857e2b 100644 --- a/apps/desktop/src/features/score/ScoreView.tsx +++ b/apps/desktop/src/features/score/ScoreView.tsx @@ -1,4 +1,4 @@ -import { useMemo, useRef, useState } from "react"; +import { useId, useMemo, useRef, useState } from "react"; import { FileMusic, FilePlus2, Loader2, Trash2 } from "lucide-react"; import type { RehearsalSong, ScoreAttachment } from "@bandscope/shared-types"; import { createTranslator, detectPreferredLocale } from "../../i18n"; @@ -29,7 +29,14 @@ export interface ScoreViewProps { function bridgeErrorDetail(error: unknown, fallback: string): string { const raw = error instanceof Error ? error.message : typeof error === "string" ? error : null; const firstLine = raw?.split(/\r?\n/)[0]?.trim(); - return firstLine ? firstLine : fallback; + if (!firstLine) return fallback; + + // Protect against dependency information leakage (paths and secrets) + if (firstLine.includes("/") || firstLine.includes("\\") || firstLine.toLowerCase().includes("token=")) { + return fallback; + } + + return firstLine; } /** @@ -39,6 +46,7 @@ function bridgeErrorDetail(error: unknown, fallback: string): string { */ export function ScoreView({ song, projectId, onSongUpdate }: ScoreViewProps) { const t = useMemo(() => createTranslator(detectPreferredLocale()), []); + const scoreRequiresProjectId = useId(); const attachments = useMemo(() => song.scoreAttachments ?? [], [song.scoreAttachments]); const [selected, setSelected] = useState(null); const [pdfBytes, setPdfBytes] = useState(null); @@ -78,8 +86,8 @@ export function ScoreView({ song, projectId, onSongUpdate }: ScoreViewProps) { /** * Attach a new score PDF via the native picker and open it. The attach - * control is disabled while `isAttaching`, so overlapping attaches cannot be - * started; the active project id is supplied by the enabled control. + * control is action-guarded while `isAttaching`, so overlapping attaches + * cannot be started; the active project id is supplied by the enabled control. */ const handleAttach = async (activeProjectId: string) => { setError(null); @@ -134,10 +142,18 @@ export function ScoreView({ song, projectId, onSongUpdate }: ScoreViewProps) {

{t("scoreViewSubtitle")}

{!projectId && ( -

+

{t("scoreRequiresProject")}

)} @@ -183,11 +202,19 @@ export function ScoreView({ song, projectId, onSongUpdate }: ScoreViewProps) { > diff --git a/apps/desktop/src/features/score/ScoreViewer.disabled-navigation-accessibility.test.tsx b/apps/desktop/src/features/score/ScoreViewer.disabled-navigation-accessibility.test.tsx new file mode 100644 index 000000000..ff66c6724 --- /dev/null +++ b/apps/desktop/src/features/score/ScoreViewer.disabled-navigation-accessibility.test.tsx @@ -0,0 +1,117 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { PDFDocumentLoadingTask, PDFDocumentProxy } from "pdfjs-dist"; +import { ScoreViewer } from "./ScoreViewer"; +import { loadScorePdf } from "./pdfjs"; + +vi.mock("./pdfjs", () => ({ + loadScorePdf: vi.fn() +})); + +vi.mock("../../i18n", () => ({ + createTranslator: () => (key: string) => + ({ + scoreViewerPrevPage: "Previous page", + scoreViewerNextPage: "Next page", + scoreViewerPageIndicator: "Page {current} of {total}", + scoreViewerZoomIn: "Zoom in", + scoreViewerZoomOut: "Zoom out", + scoreViewerFitWidth: "Fit width", + scoreViewerPrevPageDisabled: "Already at the first page", + scoreViewerNextPageDisabled: "Already at the last page" + })[key] ?? key, + detectPreferredLocale: () => "en" +})); + +const SAMPLE_BYTES = new Uint8Array([0x25, 0x50, 0x44, 0x46]); + +function createFakeDocument(): PDFDocumentProxy { + const page = { + getViewport: vi.fn(({ scale }: { scale: number }) => ({ + width: 600 * scale, + height: 800 * scale + })), + render: vi.fn(() => ({ + promise: Promise.resolve(), + cancel: vi.fn() + })) + }; + + return { + numPages: 3, + getPage: vi.fn(() => Promise.resolve(page)) + } as unknown as PDFDocumentProxy; +} + +describe("ScoreViewer disabled page navigation accessibility", () => { + beforeEach(() => { + vi.mocked(loadScorePdf).mockReset(); + vi.mocked(loadScorePdf).mockReturnValue({ + promise: Promise.resolve(createFakeDocument()), + destroy: vi.fn(() => Promise.resolve()) + } as unknown as PDFDocumentLoadingTask); + }); + + it("keeps boundary reasons hoverable, focusable, and dismissible with Escape", async () => { + render(); + + expect(await screen.findByText("Page 1 of 3")).toBeInTheDocument(); + const previousButton = screen.getByRole("button", { name: "Previous page" }); + const nextButton = screen.getByRole("button", { name: "Next page" }); + const previousDescriptionId = previousButton.getAttribute("aria-describedby"); + + expect(previousButton).toHaveAttribute("aria-disabled", "true"); + expect(previousButton).not.toHaveAttribute("title"); + expect(previousDescriptionId).toBeTruthy(); + + const previousReason = screen.getByRole("tooltip"); + expect(previousReason).toHaveAttribute("id", previousDescriptionId); + expect(previousReason).toHaveTextContent("Already at the first page"); + expect(previousReason).toHaveClass( + "group-hover:opacity-100", + "group-focus-within:opacity-100", + "motion-reduce:transition-none", + "pointer-events-none", + "group-hover:pointer-events-auto", + "group-focus-within:pointer-events-auto" + ); + expect(previousReason).not.toHaveClass("mb-2"); + previousButton.focus(); + expect(previousButton).toHaveFocus(); + expect(nextButton).not.toHaveAttribute("aria-describedby"); + + fireEvent.keyDown(document, { key: "Escape" }); + expect(screen.queryByRole("tooltip")).not.toBeInTheDocument(); + expect(previousButton).not.toHaveAttribute("aria-describedby"); + + previousButton.blur(); + previousButton.focus(); + expect(previousButton).toHaveAttribute("aria-describedby"); + expect(screen.getByRole("tooltip")).toHaveTextContent("Already at the first page"); + + fireEvent.click(nextButton); + fireEvent.click(nextButton); + expect(await screen.findByText("Page 3 of 3")).toBeInTheDocument(); + + const nextDescriptionId = nextButton.getAttribute("aria-describedby"); + expect(nextButton).toHaveAttribute("aria-disabled", "true"); + expect(nextButton).not.toHaveAttribute("title"); + expect(nextDescriptionId).toBeTruthy(); + + const nextReason = screen.getByRole("tooltip"); + expect(nextReason).toHaveAttribute("id", nextDescriptionId); + expect(nextReason).toHaveTextContent("Already at the last page"); + expect(nextReason).toHaveClass( + "group-hover:opacity-100", + "group-focus-within:opacity-100", + "motion-reduce:transition-none", + "pointer-events-none", + "group-hover:pointer-events-auto", + "group-focus-within:pointer-events-auto" + ); + expect(nextReason).not.toHaveClass("mb-2"); + nextButton.focus(); + expect(nextButton).toHaveFocus(); + expect(previousButton).not.toHaveAttribute("aria-describedby"); + }); +}); \ No newline at end of file diff --git a/apps/desktop/src/features/score/ScoreViewer.test.tsx b/apps/desktop/src/features/score/ScoreViewer.test.tsx index 3ac2dd605..bd4761639 100644 --- a/apps/desktop/src/features/score/ScoreViewer.test.tsx +++ b/apps/desktop/src/features/score/ScoreViewer.test.tsx @@ -1,4 +1,4 @@ -import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, createEvent, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { PDFDocumentLoadingTask, PDFDocumentProxy } from "pdfjs-dist"; import { ScoreViewer } from "./ScoreViewer"; @@ -120,8 +120,15 @@ describe("ScoreViewer", () => { expect(page.render).toHaveBeenCalled(); }); expect(page.getViewport).toHaveBeenCalledWith({ scale: 1 }); - expect(screen.getByRole("button", { name: "Previous page" })).toBeDisabled(); - expect(screen.getByRole("button", { name: "Next page" })).toBeEnabled(); + const prevBtn = screen.getByRole("button", { name: "Previous page" }); + const nextBtn = screen.getByRole("button", { name: "Next page" }); + expect(prevBtn).toHaveAttribute("aria-disabled", "true"); + expect(prevBtn).toHaveClass("aria-disabled:cursor-not-allowed", "aria-disabled:opacity-60"); + expect(nextBtn).toHaveAttribute("aria-disabled", "false"); + + const clickEvent = createEvent.click(prevBtn); + fireEvent(prevBtn, clickEvent); + expect(clickEvent.defaultPrevented).toBe(true); }); it("shows the file name when provided", async () => { @@ -174,14 +181,20 @@ describe("ScoreViewer", () => { expect(await screen.findByText("Page 1 of 3")).toBeInTheDocument(); const previousButton = screen.getByRole("button", { name: "Previous page" }); const nextButton = screen.getByRole("button", { name: "Next page" }); - expect(previousButton).toBeDisabled(); + expect(previousButton).toHaveAttribute("aria-disabled", "true"); + expect(previousButton).toHaveClass("aria-disabled:cursor-not-allowed", "aria-disabled:opacity-60"); fireEvent.click(nextButton); expect(screen.getByText("Page 2 of 3")).toBeInTheDocument(); fireEvent.click(nextButton); expect(screen.getByText("Page 3 of 3")).toBeInTheDocument(); - expect(nextButton).toBeDisabled(); + expect(nextButton).toHaveAttribute("aria-disabled", "true"); + expect(nextButton).toHaveClass("aria-disabled:cursor-not-allowed", "aria-disabled:opacity-60"); + + const clickEvent = createEvent.click(nextButton); + fireEvent(nextButton, clickEvent); + expect(clickEvent.defaultPrevented).toBe(true); await waitFor(() => { expect(doc.getPage).toHaveBeenCalledWith(3); diff --git a/apps/desktop/src/features/score/ScoreViewer.tsx b/apps/desktop/src/features/score/ScoreViewer.tsx index 82692469e..89a6bdaf1 100644 --- a/apps/desktop/src/features/score/ScoreViewer.tsx +++ b/apps/desktop/src/features/score/ScoreViewer.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useId, useMemo, useRef, useState } from "react"; import type { PDFDocumentProxy, RenderTask } from "pdfjs-dist"; import { AlertCircle, @@ -47,6 +47,8 @@ const MAX_ZOOM = 4; */ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps) { const t = useMemo(() => createTranslator(detectPreferredLocale()), []); + const previousDisabledDescriptionId = useId(); + const nextDisabledDescriptionId = useId(); const [status, setStatus] = useState("LOADING"); const [errorMessage, setErrorMessage] = useState(null); const [pdfDocument, setPdfDocument] = useState(null); @@ -56,6 +58,7 @@ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps const [fitWidth, setFitWidth] = useState(true); const [containerWidth, setContainerWidth] = useState(0); const [retryToken, setRetryToken] = useState(0); + const [boundaryReasonDismissed, setBoundaryReasonDismissed] = useState(false); const canvasRef = useRef(null); const containerRef = useRef(null); @@ -65,6 +68,16 @@ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps } }, [data, status, onStatusChange]); + useEffect(() => { + const dismissBoundaryReason = (event: KeyboardEvent) => { + if (event.key === "Escape") { + setBoundaryReasonDismissed(true); + } + }; + document.addEventListener("keydown", dismissBoundaryReason); + return () => document.removeEventListener("keydown", dismissBoundaryReason); + }, []); + useEffect(() => { if (data === null) { return; @@ -84,6 +97,7 @@ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps setPdfDocument(loadedDocument); setPageCount(loadedDocument.numPages); setPageNumber(1); + setBoundaryReasonDismissed(false); setStatus("READY"); }) .catch((error: unknown) => { @@ -153,11 +167,13 @@ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps /** Move to the previous page, clamped at the first page. */ const goToPreviousPage = () => { + setBoundaryReasonDismissed(false); setPageNumber((current) => Math.max(1, current - 1)); }; /** Move to the next page, clamped at the last page. */ const goToNextPage = () => { + setBoundaryReasonDismissed(false); setPageNumber((current) => Math.min(pageCount, current + 1)); }; @@ -180,6 +196,7 @@ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps /** Re-run the load state machine with the same validated bytes. */ const retry = () => { + setBoundaryReasonDismissed(false); setRetryToken((current) => current + 1); }; @@ -241,6 +258,10 @@ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps const pageIndicator = t("scoreViewerPageIndicator") .replace("{current}", String(pageNumber)) .replace("{total}", String(pageCount)); + const previousPageUnavailable = pageNumber <= 1; + const nextPageUnavailable = pageNumber >= pageCount; + const unavailableReasonClassName = + "pointer-events-none absolute bottom-full left-1/2 z-10 w-max max-w-48 -translate-x-1/2 rounded-md border border-white/10 bg-slate-950 px-2 py-1 text-center text-xs text-slate-100 opacity-0 shadow-lg transition-opacity motion-reduce:transition-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto group-hover:opacity-100 group-focus-within:opacity-100"; return ( @@ -258,6 +279,7 @@ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps size="icon-lg" className="size-12" aria-label={t("scoreViewerZoomOut")} + title={t("scoreViewerZoomOut")} onClick={zoomOut} > ); -} +} \ No newline at end of file diff --git a/apps/desktop/src/locales/en/common.json b/apps/desktop/src/locales/en/common.json index d803a765e..9fc338be5 100644 --- a/apps/desktop/src/locales/en/common.json +++ b/apps/desktop/src/locales/en/common.json @@ -149,6 +149,8 @@ "practiceProgressLabel": "Practice Progress", "decreasePracticeProgressLabel": "Decrease progress", "increasePracticeProgressLabel": "Increase progress", + "scoreViewerPrevPageDisabled": "Already at the first page", + "scoreViewerNextPageDisabled": "Already at the last page", "workspaceFirstRangeTitle": "Tonight's first range", "workspaceFirstRangeCheck": "{roleName} sits {lowestNote}–{highestNote} in {sectionLabel}. Check that span on your instrument before the {sectionLabel}.", "workspaceFirstRangeClash": "{roleName} sits {lowestNote}–{highestNote} in {sectionLabel}. Hear that clash on your instrument before the {sectionLabel}.", diff --git a/apps/desktop/src/locales/ko/common.json b/apps/desktop/src/locales/ko/common.json index 0f6c6c66d..e86f6b610 100644 --- a/apps/desktop/src/locales/ko/common.json +++ b/apps/desktop/src/locales/ko/common.json @@ -149,6 +149,8 @@ "practiceProgressLabel": "연습 진척도", "decreasePracticeProgressLabel": "진척도 감소", "increasePracticeProgressLabel": "진척도 증가", + "scoreViewerPrevPageDisabled": "첫 번째 페이지입니다", + "scoreViewerNextPageDisabled": "마지막 페이지입니다", "workspaceFirstRangeTitle": "오늘 먼저 볼 음역", "workspaceFirstRangeCheck": "{sectionLabel}의 {roleName}은 {lowestNote}–{highestNote}입니다. {sectionLabel} 들어가기 전에 그 음역을 악기로 확인해 보세요.", "workspaceFirstRangeClash": "{sectionLabel}의 {roleName}은 {lowestNote}–{highestNote}이고 다른 파트와 겹칩니다. {sectionLabel} 들어가기 전에 그 충돌을 악기로 들어 보세요.", diff --git a/docs/doctoring/accessible-disabled-score-navigation.md b/docs/doctoring/accessible-disabled-score-navigation.md new file mode 100644 index 000000000..b1efa7ea0 --- /dev/null +++ b/docs/doctoring/accessible-disabled-score-navigation.md @@ -0,0 +1,56 @@ +# Accessible disabled score actions + +## Scope + +BandScope keeps selected score actions discoverable when they are unavailable by using `aria-disabled="true"` plus guarded click handlers instead of native `disabled`. The contract covers the Add score control when no active project exists, existing-score Open/Remove controls without an active project, and score-viewer Previous/Next page controls at pagination boundaries. + +For project-bound actions, the visible localized project requirement is associated programmatically through `aria-describedby`; Add/Open/Remove therefore expose the same recovery information to assistive technology while remaining keyboard-focusable. For pagination boundaries, the localized first/last-page reason is rendered as an in-document `role="tooltip"` associated through `aria-describedby` with a renderer-owned `useId()` target. The tooltip becomes visually available on pointer hover or keyboard focus. Its hidden state does not intercept pointer input; once hover/focus reveals it, pointer hit testing is enabled and the popup is adjacent to the owning control so the pointer can move continuously onto the explanation without crossing a dead gap. `Escape` dismisses the tooltip and its description reference without moving pointer hover or keyboard focus. Re-entering the control with the pointer or returning keyboard focus makes the currently valid boundary reason available again. Native `title` is not used as the unavailable-state explanation, avoiding a second competing description channel and a keyboard/touch-only gap. + +The Add score control is also action-guarded while an attachment operation is already pending. It remains rendered and exposes `aria-disabled="true"`, while the click boundary rejects duplicate activation so one in-flight attach cannot start a second native picker/storage mutation. + +This document records the accessibility rationale for PR #731 only. It does not claim that keeping every disabled control focusable is universally preferable, and it does not convert source-level behavior into a WCAG conformance claim without current-head browser and assistive-technology evidence. + +## Contract + +- Add/Open/Remove score actions remain keyboard-focusable when no active project exists, so their presence and unavailable state can be discovered. +- `aria-disabled` communicates that an action is currently not operable; the guarded click handler remains the actual fail-closed action boundary. +- Project-bound unavailable actions use `aria-describedby` to point to the visible localized project requirement rather than duplicating hidden recovery copy. +- The Add score action also blocks repeated activation while an attach is already pending; no second bridge request is issued from the guarded branch. +- A boundary page-navigation button remains keyboard-focusable so its presence and unavailable state can be discovered. +- Boundary `aria-describedby` points to a localized `role="tooltip"` explanation only while that exact navigation action is unavailable and the explanation has not been dismissed. +- EN boundary copy states the actual reason: `Already at the first page` / `Already at the last page`. KO uses `첫 번째 페이지입니다` / `마지막 페이지입니다`. +- Pointer hover and keyboard focus reveal the unavailable pagination explanation. Hidden tooltip content starts with `pointer-events-none`; `group-hover`/`group-focus-within` switch it to pointer-active at the same time it becomes visible, so an invisible popup cannot steal input. +- The popup is positioned directly against the control's hover geometry rather than across a margin gap, allowing the pointer to move from the control onto the explanation while the parent hover state remains active. +- Pressing `Escape` removes the author-controlled tooltip and `aria-describedby` reference while focus/hover can remain in place. A later pointer re-entry or keyboard refocus restores the currently valid explanation. +- Enabled pagination controls may keep their ordinary action title but do not retain stale disabled-state descriptions or tooltips. +- Description IDs are renderer-owned and generated with React `useId()`; analysis or file metadata never becomes DOM-ID authority. + +## Verification + +`apps/desktop/src/features/score/ScoreView.disabled-action-accessibility.test.tsx` verifies that Add/Open/Remove controls without an active project remain focusable, carry `aria-disabled`, resolve `aria-describedby` to the visible localized project requirement, expose recovery titles, and reject activation without invoking the desktop bridge. + +`apps/desktop/src/features/score/ScoreView.test.tsx` independently verifies the project-missing guarded branches and the in-flight Add score branch: a repeated click while the first attach promise is pending is prevented and does not issue a second attach request. + +`apps/desktop/src/features/score/ScoreViewer.disabled-navigation-accessibility.test.tsx` verifies both ends of a three-page document. The unavailable Previous action on page 1 and unavailable Next action on page 3 each resolve `aria-describedby` to the reason-specific localized `role="tooltip"`, omit a competing unavailable-state native title, remain focusable, expose focus/hover visibility classes, and require state-dependent pointer hit testing with no margin dead gap. The page-1 contract also presses `Escape`, requires the tooltip and description reference to disappear without navigation, then re-focuses the same control and requires the valid reason to return. + +The RED→fix evidence for the WCAG 1.4.13 repairs is intentionally split: + +- `8e0012d46cc0603a836119e47cc191f462c7dc1b` first required pointer access to the boundary tooltip before `78a5e60a1de7a259c45798959ca497456994fb2f` removed unconditional pointer suppression. +- `b13c3859712f94cb66bbbbe24440f7682b3c47e7` required Escape dismissal before `1a92b71f168a6bec64ad75de2f64f3a6fef4afa5` added dismiss-and-retrigger behavior. +- A further geometry/input review found that unconditional pointer hit testing makes the invisible tooltip an input target and that a visual margin can create a hover dead zone. RED `a150059cf73540078741fbc8e15ab11f65a0893c` requires pointer hit testing only while the popup is revealed and no `mb-2` gap; fix `a941cf176d15d98ed2f31751a61dcb798eb36daa` implements that continuous hover path. + +## Standards and guidance boundary + +WAI-ARIA 1.2 defines `aria-disabled` as conveying a perceivable but disabled state; application code still owns suppression of behavior. WCAG 2.2 Success Criterion 1.4.13 requires author-controlled content triggered by hover or focus to be dismissible when applicable, hoverable when pointer hover triggers it, and persistent while the trigger remains valid. The WAI-ARIA Authoring Practices keyboard guidance explains why focusable disabled controls can be appropriate when discoverability matters, but this remains a design trade-off rather than a universal rule. WCAG Technique ARIA1 documents `aria-describedby` as a mechanism for associating descriptive information with a user-interface control through an in-document ID reference. + +These references define acceptance semantics for the source and browser tests; they do not by themselves establish WCAG conformance, screen-reader interoperability, or certification. + +## References + +World Wide Web Consortium. (2023). *Accessible Rich Internet Applications (WAI-ARIA) 1.2*. https://www.w3.org/TR/wai-aria/ + +World Wide Web Consortium. (2023). *Web Content Accessibility Guidelines (WCAG) 2.2*. https://www.w3.org/TR/WCAG22/#content-on-hover-or-focus + +World Wide Web Consortium, Web Accessibility Initiative. (n.d.). *Developing a keyboard interface*. WAI-ARIA Authoring Practices Guide. Retrieved September 6, 2026, from https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/ + +World Wide Web Consortium, Web Accessibility Initiative. (n.d.). *ARIA1: Using the aria-describedby property to provide a descriptive label for user interface controls*. Techniques for WCAG 2.2. Retrieved September 6, 2026, from https://www.w3.org/WAI/WCAG22/Techniques/aria/ARIA1 \ No newline at end of file