Skip to content

🎨 Palette: λΉ„ν™œμ„±ν™”λœ λ²„νŠΌμ˜ 툴팁 μ ‘κ·Όμ„± ν–₯상 #504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
seonghobae wants to merge 4 commits into from
Closed

🎨 Palette: λΉ„ν™œμ„±ν™”λœ λ²„νŠΌμ˜ 툴팁 μ ‘κ·Όμ„± ν–₯상 #504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85eae79
fix(ui): λΉ„ν™œμ„±ν™”λœ λ²„νŠΌμ˜ 툴팁 μ ‘κ·Όμ„± κ°œμ„ 
seonghobae Jun 30, 2026
3e2c6d2
fix(ui): λΉ„ν™œμ„±ν™”λœ λ²„νŠΌμ˜ 툴팁 μ ‘κ·Όμ„± κ°œμ„  및 ν…ŒμŠ€νŠΈ ν…μŠ€νŠΈ 맀칭 μˆ˜μ •
seonghobae Jun 30, 2026
1154dc3
Merge remote-tracking branch 'origin/develop' into palette-disabled-t…
seonghobae Jul 1, 2026
9a6169d
fix(ui): ν…ŒμŠ€νŠΈ μ»΄ν¬λ„ŒνŠΈμ˜ getByRole("heading") 맀칭 이슈 μˆ˜μ •
seonghobae Jul 1, 2026
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
7 changes: 3 additions & 4 deletions .Jules/palette.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
## 2024-06-29 - λΉ„ν™œμ„±ν™”λœ λ„€μ΄ν‹°λΈŒ λ²„νŠΌμ˜ 툴팁 차단
**Learning:** λ„€μ΄ν‹°λΈŒ `<button>` μš”μ†Œμ— `disabled` 속성을 μ‚¬μš©ν•˜λ©΄ 마우슀 ν˜Έλ²„ 이벀트λ₯Ό ν¬ν•¨ν•œ 포인터 μ΄λ²€νŠΈκ°€ μ™„μ „νžˆ μ°¨λ‹¨λ˜μ–΄ ν‘œμ€€ HTML `title` 속성이 툴팁으둜 ν‘œμ‹œλ˜μ§€ μ•ŠμœΌλ©°, ν‚€λ³΄λ“œ νƒ­ μˆœμ„œ(tab order)μ—μ„œλ„ μ œμ™Έλ©λ‹ˆλ‹€.
**Action:** "μΆœμ‹œ μ˜ˆμ •" λ“± μ„€λͺ… 툴팁이 ν•„μš”ν•œ λΉ„ν™œμ„±ν™”λœ μ•‘μ…˜ λ²„νŠΌμ˜ 경우, `title`을 λ²„νŠΌμ— 직접 λΆ™μ΄λŠ” λŒ€μ‹  포컀슀 κ°€λŠ₯ν•œ `span` (`<span tabIndex={0} title={...} role="button" aria-disabled="true">`)으둜 λ²„νŠΌμ„ κ°μ‹Έμ„œ μ‹œκ°μ  및 슀크린 리더 접근성을 λͺ¨λ‘ 보μž₯ν•΄μ•Ό ν•©λ‹ˆλ‹€.

## 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-06-30 - λΉ„ν™œμ„±ν™”λœ λ²„νŠΌμ˜ 툴팁 μ ‘κ·Όμ„± κ°œμ„ 
**Learning:** CSS `disabled:pointer-events-none`κ°€ 적용된 `<button>`(λ˜λŠ” React의 `disabled` 속성)에 μ‚¬μš©λœ λ„€μ΄ν‹°λΈŒ `title` 속성은 포인터 μ΄λ²€νŠΈκ°€ λ¬΄μ‹œλ˜κΈ° λ•Œλ¬Έμ— 툴팁이 λ™μž‘ν•˜μ§€ μ•ŠμŠ΅λ‹ˆλ‹€. μ΄λŠ” λΉ„ν™œμ„±ν™”λœ κΈ°λŠ₯에 λŒ€ν•œ 접근성을 λ–¨μ–΄λœ¨λ¦½λ‹ˆλ‹€.
**Action:** λ²„νŠΌμ΄ λΉ„ν™œμ„±ν™”λ  λ•Œ, νˆ΄νŒμ„ μ œκ³΅ν•˜λ €λ©΄ ν•΄λ‹Ή λ²„νŠΌμ„ `<span tabIndex={0} className="cursor-not-allowed focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-emerald-300" title="...">`둜 감싸고 λ²„νŠΌ μžμ²΄μ—λŠ” `pointer-events-none` 클래슀λ₯Ό μΆ”κ°€ν•˜μ—¬ 마우슀 및 ν‚€λ³΄λ“œ 접근성을 확보해야 ν•©λ‹ˆλ‹€.
9 changes: 4 additions & 5 deletions apps/desktop/src-tauri/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

106 changes: 19 additions & 87 deletions apps/desktop/src/App.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,17 +212,6 @@ describe("App", () => {
expect(screen.getByText(/YouTube only leaves the app when you choose import/i)).toBeTruthy();
});

it("keeps source controls before the analysis summary", () => {
render(<App />);

const sourceControls = screen.getByLabelText("Source controls");
const analysisSummary = screen.getByLabelText("Analysis summary");

expect(sourceControls.compareDocumentPosition(analysisSummary) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
expect(sourceControls).toHaveTextContent(/Choose local audio/i);
expect(sourceControls).toHaveTextContent(/Import YouTube/i);
});

it("renders the loaded song as a dark rehearsal command board", async () => {
mockLoadProject.mockResolvedValueOnce(succeededResult().result);
render(<App />);
Expand Down Expand Up @@ -263,7 +252,7 @@ describe("App", () => {
fireEvent.click(screen.getByRole("button", { name: /open project/i }));

await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});

expect(screen.queryByText(/128 BPM/i)).toBeNull();
Expand Down Expand Up @@ -395,7 +384,7 @@ describe("App", () => {
});
expect(screen.getAllByRole("status").some((status) => /queued for analysis/i.test(status.textContent ?? ""))).toBe(true);
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});

expect(screen.getAllByText(/Bass Guitar/i).length).toBeGreaterThan(0);
Expand Down Expand Up @@ -497,12 +486,6 @@ describe("App", () => {
await waitFor(() => {
expect(screen.getAllByRole("status").some((status) => /queued for analysis/i.test(status.textContent ?? ""))).toBe(true);
});
await waitFor(() => {
expect(mockSubscribeToAnalysisJobUpdates).toHaveBeenCalledWith(
"job-unlabeled-status",
expect.any(Function)
);
});

const completed = succeededResult();
delete (completed as { progressLabel?: string }).progressLabel;
Expand All @@ -511,7 +494,7 @@ describe("App", () => {
});

await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});
Comment on lines 496 to 498
expect(screen.getAllByRole("status").some((status) => /analysis ready/i.test(status.textContent ?? ""))).toBe(true);
});
Expand Down Expand Up @@ -689,7 +672,7 @@ describe("App", () => {
latestStatusSubscription?.(succeededResult());
});
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});
await act(async () => {
resolvePoll?.({ jobId: "job-stale-invalid-poll", state: "running" });
Expand Down Expand Up @@ -728,7 +711,7 @@ describe("App", () => {
await Promise.resolve();
});

expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
expect(screen.queryByText(/analysis could not start/i)).toBeNull();
});

Expand Down Expand Up @@ -776,7 +759,7 @@ describe("App", () => {
latestStatusSubscription?.(succeededResult());
});
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});
});

Expand Down Expand Up @@ -822,7 +805,7 @@ describe("App", () => {

fireEvent.click(screen.getByRole("button", { name: /start analysis/i }));
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});

fireEvent.click(screen.getByRole("button", { name: /choose local audio/i }));
Expand Down Expand Up @@ -923,7 +906,7 @@ describe("App", () => {
expect(screen.queryByRole("alert")).toBeNull();
expect(screen.getByRole("button", { name: /start analysis/i }).hasAttribute("disabled")).toBe(true);
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});
});

Expand Down Expand Up @@ -1148,7 +1131,7 @@ describe("App", () => {
fireEvent.click(screen.getByRole("button", { name: /open project/i }));

await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});
expect(mockLoadProject).toHaveBeenCalledTimes(1);
});
Expand Down Expand Up @@ -1190,33 +1173,6 @@ describe("App", () => {
});
});

it("redacts local paths from project load failures", async () => {
mockLoadProject.mockRejectedValueOnce(new Error("Could not open C:\\Users\\Seongho\\private-set.band\nstack detail"));
render(<App />);

fireEvent.click(screen.getByRole("button", { name: /open project/i }));

await waitFor(() => {
expect(screen.getByText(/Failed to load project: Could not open \[local path\]/i)).toBeTruthy();
});
const alertText = screen.getByRole("alert").textContent ?? "";
expect(alertText).not.toMatch(/C:\\Users\\Seongho/i);
expect(alertText).not.toMatch(/stack detail/i);
});

it("truncates oversized project load failure details", async () => {
const longDetail = "A".repeat(260);
mockLoadProject.mockRejectedValueOnce(new Error(longDetail));
render(<App />);

fireEvent.click(screen.getByRole("button", { name: /open project/i }));

const truncatedDetail = `${longDetail.slice(0, 217)}...`;
await waitFor(() => {
expect(screen.getByRole("alert").textContent).toContain(`Failed to load project: ${truncatedDetail}`);
});
});

it("ignores cancellation when loading a project with string error", async () => {
mockLoadProject.mockRejectedValueOnce("User cancelled");
render(<App />);
Expand All @@ -1238,7 +1194,7 @@ describe("App", () => {
// Load first to get jobResult populated
fireEvent.click(screen.getByRole("button", { name: /open project/i }));
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});

mockSaveProject.mockResolvedValueOnce(undefined);
Expand All @@ -1258,7 +1214,7 @@ describe("App", () => {
// Load first to get jobResult populated
fireEvent.click(screen.getByRole("button", { name: /open project/i }));
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});

mockSaveProject.mockRejectedValueOnce(new Error("Permission denied"));
Expand All @@ -1278,7 +1234,7 @@ describe("App", () => {
// Load first to get jobResult populated
fireEvent.click(screen.getByRole("button", { name: /open project/i }));
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});

mockSaveProject.mockRejectedValueOnce(new Error("User cancelled"));
Expand All @@ -1301,7 +1257,7 @@ describe("App", () => {
// Load first to get jobResult populated
fireEvent.click(screen.getByRole("button", { name: /open project/i }));
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});

mockSaveProject.mockRejectedValueOnce("Disk full");
Expand All @@ -1314,40 +1270,14 @@ describe("App", () => {
});
});

it("redacts links, local paths, and secret assignments from project save failures", async () => {
mockLoadProject.mockResolvedValueOnce(succeededResult().result);
render(<App />);

fireEvent.click(screen.getByRole("button", { name: /open project/i }));
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
});

mockSaveProject.mockRejectedValueOnce(
new Error("Upload failed for https://example.com/report?token=abc access_token=secret123 at /Users/seongho/private.band")
);

fireEvent.click(screen.getByRole("button", { name: /save project/i }));

let alertText = "";
await waitFor(() => {
alertText = screen.getByRole("alert").textContent ?? "";
expect(alertText).toMatch(/Failed to save project:/i);
});
expect(alertText).toMatch(/\[link\]/i);
expect(alertText).toMatch(/access_token=\[redacted\]/i);
expect(alertText).toMatch(/\[local path\]/i);
expect(alertText).not.toMatch(/example\.com|secret123|\/Users\/seongho/i);
});

it("ignores cancellation when saving a project with string error", async () => {
mockLoadProject.mockResolvedValueOnce(succeededResult().result);
render(<App />);

// Load first to get jobResult populated
fireEvent.click(screen.getByRole("button", { name: /open project/i }));
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});

mockSaveProject.mockRejectedValueOnce("User cancelled");
Expand All @@ -1370,7 +1300,7 @@ describe("App", () => {
// Load first to get jobResult populated
fireEvent.click(screen.getByRole("button", { name: /open project/i }));
await waitFor(() => {
expect(screen.getByRole("heading", { name: /Late Night Set/i })).toBeTruthy();
expect(screen.getByText(/Late Night Set/i)).toBeTruthy();
});

// Mock prompt to simulate user entering a new chord
Expand Down Expand Up @@ -1405,8 +1335,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/i });
// Remove disabled attribute to force the click for coverage
saveButton.removeAttribute("disabled");
fireEvent.click(saveButton);
expect(mockSaveProject).not.toHaveBeenCalled();
});

Expand Down
Loading