-
Notifications
You must be signed in to change notification settings - Fork 0
feat(workspace): name tonight's first vamp plan on the map #1024
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.
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
base: develop
Are you sure you want to change the base?
Changes from all commits
e124451
c701b82
e665ae0
a6fbd9c
dbf692a
d349735
6ef3dd6
2dfb27a
cb9c6ff
94c091c
bc23aef
7268ae7
3e0c441
bdc7791
0f20bc6
335c5db
49be3fe
198f4fa
95d59b4
addb963
c03001e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,43 @@ | ||
| ## 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-05-18 - Added focus visible styles for keyboard navigation | ||
| **Learning:** Interactive inline buttons (like the chord editor) and scrollable regions with `tabIndex={0}` do not automatically get focus visible styles, meaning keyboard users tabbing through won't know they are focused on them. Unlike central `<Button />` components which bake focus states in, these custom inline interactive elements need explicit focus styling. | ||
| **Action:** Always add explicit focus visible styles (e.g., `focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300`) to custom interactive elements and scrollable regions with `tabIndex={0}` for proper keyboard accessibility. | ||
|
|
||
| ## 2024-05-24 - Visual tooltips for disabled icon-only buttons | ||
| **Learning:** Icon-only buttons with `aria-label` are accessible to screen readers, but sighted users relying on mouse hover don't get context if the `title` attribute is missing, especially when the button is disabled and its purpose is unclear (e.g. "coming soon"). | ||
| **Action:** Always add a `title` attribute mirroring the `aria-label` (or providing a specific disabled reason) to icon-only buttons so sighted users also receive explanatory tooltips on hover. | ||
|
|
||
| ## 2026-06-13 - Added screen reader text for tooltip divs | ||
| **Learning:** When using `title` attributes on non-interactive elements like icon-only `div`s for tooltips, screen readers might not announce them properly because they aren't focusable. The visual tooltip is not enough for accessibility. | ||
| **Action:** Always add a visually hidden `<span className="sr-only">[Tooltip Text]</span>` inside non-interactive elements that rely on a `title` attribute so that screen readers have text content to announce. | ||
|
|
||
| ## 2026-06-18 - Added keyboard accessibility to scrollable regions | ||
| **Learning:** Horizontally scrollable regions (like the `SectionRoadmap` component) are not accessible to keyboard-only users unless they can receive focus. Keyboard users must be able to focus the container to scroll its content using arrow keys. | ||
| **Action:** For proper keyboard accessibility in custom scrollable regions, always include `tabIndex={0}`, an appropriate `aria-label`, `role="region"`, and explicit focus visible styling (e.g., `focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300`). | ||
|
|
||
| ## 2026-06-19 - Internationalization | ||
| **Learning:** The desktop app uses i18n via json files located in `apps/desktop/src/locales/` | ||
| **Action:** When adding new text strings, make sure to add it to all locale files. | ||
|
|
||
| ## 2026-06-25 - Native tooltips on disabled elements | ||
| **Learning:** Standard HTML `title` attributes used as tooltips do not render on elements that use Tailwind's `pointer-events-none` class, which is often applied to `disabled:` variants in Base UI and styled components. | ||
| **Action:** Do not rely on native `title` attributes for explaining disabled states on buttons with `pointer-events-none`. Instead, either use a custom tooltip component or ensure focus/interactive styles are preserved if an explanation is strictly required. | ||
|
|
||
| ## 2024-06-29 - 비활성화된 네이티브 버튼의 툴팁 차단 | ||
| **Learning:** 네이티브 `<button>` 요소에 `disabled` 속성을 사용하면 마우스 호버 이벤트를 포함한 포인터 이벤트가 완전히 차단되어 표준 HTML `title` 속성이 툴팁으로 표시되지 않으며, 키보드 탭 순서(tab order)에서도 제외됩니다. | ||
| **Action:** "출시 예정" 등 설명 툴팁이 필요한 비활성화된 액션 버튼의 경우, `title`을 버튼에 직접 붙이는 대신 포커스 가능한 `span` (`<span tabIndex={0} title={...} role="button" aria-disabled="true">`)으로 버튼을 감싸서 시각적 및 스크린 리더 접근성을 모두 보장해야 합니다. | ||
|
Comment on lines
+29
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- applicable repository conventions ---'
for f in /tmp/coderabbit-repo-knowledge/contextualwisdomlab-bandscope-65d7df2c/*/*.md; do
case "$f" in
*/learnings/*) continue ;;
esac
printf '\n### %s\n' "$f"
head -80 "$f"
done
printf '%s\n' '--- target file ---'
cat -n .jules/palette.md | sed -n '1,60p'
printf '%s\n' '--- relevant guidance occurrences ---'
rg -n -C 3 'role="button"|aria-disabled|tabIndex|disabled|툴팁|wrapper|button' .jules/palette.mdRepository: ContextualWisdomLab/bandscope Length of output: 18659 🌐 Web query:
💡 Result: When a HTML button uses the native disabled attribute, it becomes non-interactive and inaccessible in several specific ways [1]: Focusability: A button with the native disabled attribute cannot receive focus [1]. It is removed from the tab order, meaning keyboard users cannot navigate to it, and screen reader users cannot discover it via standard keyboard navigation [1][2]. Accessibility Tree and Screen Readers: Because disabled buttons are not focusable, they are generally not announced by screen readers when a user navigates through a page using the Tab key [2]. This can obscure the existence of the button from users who rely on assistive technology [3]. Tooltip (title attribute): While native disabled elements previously did not display title attribute tooltips, modern browsers have updated this behavior [4]. In modern browsers, hovering over a disabled button with a title attribute will typically display the tooltip [4]. However, because the element cannot receive focus, keyboard-only users will not be able to trigger this tooltip [1][4]. Alternative (aria-disabled): To improve accessibility when a button must be "disabled" but still discoverable, developers often use aria-disabled="true" [5][6]. Unlike the native disabled attribute, elements with aria-disabled remain focusable and continue to exist in the accessibility tree, allowing screen readers to announce them as "disabled" or "dimmed" [5][6]. When using this approach, developers must manually manage the element's functionality (e.g., suppressing click events in JavaScript) and ensure the element is focusable [5][6]. Using aria-disabled is often recommended when you want users to discover the button and potentially understand why it is currently unavailable [5][6]. Citations:
비활성 버튼의 접근성 설명과 wrapper 예시를 수정하세요.
🤖 Prompt for AI Agents |
||
|
|
||
| ## 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-05-24 - Avoid nesting native buttons with ARIA role button on wrappers | ||
| **Learning:** Adding `role="button"` to a `span` or `div` wrapper that contains a native `<button>` element inside violates ARIA specifications. Interactive roles (like `button`) must not contain other interactive elements (even if the inner element is disabled or has `aria-hidden`), as this causes invalid/redundant accessibility trees and screen reader confusion. | ||
| **Action:** Always verify wrappers used to implement tooltips for disabled buttons are standard elements (e.g., `<span tabIndex={0} title="...">`) but *do not* assign `role="button"` to the wrapper itself. | ||
|
|
||
| ## 2026-07-02 - Inline clear buttons preserve focus | ||
| **Learning:** Inline clear buttons often unmount immediately after clearing state, which can drop keyboard focus to the document body. | ||
| **Action:** Move focus back to the owning input before clearing state, and cover the behavior with a DOM focus test. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,7 @@ This section applies to any agent (Claude, Codex, Cursor, opencode, ...) working | |
| - Keep UI and analysis engine decoupled through shared contracts. | ||
| - Prefer minimal, test-first changes for production code. | ||
| - Prefer practical, friendly, rehearsal-first wording over academic or authority-heavy language. | ||
| - Name tonight's first vamp plan with the owning part when an active role is corroborated, the owned `vampPlan` copy, the labeled section, and the time so the next action is obvious. Do not invent that copy from groove, cue, simplification, overlap, range, chord labels, function labels, setup notes, transposition plans, fill plans, tuning plans, dynamics plans, articulation plans, hook plans, solo plans, pad plans, confirmed overrides, harmonic explanations, or confidence notes. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win 첫 vamp plan의 fail-closed 생성 조건을 모든 계약 문서에 동일하게 기록하세요. 동일한 role이 section boundary를 유지하고 정확히 하나의 다른 role만 다음 section에서 진입할 때만 계획을 생성해야 합니다. ambiguous, unknown, heuristic-only evidence에서는 unavailable 상태를 유지해야 합니다.
📍 Affects 4 files
🤖 Prompt for AI Agents |
||
| - Do not reduce the product to a chord analyzer when form, timing, player coordination, playable ranges, simplification, and setup cues are the real rehearsal blockers. | ||
| - Do not frame usability as a reason to accept weak analysis quality; BandScope should aim for both easy use and high accuracy. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import { render, screen } from "@testing-library/react"; | ||
| import { createDemoRehearsalSong } from "@bandscope/shared-types"; | ||
| import { expect, it } from "vitest"; | ||
| import { FirstVampPlanCallout } from "./FirstVampPlanCallout"; | ||
|
|
||
| it("gives co-mounted vamp-plan callouts distinct DOM identities", () => { | ||
| render( | ||
| <> | ||
| <FirstVampPlanCallout song={createDemoRehearsalSong()} /> | ||
| <FirstVampPlanCallout song={createDemoRehearsalSong()} /> | ||
| </> | ||
| ); | ||
|
|
||
| const callouts = screen.getAllByRole("complementary", { | ||
| name: "Tonight's first vamp plan" | ||
| }); | ||
| const ids = callouts.map((callout) => callout.id); | ||
|
|
||
| expect(ids.every((id) => id.length > 0)).toBe(true); | ||
| expect(new Set(ids).size).toBe(callouts.length); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| import { render } from "@testing-library/react"; | ||
| import { createDemoRehearsalSong } from "@bandscope/shared-types"; | ||
| import { afterEach, describe, expect, it, vi } from "vitest"; | ||
| import { FirstVampPlanCallout } from "./FirstVampPlanCallout"; | ||
|
|
||
| describe("FirstVampPlanCallout resolver reuse", () => { | ||
| afterEach(() => { | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it("does not rescan role metadata when a parent rerenders the same song object", () => { | ||
| const song = createDemoRehearsalSong(); | ||
| const role = song.sections[0]!.roles.find((candidate) => candidate.id === "lead-vocal")!; | ||
| const descriptorSpy = vi.spyOn(Object, "getOwnPropertyDescriptor"); | ||
|
|
||
| const { rerender } = render(<FirstVampPlanCallout song={song} />); | ||
| const firstScanCount = descriptorSpy.mock.calls.filter(([target]) => target === role).length; | ||
| expect(firstScanCount).toBeGreaterThan(0); | ||
|
|
||
| rerender(<FirstVampPlanCallout song={song} />); | ||
| const secondScanCount = descriptorSpy.mock.calls.filter(([target]) => target === role).length; | ||
|
|
||
| expect(secondScanCount).toBe(firstScanCount); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: ContextualWisdomLab/bandscope
Length of output: 13783
🌐 Web query:
HTML disabled controls accessibility tree screen readers tab order pointer hover events authoritative MDN WHATWG💡 Result:
When the HTML disabled attribute is applied to a form control, it fundamentally alters the element's behavior across multiple layers of the browser, including the accessibility tree, focus order, and event handling [1][2]. Accessibility Tree and Tab Order Disabled form controls are effectively removed from the page's sequential keyboard navigation (tab order) [1][3]. Because they are not focusable, they are typically excluded from the tab sequence, preventing keyboard users from landing on them [1][4]. In the accessibility tree, these elements are marked as disabled, signaling to assistive technologies (such as screen readers) that the element is not currently operable or editable [5][6]. Pointer and Hover Events The interaction behavior of disabled elements is strictly limited: 1. Event Dispatching: The WHATWG HTML Standard mandates that disabled form controls must prevent any click events queued on the user interaction task source from being dispatched [2]. 2. Pointer Events: While the standard is explicit about click events, the behavior regarding pointer events (e.g., pointerdown, pointermove) has historically been inconsistent across browser engines [7][8]. Many browsers historically prevented pointer events from firing on disabled elements to mirror legacy mouse event behavior, though this is not consistently enforced as a universal spec requirement across all event types [7][8]. 3. Hover Events: Because disabled elements are generally treated as non-interactive, they typically do not trigger hover-related states or events in the same way as enabled elements, and browsers often apply default "greyed-out" styling to visually communicate this state [1][4]. Comparison with aria-disabled The native disabled attribute provides comprehensive browser-level handling, including removing the element from focus order and preventing activation [1][5]. In contrast, the aria-disabled="true" attribute only communicates the disabled state to assistive technologies; it does not automatically make the element non-focusable, nor does it disable the element's functionality or interactivity [5][9]. Developers using aria-disabled must manually implement logic to prevent focus, keyboard interaction, and pointer events if they wish to replicate the behavior of a natively disabled control [5][9]. Authoritative Guidance For form controls, the native disabled attribute is the authoritative and recommended approach because it provides built-in accessibility, prevents form submission, and enforces non-interactivity [5][9]. The inert attribute is also available for disabling entire subtrees, though it is intended for cases where you want to render content non-interactive and non-perceivable in a more global sense than the granular disabled attribute [3].
Citations:
스크린 리더 동작 설명을 정확히 수정하세요.
네이티브
disabled컨트롤은 스크린 리더에서 완전히 숨겨지지 않습니다. 접근성 트리에 남아 disabled 상태로 전달될 수 있습니다. 대신 탭 순서에서 제외되고 활성화 이벤트가 차단됩니다. 포인터 이벤트는 브라우저와 이벤트 유형에 따라 다르므로 “모든 포인터 및 hover 이벤트를 차단한다”고 일반화하지 마세요.🤖 Prompt for AI Agents