From 02b2a7dc61c9cf70acbf0b6c22aea2673851cde7 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:19:21 +0000 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=8E=A8=20Palette:=20ScoreViewer=20?= =?UTF-8?q?=EC=95=84=EC=9D=B4=EC=BD=98=20=EB=B2=84=ED=8A=BC=20=EC=A0=91?= =?UTF-8?q?=EA=B7=BC=EC=84=B1=20=EA=B0=9C=EC=84=A0=20=EB=B0=8F=20=ED=88=B4?= =?UTF-8?q?=ED=8C=81=20=EC=B6=94=EA=B0=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/palette.md | 3 +++ .../src/features/score/ScoreViewer.test.tsx | 25 +++++++++++++++---- .../src/features/score/ScoreViewer.tsx | 21 +++++++++++++--- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/.jules/palette.md b/.jules/palette.md index c05638899..83b776478 100644 --- a/.jules/palette.md +++ b/.jules/palette.md @@ -1,3 +1,6 @@ ## 2024-05-19 - Replace HTML disabled with aria-disabled="true" for Accessible Tooltips **Learning:** Native HTML `disabled` attributes completely hide elements from screen readers and block all pointer/hover events, preventing tooltips from functioning for disabled elements. **Action:** Replace `disabled` with `aria-disabled="true"`, enforce block click handlers via `e.preventDefault()`, and add a title tooltip directly to the element to maintain full tooltip accessibility and keyboard focus support for visually impaired and mouse users. +## 2024-07-28 - Validate `aria-disabled` styling on UI components +**Learning:** When switching from native `disabled` to `aria-disabled` for better tooltip support and screen reader context, it's crucial to verify if the UI component definitions (e.g., using `cva` in Tailwind) actually support the `aria-disabled:` variants. In `button.tsx`, `aria-disabled:opacity-50` and `aria-disabled:cursor-not-allowed` were already present, ensuring visual regressions didn't occur. Wrapping elements in `` is not a good practice as it creates invalid nested interactive elements. +**Action:** Always inspect the underlying CSS utility variants (like `cva` configurations) to ensure `aria-disabled:hover` and `aria-disabled:opacity` are explicitly handled when updating accessible disabled states. diff --git a/apps/desktop/src/features/score/ScoreViewer.test.tsx b/apps/desktop/src/features/score/ScoreViewer.test.tsx index 3ac2dd605..d8a0faa00 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 previousButton = screen.getByRole("button", { name: "Previous page" }); + expect(previousButton).toHaveAttribute("aria-disabled", "true"); + + const previousClickEvent = createEvent.click(previousButton); + fireEvent(previousButton, previousClickEvent); + expect(previousClickEvent.defaultPrevented).toBe(true); + + expect(screen.getByRole("button", { name: "Next page" })).not.toHaveAttribute("aria-disabled", "true"); }); it("shows the file name when provided", async () => { @@ -174,14 +181,22 @@ 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"); + const previousClickEvent = createEvent.click(previousButton); + fireEvent(previousButton, previousClickEvent); + expect(previousClickEvent.defaultPrevented).toBe(true); 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"); + const nextClickEvent = createEvent.click(nextButton); + fireEvent(nextButton, nextClickEvent); + expect(nextClickEvent.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..24a54e8f4 100644 --- a/apps/desktop/src/features/score/ScoreViewer.tsx +++ b/apps/desktop/src/features/score/ScoreViewer.tsx @@ -152,12 +152,20 @@ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps }, [status, pdfDocument, pageNumber, zoom, fitWidth, containerWidth]); /** Move to the previous page, clamped at the first page. */ - const goToPreviousPage = () => { + const goToPreviousPage = (e: React.MouseEvent) => { + if (pageNumber <= 1) { + e.preventDefault(); + return; + } setPageNumber((current) => Math.max(1, current - 1)); }; /** Move to the next page, clamped at the last page. */ - const goToNextPage = () => { + const goToNextPage = (e: React.MouseEvent) => { + if (pageNumber >= pageCount) { + e.preventDefault(); + return; + } setPageNumber((current) => Math.min(pageCount, current + 1)); }; @@ -258,6 +266,7 @@ export function ScoreViewer({ data, fileName, onStatusChange }: ScoreViewerProps size="icon-lg" className="size-12" aria-label={t("scoreViewerZoomOut")} + title={t("scoreViewerZoomOut")} onClick={zoomOut} >