-
Notifications
You must be signed in to change notification settings - Fork 0
feat(workspace): guide tonight's first dropout on map and player #914
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
Open
seonghobae
wants to merge
40
commits into
develop
Choose a base branch
from
feat/workspace-first-dropout-handoff
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
851964d
feat(workspace): name tonight's first dropout on the map
seonghobae 7df6d90
test(workspace): distinguish dropout navigation from playback
seonghobae 2e7baaa
fix(workspace): label dropout map navigation honestly
seonghobae 26ac05e
feat(i18n): distinguish dropout navigation from playback
seonghobae 6a852b7
feat(i18n): localize dropout map navigation
seonghobae 3decb43
docs(workspace): distinguish dropout navigation from playback
seonghobae 9ddd273
docs(workspace): align dropout action architecture copy
seonghobae bd7e3b1
test(workspace): decouple dropout navigation from analysis ids
seonghobae b60e0f4
fix(workspace): use renderer-owned dropout navigation targets
seonghobae f56bed7
fix(workspace): keep dropout analysis ids out of DOM authority
seonghobae d93ab2d
test(workspace): align dropout map contracts with current behavior
seonghobae 15f68b7
test(workspace): reject cross-section dropout targets
seonghobae 6f3a8fc
fix(workspace): keep dropout handoffs section-local
seonghobae f2dab08
test(workspace): ignore inactive dropout nodes
seonghobae 6f02d69
fix(workspace): ignore inactive dropout nodes
seonghobae f58f67e
test(workspace): reject inconsistent dropout handoffs
seonghobae 91ae7d4
test(workspace): keep dropout action mode authoritative
seonghobae ed8c0de
test(workspace): model reciprocal dropout graph evidence
seonghobae 3d02d98
fix(workspace): require reciprocal dropout handoffs
seonghobae c3cfbf7
fix(workspace): keep dropout action mode authoritative
seonghobae f2a7b73
docs(workspace): define dropout action authority
seonghobae c28ca04
test: reject non-boolean dropout activity flags
seonghobae 99c8bd4
fix: require boolean dropout activity evidence
seonghobae a6ae351
test: reject malformed dropout role ids
seonghobae 55ac6da
fix: reject malformed dropout role ids
seonghobae 4227399
test(workspace): reject ambiguous dropout identities
seonghobae 5ded223
fix(workspace): fail closed on ambiguous dropout identities
seonghobae c711dbd
test(workspace): keep dropout fixture identities unique
seonghobae 7c14a93
test(workspace): reject malformed dropout evidence
seonghobae 9ddb70a
fix(workspace): contain malformed dropout evidence
seonghobae 85e4dd5
refactor(workspace): preserve valid dropout edges
seonghobae b6b7ae7
test(workspace): require executed dropout actions
seonghobae 6c602f8
feat(i18n): localize section form labels
seonghobae 745008c
fix(workspace): arm dropout only after action
seonghobae 64fbd94
test(workspace): lock first-dropout repair contracts
seonghobae c234b6c
test(workspace): model successful map navigation in integration
seonghobae 73edbfe
fix(workspace): ignore malformed handoff entries safely
seonghobae a0b5fe9
fix(i18n): remove unsafe Korean role particles
seonghobae dd72cc1
Merge remote-tracking branch 'origin/develop' into HEAD
seonghobae 550dbd6
Merge remote-tracking branch 'origin/develop' into HEAD
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { fireEvent, render, screen } from "@testing-library/react"; | ||
| import { createDemoRehearsalSong } from "@bandscope/shared-types"; | ||
| import { describe, expect, it, vi } from "vitest"; | ||
| import { PlayerFeature } from "./index"; | ||
|
|
||
| describe("PlayerFeature", () => { | ||
| it("asks the room to analyze first when no song is loaded", () => { | ||
| render(<PlayerFeature title="Player" />); | ||
| expect( | ||
| screen.getByText("Analyze tonight's song first, then hear the first dropout from this player.") | ||
| ).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("keeps the dropout hear action unavailable without a player playback callback", () => { | ||
| render(<PlayerFeature title="Player" song={createDemoRehearsalSong()} />); | ||
|
|
||
| expect( | ||
| screen.queryByRole("button", { | ||
| name: "Hear Bass Guitar drop out for Lead Vocal at 0:30" | ||
| }) | ||
| ).toBeNull(); | ||
| expect(screen.getByText("Bass Guitar hands off to Lead Vocal at the end of the verse (0:30).")).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("delegates the dropout hear action to the owning player callback", () => { | ||
| const onPlayFromSeconds = vi.fn(); | ||
| render( | ||
| <PlayerFeature | ||
| title="Player" | ||
| song={createDemoRehearsalSong()} | ||
| onPlayFromSeconds={onPlayFromSeconds} | ||
| /> | ||
| ); | ||
|
|
||
| fireEvent.click( | ||
| screen.getByRole("button", { | ||
| name: "Hear Bass Guitar drop out for Lead Vocal at 0:30" | ||
| }) | ||
| ); | ||
|
|
||
| expect(onPlayFromSeconds).toHaveBeenCalledTimes(1); | ||
| expect(onPlayFromSeconds).toHaveBeenCalledWith(30); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
174 changes: 174 additions & 0 deletions
174
apps/desktop/src/features/workspace/FirstDropoutCallout.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| import { fireEvent, render, screen } from "@testing-library/react"; | ||
| import { | ||
| createDemoRehearsalSong, | ||
| type RehearsalSong | ||
| } from "@bandscope/shared-types"; | ||
| import { afterEach, describe, expect, it, vi } from "vitest"; | ||
| import { FirstDropoutCallout } from "./FirstDropoutCallout"; | ||
|
|
||
| function appendSongStructureTarget() { | ||
| const grid = document.createElement("div"); | ||
| grid.dataset.testid = "song-structure-grid"; | ||
| const target = document.createElement("div"); | ||
| const scrollIntoView = vi.fn(); | ||
| Object.defineProperty(target, "scrollIntoView", { | ||
| configurable: true, | ||
| value: scrollIntoView | ||
| }); | ||
| grid.appendChild(target); | ||
| document.body.appendChild(grid); | ||
| return { grid, scrollIntoView }; | ||
| } | ||
|
|
||
| describe("FirstDropoutCallout", () => { | ||
| afterEach(() => { | ||
| vi.unstubAllGlobals(); | ||
| }); | ||
|
|
||
| it("names the first dropout as map navigation, scrolls to its rendered section, and arms that action", () => { | ||
| const { grid, scrollIntoView } = appendSongStructureTarget(); | ||
|
|
||
| render(<FirstDropoutCallout song={createDemoRehearsalSong()} />); | ||
|
|
||
| const action = screen.getByRole("button", { | ||
| name: "Open Bass Guitar dropout for Lead Vocal at 0:30" | ||
| }); | ||
| expect(action).toBeTruthy(); | ||
| fireEvent.click(action); | ||
| expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); | ||
| expect(screen.getByText(/Start the last bar of Bass Guitar before Lead Vocal takes the verse \(0:30\)/)).toBeTruthy(); | ||
|
|
||
| grid.remove(); | ||
| }); | ||
|
|
||
| it("does not claim map navigation completed when the rendered section target is missing", () => { | ||
| render(<FirstDropoutCallout song={createDemoRehearsalSong()} />); | ||
|
|
||
| fireEvent.click( | ||
| screen.getByRole("button", { | ||
| name: "Open Bass Guitar dropout for Lead Vocal at 0:30" | ||
| }) | ||
| ); | ||
|
|
||
| expect( | ||
| screen.getByText("Bass Guitar hands off to Lead Vocal at the end of the verse (0:30).") | ||
| ).toBeTruthy(); | ||
| expect( | ||
| screen.queryByText(/Start the last bar of Bass Guitar before Lead Vocal takes the verse \(0:30\)/) | ||
| ).toBeNull(); | ||
| }); | ||
|
|
||
| it("keeps workspace-scroll authoritative even when a playback callback is also supplied", () => { | ||
| const { grid, scrollIntoView } = appendSongStructureTarget(); | ||
| const onHearDropout = vi.fn(); | ||
|
|
||
| render( | ||
| <FirstDropoutCallout | ||
| song={createDemoRehearsalSong()} | ||
| actionMode="workspace-scroll" | ||
| onHearDropout={onHearDropout} | ||
| /> | ||
| ); | ||
|
|
||
| fireEvent.click( | ||
| screen.getByRole("button", { | ||
| name: "Open Bass Guitar dropout for Lead Vocal at 0:30" | ||
| }) | ||
| ); | ||
| expect(onHearDropout).not.toHaveBeenCalled(); | ||
| expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); | ||
|
|
||
| grid.remove(); | ||
| }); | ||
|
|
||
| it("navigates by renderer-owned section position instead of untrusted analysis ids", () => { | ||
| const song = createDemoRehearsalSong(); | ||
| song.sections[0]!.id = "analysis section / duplicate"; | ||
| const { grid, scrollIntoView } = appendSongStructureTarget(); | ||
|
|
||
| render(<FirstDropoutCallout song={song} />); | ||
|
|
||
| fireEvent.click( | ||
| screen.getByRole("button", { | ||
| name: "Open Bass Guitar dropout for Lead Vocal at 0:30" | ||
| }) | ||
| ); | ||
| expect(scrollIntoView).toHaveBeenCalledWith({ block: "nearest", behavior: "smooth" }); | ||
|
|
||
| grid.remove(); | ||
| }); | ||
|
|
||
| it("shows fresh guidance when the first dropout changes or returns later", () => { | ||
| const initialSong = createDemoRehearsalSong(); | ||
| const { grid } = appendSongStructureTarget(); | ||
| const { rerender } = render(<FirstDropoutCallout song={initialSong} />); | ||
|
|
||
| fireEvent.click( | ||
| screen.getByRole("button", { | ||
| name: "Open Bass Guitar dropout for Lead Vocal at 0:30" | ||
| }) | ||
| ); | ||
| expect(screen.getByText(/Start the last bar of Bass Guitar before Lead Vocal takes the verse \(0:30\)/)).toBeTruthy(); | ||
|
|
||
| const replacementSong = createDemoRehearsalSong(); | ||
| replacementSong.id = "demo-song-replacement"; | ||
| replacementSong.sections[0]!.roles[0]!.name = "Upright Bass"; | ||
| rerender(<FirstDropoutCallout song={replacementSong} />); | ||
| expect(screen.getByText("Upright Bass hands off to Lead Vocal at the end of the verse (0:30).")).toBeTruthy(); | ||
|
|
||
| rerender(<FirstDropoutCallout song={initialSong} />); | ||
| expect(screen.getByText("Bass Guitar hands off to Lead Vocal at the end of the verse (0:30).")).toBeTruthy(); | ||
|
|
||
| grid.remove(); | ||
| }); | ||
|
|
||
| it("keeps placeholder-looking rehearsal data literal", () => { | ||
| const song = createDemoRehearsalSong(); | ||
| song.sections[0]!.roles[0]!.name = "{section}"; | ||
|
|
||
| render(<FirstDropoutCallout song={song} />); | ||
|
|
||
| expect( | ||
| screen.getByRole("button", { | ||
| name: "Open {section} dropout for Lead Vocal at 0:30" | ||
| }) | ||
| ).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("tells the room to stay on the map when no dropout exists", () => { | ||
| const song = createDemoRehearsalSong(); | ||
| song.sections = []; | ||
| render(<FirstDropoutCallout song={song} />); | ||
| expect( | ||
| screen.getByText("No dropout yet. Stay on tonight's map until a part hands off.") | ||
| ).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("contains a malformed runtime song root instead of crashing the callout", () => { | ||
| render(<FirstDropoutCallout song={null as unknown as RehearsalSong} />); | ||
|
|
||
| expect( | ||
| screen.getByText("No dropout yet. Stay on tonight's map until a part hands off.") | ||
| ).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("localizes the section form and keeps Korean role interpolation free of unsafe fixed particles", () => { | ||
| vi.stubGlobal("navigator", { language: "ko-KR" }); | ||
| const song = createDemoRehearsalSong(); | ||
| song.sections[0]!.roles[0]!.name = "베이스 기타"; | ||
| song.sections[0]!.roles[2]!.name = "리드 보컬"; | ||
| const { grid } = appendSongStructureTarget(); | ||
|
|
||
| render(<FirstDropoutCallout song={song} />); | ||
|
|
||
| expect(screen.getByText("0:30 벌스 끝 파트 인계: 베이스 기타 → 리드 보컬.")).toBeTruthy(); | ||
| expect(screen.queryByText(/verse 끝/)).toBeNull(); | ||
|
|
||
| fireEvent.click(screen.getByRole("button", { name: /드롭아웃 위치 열기/ })); | ||
| expect( | ||
| screen.getByText("0:30 벌스: 리드 보컬 진입 전에 베이스 기타의 마지막 마디를 시작하세요.") | ||
| ).toBeTruthy(); | ||
|
|
||
| grid.remove(); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🟡 Duplicate changelog bullet under 0.1.4
Two bullets now describe unit tests for the chords and ranges features, both citing the same two test files. The reworded '100% coverage' line was added alongside the original verbatim line instead of replacing it.
Was this helpful? React with 👍 or 👎 to provide feedback.