-
Notifications
You must be signed in to change notification settings - Fork 0
feat(workspace): guide tonight's first lyric cue on map and player #913
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-hear-first-lyric-cue
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
4e54219
feat(workspace): hear tonight's first lyric cue from the map
seonghobae 193b0d7
fix(workspace): document lyric cue helper
seonghobae 247d144
test(workspace): keep lyric cue placeholders literal
seonghobae 7688cd7
fix(workspace): interpolate lyric cue copy once
seonghobae 270565c
test(workspace): cover lyric cue replacement state
seonghobae af267b8
fix(workspace): reset heard state for changed lyric cue
seonghobae 66919a2
docs(design): clarify lyric cue unavailable action contract
seonghobae 84ec7fa
test(workspace): assert localized fresh lyric guidance
seonghobae 7d9d7a3
test(player): require executable lyric cue action
seonghobae 5228616
fix(player): require executable lyric cue action
seonghobae 063b629
fix(player): delegate lyric cue playback
seonghobae 156768c
test(workspace): reset returning lyric cue state
seonghobae 744e554
fix(workspace): clear stale returning lyric cue
seonghobae 41d9748
test(workspace): reject unsafe lyric cue metadata
seonghobae bd135db
fix(workspace): validate lyric cue candidates
seonghobae 778a2ed
test(workspace): cover malformed lyric value
seonghobae bc588cd
test(workspace): distinguish lyric map navigation from playback
seonghobae c5637be
fix(workspace): label lyric map navigation honestly
seonghobae b01dac7
feat(i18n): distinguish lyric navigation from playback
seonghobae 3783bc0
feat(i18n): localize lyric map navigation
seonghobae 431a153
docs(workspace): distinguish lyric navigation from playback
seonghobae 2247c58
docs(workspace): align lyric cue surface authority
seonghobae 3c367b5
test(workspace): decouple lyric navigation from analysis ids
seonghobae 5afe0bb
test(workspace): bind lyric scroll expectations to rendered positions
seonghobae 5c75eec
fix(workspace): use renderer-owned lyric navigation targets
seonghobae 167914e
test(workspace): target rendered lyric map positions directly
seonghobae d360928
fix(workspace): scroll lyric cues by rendered section position
seonghobae 7bf55bc
fix(workspace): keep analysis ids out of DOM authority
seonghobae 52ee90a
test(workspace): align lyric map contracts with current behavior
seonghobae bea2bd2
test(a11y): require reduced-motion lyric navigation
seonghobae 5d0e5b2
fix(a11y): honor reduced motion for lyric navigation
seonghobae 18d773e
docs(a11y): record reduced-motion lyric navigation contract
seonghobae ca849a2
docs(changelog): note reduced-motion lyric scroll
seonghobae 9281b46
test: reject malformed lyric role ids
seonghobae a92bd8f
fix: reject malformed lyric role ids
seonghobae 548cd1d
test(workspace): lock particle-safe Korean lyric-cue copy
seonghobae c56a589
fix(workspace): keep Korean lyric-cue role copy particle-safe
seonghobae 8401731
fix(workspace): arm lyric cue after navigation lands and keep Korean …
seonghobae c17abdc
Merge remote-tracking branch 'origin/develop' into HEAD
seonghobae 7bd780a
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 lyric cue from this player.") | ||
| ).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("keeps the lyric hear action unavailable without a player playback callback", () => { | ||
| render(<PlayerFeature title="Player" song={createDemoRehearsalSong()} />); | ||
|
|
||
| expect( | ||
| screen.queryByRole("button", { | ||
| name: "Hear Lead Vocal enter on “city lights” in the verse at 0:10" | ||
| }) | ||
| ).toBeNull(); | ||
| expect(screen.getByText("Lead Vocal enters the verse on “city lights” at 0:10.")).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("delegates the lyric 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 Lead Vocal enter on “city lights” in the verse at 0:10" | ||
| }) | ||
| ); | ||
|
|
||
| expect(onPlayFromSeconds).toHaveBeenCalledTimes(1); | ||
| expect(onPlayFromSeconds).toHaveBeenCalledWith(10); | ||
| }); | ||
| }); |
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
55 changes: 55 additions & 0 deletions
55
apps/desktop/src/features/workspace/FirstLyricCueCallout.reduced-motion.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,55 @@ | ||
| import { fireEvent, render, screen } from "@testing-library/react"; | ||
| import { createDemoRehearsalSong } from "@bandscope/shared-types"; | ||
| import { afterEach, describe, expect, it, vi } from "vitest"; | ||
| import { FirstLyricCueCallout } from "./FirstLyricCueCallout"; | ||
|
|
||
| afterEach(() => { | ||
| vi.unstubAllGlobals(); | ||
| document.querySelector('[data-testid="song-structure-grid"]')?.remove(); | ||
| }); | ||
|
|
||
| /** Mount the renderer-owned song-structure target used by workspace navigation. */ | ||
| 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 scrollIntoView; | ||
| } | ||
|
|
||
| describe("FirstLyricCueCallout reduced-motion navigation", () => { | ||
| it("avoids smooth scrolling when the user requests reduced motion", () => { | ||
| const scrollIntoView = appendSongStructureTarget(); | ||
| vi.stubGlobal( | ||
| "matchMedia", | ||
| vi.fn((query: string) => ({ | ||
| matches: query === "(prefers-reduced-motion: reduce)", | ||
| media: query, | ||
| onchange: null, | ||
| addListener: vi.fn(), | ||
| removeListener: vi.fn(), | ||
| addEventListener: vi.fn(), | ||
| removeEventListener: vi.fn(), | ||
| dispatchEvent: vi.fn() | ||
| })) | ||
| ); | ||
|
|
||
| render(<FirstLyricCueCallout song={createDemoRehearsalSong()} />); | ||
| fireEvent.click( | ||
| screen.getByRole("button", { | ||
| name: "Open Lead Vocal lyric cue “city lights” in the verse at 0:10" | ||
| }) | ||
| ); | ||
|
|
||
| expect(scrollIntoView).toHaveBeenCalledWith({ | ||
| block: "nearest", | ||
| behavior: "auto" | ||
| }); | ||
| }); | ||
| }); |
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 entry for new UI tests
The 0.1.4 changelog line about new UI unit tests is replaced by two lines: one with new '100% 테스트 커버리지' wording and one identical to the original (CHANGELOG.md). The second is a verbatim duplicate describing the same tests.
Was this helpful? React with 👍 or 👎 to provide feedback.