Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .Jules/palette.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,7 @@
## 2024-07-01 - Testing components with focusable disabled button wrappers
**Learning:** When native disabled buttons are wrapped in a focusable `span` to provide accessible tooltips, tests that previously found and clicked the `button` (by temporarily removing the `disabled` attribute) may fail or become overly complex. It is cleaner and more accurate to query the wrapper element (e.g. via its `title`) and fire events on it, reflecting the actual accessible DOM structure.
**Action:** When testing UI components that wrap disabled buttons in a focusable span for accessibility (e.g., using a tooltip/title), use `screen.getByTitle(...)` to query the wrapper element for interactions like `fireEvent.click` rather than `screen.getByRole('button')`.

## 2024-07-02 - Accessible tooltips for disabled native buttons
**Learning:** Wrapped natively disabled buttons in a `span role="button"` to attach a tooltip and bypass the pointer-event block causes invalid HTML and creates nested interactive elements, which harms screen reader accessibility and violates ARIA standards.
**Action:** To make disabled buttons accessible and their tooltips functional, do NOT wrap them in a `span role="button"` or apply `aria-hidden="true"` to the nested button. Instead, use the native `<button>` element, remove the HTML `disabled` and `pointer-events-none` attributes, apply `aria-disabled="true"`, block clicks with `onClick={(e) => e.preventDefault()}`, and place the `title` attribute directly on the button. Ensure visual disabled states are maintained via `opacity` and `cursor-not-allowed`.
26 changes: 19 additions & 7 deletions apps/desktop/src/App.test.tsx
Original file line number Diff line number Diff line change
@@ -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 { beforeEach, describe, expect, it, vi } from "vitest";
import { App } from "./App";

Expand Down Expand Up @@ -1405,8 +1405,10 @@ describe("App", () => {

it("does nothing when Save Project is clicked but there is no jobResult", () => {
render(<App />);
const saveSpan = screen.getByTitle("Analyze a song to enable saving");
fireEvent.click(saveSpan);
const saveButton = screen.getByRole("button", { name: "Save Project" });
const event = createEvent.click(saveButton);
fireEvent(saveButton, event);
expect(event.defaultPrevented).toBe(true);
expect(mockSaveProject).not.toHaveBeenCalled();
});

Expand All @@ -1427,10 +1429,20 @@ describe("App", () => {
});


it("renders disabled Settings and Help buttons as focusable spans for accessibility", () => {
it("renders disabled Settings and Help buttons accessibly without span wrappers", () => {
render(<App />);
const settingsSpan = screen.getByTitle("Settings coming soon");
expect(settingsSpan).toHaveAttribute("tabIndex", "0");
expect(settingsSpan).toHaveAttribute("role", "button");
const settingsButton = screen.getByTitle("Settings coming soon");
expect(settingsButton.tagName).toBe("BUTTON");
expect(settingsButton).toHaveAttribute("aria-disabled", "true");
const settingsEvent = createEvent.click(settingsButton);
fireEvent(settingsButton, settingsEvent);
expect(settingsEvent.defaultPrevented).toBe(true);

const helpButton = screen.getByTitle("Help coming soon");
expect(helpButton.tagName).toBe("BUTTON");
expect(helpButton).toHaveAttribute("aria-disabled", "true");
const helpEvent = createEvent.click(helpButton);
fireEvent(helpButton, helpEvent);
expect(helpEvent.defaultPrevented).toBe(true);
});
});
50 changes: 29 additions & 21 deletions apps/desktop/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -535,18 +535,26 @@ export function App() {
</div>

<div className="flex items-center justify-between text-slate-400">
<span tabIndex={0} role="button" aria-disabled="true" title="Settings coming soon" className="inline-block cursor-not-allowed rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300">
<button
type="button"
title="Settings coming soon"
aria-disabled="true"
onClick={(e) => e.preventDefault()}
className="cursor-not-allowed rounded-xl p-2 text-slate-600 opacity-50 transition focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 aria-disabled:hover:bg-transparent"
>
<span className="sr-only">Settings coming soon</span>
<button type="button" disabled aria-hidden="true" className="pointer-events-none rounded-xl p-2 text-slate-600 transition">
<Settings className="size-5" aria-hidden="true" />
</button>
</span>
<span tabIndex={0} role="button" aria-disabled="true" title="Help coming soon" className="inline-block cursor-not-allowed rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300">
<Settings className="size-5" aria-hidden="true" />
</button>
<button
type="button"
title="Help coming soon"
aria-disabled="true"
onClick={(e) => e.preventDefault()}
className="cursor-not-allowed rounded-xl p-2 text-slate-600 opacity-50 transition focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 aria-disabled:hover:bg-transparent"
>
<span className="sr-only">Help coming soon</span>
<button type="button" disabled aria-hidden="true" className="pointer-events-none rounded-xl p-2 text-slate-600 transition">
<CircleHelp className="size-5" aria-hidden="true" />
</button>
</span>
<CircleHelp className="size-5" aria-hidden="true" />
</button>
</div>
</div>
</aside>
Expand Down Expand Up @@ -646,17 +654,17 @@ export function App() {
Save Project
</Button>
) : (
<span tabIndex={0} role="button" aria-disabled="true" title="Analyze a song to enable saving" className="inline-block cursor-not-allowed rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300">
<Button
disabled
variant="outline"
className="min-h-11 border-white/10 bg-white/5 font-semibold text-slate-100"
aria-label="Save Project"
>
<Save className="mr-2 size-4" aria-hidden="true" />
Save Project
</Button>
</span>
<Button
variant="outline"
title="Analyze a song to enable saving"
aria-disabled="true"
onClick={(e) => e.preventDefault()}
className="min-h-11 cursor-not-allowed border-white/10 bg-white/5 font-semibold text-slate-100 opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 aria-disabled:hover:bg-white/5 aria-disabled:hover:text-slate-100"
aria-label="Save Project"
>
<Save className="mr-2 size-4" aria-hidden="true" />
Save Project
</Button>
)}
<Button
onClick={handleStartAnalysis}
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/features/workspace/Workspace.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe("Workspace", () => {
fireEvent.click(screen.getByRole("tab", { name: "Bass Guitar" }));

const transcribeButton = screen.getByRole("button", { name: "Transcribe Bass" }) as HTMLButtonElement;
expect(transcribeButton.disabled).toBe(false);
expect(transcribeButton.hasAttribute("aria-disabled")).toBe(false);
expect(transcribeButton.title).toBe("Transcribe part");
});

Expand Down
59 changes: 40 additions & 19 deletions apps/desktop/src/features/workspace/Workspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -311,15 +311,36 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp
<p className="text-xs font-black uppercase tracking-[0.24em] text-emerald-200">Stem Player</p>
<p className="mt-1 text-sm font-semibold text-slate-100">{activeRoleDetails?.name ?? activeRole}</p>
<div className="mt-3 flex flex-wrap gap-2">
<span tabIndex={0} role="button" aria-disabled="true" title="Coming soon" className="inline-block cursor-not-allowed rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300">
<Button type="button" disabled variant="outline" className="min-h-11 border-white/10 bg-white/5 text-slate-400">Play stem</Button>
</span>
<span tabIndex={0} role="button" aria-disabled="true" title="Coming soon" className="inline-block cursor-not-allowed rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300">
<Button type="button" disabled variant="outline" className="min-h-11 border-white/10 bg-white/5 text-slate-400">Loop section</Button>
</span>
<span tabIndex={0} role="button" aria-disabled="true" title="Coming soon" className="inline-block cursor-not-allowed rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300">
<Button type="button" disabled variant="outline" className="min-h-11 border-white/10 bg-white/5 text-slate-400">Solo / mute others</Button>
</span>
<Button
type="button"
variant="outline"
title="Coming soon"
aria-disabled="true"
onClick={(e) => e.preventDefault()}
className="min-h-11 cursor-not-allowed border-white/10 bg-white/5 text-slate-400 opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 aria-disabled:hover:bg-white/5 aria-disabled:hover:text-slate-400"
>
Play stem
</Button>
<Button
type="button"
variant="outline"
title="Coming soon"
aria-disabled="true"
onClick={(e) => e.preventDefault()}
className="min-h-11 cursor-not-allowed border-white/10 bg-white/5 text-slate-400 opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 aria-disabled:hover:bg-white/5 aria-disabled:hover:text-slate-400"
>
Loop section
</Button>
<Button
type="button"
variant="outline"
title="Coming soon"
aria-disabled="true"
onClick={(e) => e.preventDefault()}
className="min-h-11 cursor-not-allowed border-white/10 bg-white/5 text-slate-400 opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 aria-disabled:hover:bg-white/5 aria-disabled:hover:text-slate-400"
>
Solo / mute others
</Button>
{canTranscribeBass ? (
<Button
type="button"
Expand All @@ -330,16 +351,16 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp
Transcribe Bass
</Button>
) : (
<span tabIndex={0} role="button" aria-disabled="true" title={`${activeRoleDetails?.name ?? "This role"} transcription is coming soon. Bass is ready first.`} className="inline-block cursor-not-allowed rounded-xl focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300">
<Button
type="button"
disabled
variant="outline"
className="min-h-11 border-emerald-300/20 bg-emerald-300/10 font-semibold text-emerald-100 disabled:border-white/10 disabled:bg-white/5 disabled:text-slate-500"
>
Transcribe Bass
</Button>
</span>
<Button
type="button"
variant="outline"
title={`${activeRoleDetails?.name ?? "This role"} transcription is coming soon. Bass is ready first.`}
aria-disabled="true"
onClick={(e) => e.preventDefault()}
className="min-h-11 cursor-not-allowed border-white/10 bg-white/5 font-semibold text-slate-500 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 aria-disabled:hover:bg-white/5 aria-disabled:hover:text-slate-500"
>
Transcribe Bass
</Button>
)}
</div>
<div className="mt-4 grid gap-3 lg:grid-cols-2">
Expand Down
Loading