Skip to content

Commit 6cd6c1a

Browse files
committed
Improve MIDI Studio V2 piano key styling and section loop feedback - PR_26146_041-midi-studio-v2-piano-style-and-section-feedback
1 parent 621e96f commit 6cd6c1a

5 files changed

Lines changed: 257 additions & 14 deletions

File tree

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# PR_26146_041 MIDI Studio V2 Piano Style And Section Feedback Validation
2+
3+
Status: PASS
4+
5+
## Scope
6+
7+
- Set compact Bar/Beat header row backgrounds to `#3600af`.
8+
- Improved Octave Timeline piano-key styling so natural rows read as white keys, sharp rows read as black keys, and labels remain left-aligned.
9+
- Reduced section shortcut button padding, margins, and layout footprint.
10+
- Added visible selected-section region highlighting and loop region feedback when section/loop values change.
11+
- Converted missing section feedback to actionable WARN status text.
12+
- Preserved 50% grid height, vertical scrolling, square-cell zoom, note editing, playback, and Play/Stop behavior.
13+
14+
## Files Changed
15+
16+
- `tools/midi-studio-v2/js/MidiStudioV2App.js`
17+
- `tools/midi-studio-v2/js/controls/InstrumentGridControl.js`
18+
- `tools/midi-studio-v2/styles/midiStudioV2.css`
19+
- `tests/playwright/tools/MidiStudioV2.spec.mjs`
20+
- `docs/dev/reports/PR_26146_041-midi-studio-v2-piano-style-and-section-feedback_validation.md`
21+
- `docs/dev/reports/coverage_changed_js_guardrail.txt`
22+
- `docs/dev/codex_commands.md`
23+
- `docs/dev/commit_comment.txt`
24+
25+
Review artifacts are generated separately:
26+
27+
- `docs/dev/reports/codex_review.diff`
28+
- `docs/dev/reports/codex_changed_files.txt`
29+
- `docs/dev/reports/playwright_v8_coverage_report.txt`
30+
31+
## Validation Commands
32+
33+
PASS:
34+
35+
```powershell
36+
node --check tools/midi-studio-v2/js/controls/InstrumentGridControl.js
37+
node --check tools/midi-studio-v2/js/MidiStudioV2App.js
38+
node --check tests/playwright/tools/MidiStudioV2.spec.mjs
39+
node -e "const fs=require('fs'); for (const file of ['tools/midi-studio-v2/styles/midiStudioV2.css']) { const text=fs.readFileSync(file,'utf8'); let depth=0; for (const ch of text) { if (ch==='{') depth++; if (ch==='}') depth--; if (depth<0) throw new Error(file+': unexpected }'); } if (depth!==0) throw new Error(file+': unbalanced braces'); console.log(file+': CSS brace check OK'); }"
40+
npx.cmd playwright test tests/playwright/tools/MidiStudioV2.spec.mjs -g "octave timeline freezes compact headers and note labels while active cells stay textless|renders timing ruler, section navigation, and loop region visualization|reports invalid section and invalid loop handling|fast octave note editing supports drag painting keyboard shortcuts selection and timeline scroll sync" --config=codex_playwright_system_chrome.config.cjs --reporter=list --workers=1 --timeout=60000
41+
git diff --check
42+
```
43+
44+
Playwright result:
45+
46+
```text
47+
Running 4 tests using 1 worker
48+
49+
ok 1 MIDI Studio V2 - fast octave note editing supports drag painting keyboard shortcuts selection and timeline scroll sync
50+
ok 2 MIDI Studio V2 - octave timeline freezes compact headers and note labels while active cells stay textless
51+
ok 3 MIDI Studio V2 - renders timing ruler, section navigation, and loop region visualization
52+
ok 4 MIDI Studio V2 - reports invalid section and invalid loop handling
53+
54+
4 passed
55+
```
56+
57+
Notes:
58+
59+
- `git diff --check` exited 0. Git emitted line-ending warnings for existing LF/CRLF handling in touched files; no whitespace errors were reported.
60+
- Full samples smoke test was not run.
61+
62+
## Test Evidence
63+
64+
- Bar/Beat rows use `#3600af`: Playwright verifies computed header backgrounds are `rgb(54, 0, 175)`.
65+
- Piano keys distinguish natural and sharp/flat notes: Playwright verifies white/black key classes, background/color differences, and left-aligned text.
66+
- Section buttons have reduced padding/margins: Playwright verifies zero margins and compact padding on all five section preset buttons.
67+
- Invalid section names show WARN: Playwright verifies missing Bridge emits actionable WARN status text.
68+
- Valid section selection updates highlight: Playwright verifies selecting `bridge` creates visible selected-section region cells for that range.
69+
- Loop changes update highlight/status: Playwright verifies `loop -> boss` highlights a loop region and logs the updated loop region.
70+
- Play Section range: Playwright verifies Play Section starts `bridge` playback, sets the playhead to bridge, and Preview Synth snapshot records section mode.
71+
- Play Loop range: Playwright verifies Play Loop starts `loop to boss`, sets the playhead to loop, and Preview Synth snapshot records loop mode with an active loop.
72+
- Play/Stop: Playwright verifies existing keyboard/button Play and Stop behavior remains functional.
73+
74+
## Samples
75+
76+
Full samples smoke test: SKIP.
77+
78+
Reason: PR scope is limited to MIDI Studio V2 tool UI/runtime and targeted Playwright coverage; sample JSON alignment and full samples smoke are explicitly out of scope.
79+
80+
## Manual UAT
81+
82+
1. Open MIDI Studio V2 and import the UAT manifest.
83+
2. Confirm Bar/Beat headers are purple `#3600af` except active playhead cells.
84+
3. Confirm left note labels resemble piano keys and all labels are left-aligned.
85+
4. Select section and loop controls; confirm selected section and loop regions update visibly.
86+
5. Click missing section shortcuts and confirm WARN status text is actionable.
87+
6. Play a section and a loop; confirm status text names the selected range and Stop halts playback.

tests/playwright/tools/MidiStudioV2.spec.mjs

Lines changed: 86 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,6 +1011,10 @@ test.describe("MIDI Studio V2", () => {
10111011
const outputStyle = getComputedStyle(element);
10121012
const whiteLabelStyle = getComputedStyle(whiteLabel);
10131013
const blackLabelStyle = getComputedStyle(blackLabel);
1014+
const barHeaderStyle = getComputedStyle(element.querySelector('.midi-studio-v2__timing-header-row-1.midi-studio-v2__note-table-column-header[data-step-index="1"]'));
1015+
const beatHeaderStyle = getComputedStyle(element.querySelector('.midi-studio-v2__timing-header-row-2.midi-studio-v2__note-table-column-header[data-step-index="1"]'));
1016+
const barAxisStyle = getComputedStyle(element.querySelector(".midi-studio-v2__timing-header-row-1.midi-studio-v2__timing-axis-header"));
1017+
const beatAxisStyle = getComputedStyle(element.querySelector(".midi-studio-v2__timing-header-row-2.midi-studio-v2__timing-axis-header"));
10141018
const whiteLabelRect = whiteLabel.getBoundingClientRect();
10151019
const blackLabelRect = blackLabel.getBoundingClientRect();
10161020
const whiteCellRect = whiteCell.getBoundingClientRect();
@@ -1020,9 +1024,11 @@ test.describe("MIDI Studio V2", () => {
10201024
blackBackground: blackLabelStyle.backgroundImage || blackLabelStyle.backgroundColor,
10211025
blackClass: blackLabel.className,
10221026
blackColor: blackLabelStyle.color,
1027+
blackJustifyItems: blackLabelStyle.justifyItems,
10231028
blackKind: blackLabel.dataset.keyKind,
10241029
blackLabelHeight: blackLabelRect.height,
10251030
blackLabelText: blackLabel.textContent,
1031+
blackTextAlign: blackLabelStyle.textAlign,
10261032
borderRightWidth: getComputedStyle(firstNote).borderRightWidth,
10271033
canScrollVertically: element.scrollHeight > element.clientHeight,
10281034
cellCssWidth: getComputedStyle(firstNote).width,
@@ -1031,14 +1037,22 @@ test.describe("MIDI Studio V2", () => {
10311037
columnTemplate: getComputedStyle(grid).gridTemplateColumns,
10321038
containsGrid: grid?.parentElement === element,
10331039
expectedViewportHeight: window.innerHeight * 0.5,
1040+
headerBackgrounds: [
1041+
barHeaderStyle.backgroundColor,
1042+
beatHeaderStyle.backgroundColor,
1043+
barAxisStyle.backgroundColor,
1044+
beatAxisStyle.backgroundColor
1045+
],
10341046
overflowY: outputStyle.overflowY,
10351047
viewportHeight: element.getBoundingClientRect().height,
10361048
whiteBackground: whiteLabelStyle.backgroundImage || whiteLabelStyle.backgroundColor,
10371049
whiteClass: whiteLabel.className,
10381050
whiteColor: whiteLabelStyle.color,
1051+
whiteJustifyItems: whiteLabelStyle.justifyItems,
10391052
whiteKind: whiteLabel.dataset.keyKind,
10401053
whiteLabelHeight: whiteLabelRect.height,
10411054
whiteLabelText: whiteLabel.textContent,
1055+
whiteTextAlign: whiteLabelStyle.textAlign,
10421056
whiteRowDelta: Math.abs(whiteLabelRect.top - whiteCellRect.top),
10431057
whiteRowHeightDelta: Math.abs(whiteLabelRect.height - whiteCellRect.height),
10441058
blackRowDelta: Math.abs(blackLabelRect.top - blackCellRect.top),
@@ -1068,6 +1082,16 @@ test.describe("MIDI Studio V2", () => {
10681082
expect(gridLayout.blackLabelText).toBe("C#5");
10691083
expect(gridLayout.whiteBackground).not.toBe(gridLayout.blackBackground);
10701084
expect(gridLayout.whiteColor).not.toBe(gridLayout.blackColor);
1085+
expect(gridLayout.headerBackgrounds).toEqual([
1086+
"rgb(54, 0, 175)",
1087+
"rgb(54, 0, 175)",
1088+
"rgb(54, 0, 175)",
1089+
"rgb(54, 0, 175)"
1090+
]);
1091+
expect(gridLayout.whiteTextAlign).toBe("left");
1092+
expect(gridLayout.blackTextAlign).toBe("left");
1093+
expect(gridLayout.whiteJustifyItems).toBe("start");
1094+
expect(gridLayout.blackJustifyItems).toBe("start");
10711095
expect(gridLayout.whiteRowDelta).toBeLessThanOrEqual(1);
10721096
expect(gridLayout.blackRowDelta).toBeLessThanOrEqual(1);
10731097
expect(gridLayout.whiteRowHeightDelta).toBeLessThanOrEqual(1);
@@ -2105,15 +2129,73 @@ test.describe("MIDI Studio V2", () => {
21052129
sections: "intro:1, loop:1, bridge:1, boss:1, victory:1"
21062130
});
21072131
await page.locator("#normalizeInstrumentGridButton").click();
2108-
await expect(page.locator("#instrumentGridOutput")).toContainText("Bar 1");
2109-
await expect(page.locator("#instrumentGridOutput")).toContainText("Bar 5");
2132+
await expect(page.locator('.midi-studio-v2__grid-cell--bar-header[data-step-index="0"]')).toHaveText("1");
2133+
await expect(page.locator('.midi-studio-v2__grid-cell--bar-header[data-step-index="16"]')).toHaveText("5");
2134+
const sectionButtonLayout = await page.locator(".midi-studio-v2__section-preset").evaluateAll((buttons) => buttons.map((button) => {
2135+
const style = getComputedStyle(button);
2136+
return {
2137+
marginBottom: Number.parseFloat(style.marginBottom),
2138+
marginLeft: Number.parseFloat(style.marginLeft),
2139+
marginRight: Number.parseFloat(style.marginRight),
2140+
marginTop: Number.parseFloat(style.marginTop),
2141+
paddingBottom: Number.parseFloat(style.paddingBottom),
2142+
paddingLeft: Number.parseFloat(style.paddingLeft),
2143+
paddingRight: Number.parseFloat(style.paddingRight),
2144+
paddingTop: Number.parseFloat(style.paddingTop)
2145+
};
2146+
}));
2147+
expect(sectionButtonLayout).toHaveLength(5);
2148+
expect(sectionButtonLayout.every((button) => button.marginBottom === 0 && button.marginLeft === 0 && button.marginRight === 0 && button.marginTop === 0)).toBe(true);
2149+
expect(sectionButtonLayout.every((button) => button.paddingBottom <= 4 && button.paddingTop <= 4 && button.paddingLeft <= 8 && button.paddingRight <= 8)).toBe(true);
21102150
await expect(page.locator(".midi-studio-v2__grid-cell--bar")).toHaveCount(5);
21112151
await expect(page.locator(".midi-studio-v2__grid-cell--ruler").first()).toContainText("1");
21122152
await expect(page.locator(".midi-studio-v2__grid-cell--beat-header.midi-studio-v2__grid-cell--playhead-active")).toHaveAttribute("data-section", "intro");
21132153
await expect(page.locator("#instrumentGridSectionSelect")).toContainText("bridge");
21142154
await page.locator("#instrumentGridLoopStartSelect").selectOption("loop");
21152155
await page.locator("#instrumentGridLoopEndSelect").selectOption("boss");
21162156
expect(await page.locator(".midi-studio-v2__grid-cell--loop-region").count()).toBeGreaterThan(0);
2157+
await expect(page.locator("#instrumentGridTransportState")).toContainText("Loop region set: loop -> boss");
2158+
await expect(page.locator("#statusLog")).toHaveValue(/INFO Loop region set: loop -> boss\./);
2159+
await page.locator("#instrumentGridSectionSelect").selectOption("bridge");
2160+
const bridgeRegion = await page.locator(".midi-studio-v2__grid-cell--section-region").evaluateAll((cells) => ({
2161+
count: cells.length,
2162+
sections: Array.from(new Set(cells.map((cell) => cell.dataset.section)))
2163+
}));
2164+
expect(bridgeRegion.count).toBeGreaterThan(0);
2165+
expect(bridgeRegion.sections).toEqual(["bridge"]);
2166+
await expect(page.locator("#instrumentGridTransportState")).toContainText("Selected section: bridge");
2167+
await expect(page.locator("#statusLog")).toHaveValue(/INFO Timing section selected: bridge\./);
2168+
await page.locator("#playSectionButton").click();
2169+
await expect(page.locator("#instrumentGridTransportState")).toContainText("Playing section: bridge");
2170+
await expect(page.locator("#statusLog")).toHaveValue(/INFO Playing section: bridge\./);
2171+
await expect(page.locator("#statusLog")).toHaveValue(/OK Preview Synth started for section bridge with \d+ playable events\./);
2172+
expect(await page.evaluate(() => window.__midiStudioV2App.previewSynth.getSnapshot())).toMatchObject({
2173+
lastPlayback: {
2174+
label: "bridge",
2175+
mode: "section"
2176+
},
2177+
loopActive: false,
2178+
playing: true
2179+
});
2180+
await expect(page.locator(".midi-studio-v2__grid-cell--beat-header.midi-studio-v2__grid-cell--playhead-active")).toHaveAttribute("data-section", "bridge");
2181+
await page.locator("#stopTimingPreviewButton").click();
2182+
await page.evaluate(() => {
2183+
window.__midiStudioPreviewSynthEvents = [];
2184+
});
2185+
await page.locator("#playLoopButton").click();
2186+
await expect(page.locator("#instrumentGridTransportState")).toContainText("Playing loop: loop to boss");
2187+
await expect(page.locator("#statusLog")).toHaveValue(/INFO Playing loop: loop to boss\./);
2188+
await expect(page.locator("#statusLog")).toHaveValue(/OK Preview Synth started for loop loop to boss with \d+ playable events\./);
2189+
expect(await page.evaluate(() => window.__midiStudioV2App.previewSynth.getSnapshot())).toMatchObject({
2190+
lastPlayback: {
2191+
label: "loop to boss",
2192+
mode: "loop"
2193+
},
2194+
loopActive: true,
2195+
playing: true
2196+
});
2197+
await expect(page.locator(".midi-studio-v2__grid-cell--beat-header.midi-studio-v2__grid-cell--playhead-active")).toHaveAttribute("data-section", "loop");
2198+
await page.locator("#stopTimingPreviewButton").click();
21172199
await page.locator('[data-section-preset="boss"]').click();
21182200
await expect(page.locator("#instrumentGridSectionSelect")).toHaveValue("boss");
21192201
await page.locator("#jumpToSectionButton").click();
@@ -2263,11 +2345,11 @@ test.describe("MIDI Studio V2", () => {
22632345
await fillInstrumentGrid(page);
22642346
await page.locator("#normalizeInstrumentGridButton").click();
22652347
await page.locator('[data-section-preset="bridge"]').click();
2266-
await expect(page.locator("#statusLog")).toHaveValue(/FAIL Instrument grid section not found: Bridge\. Normalize a section map containing that label or choose a listed custom section\./);
2348+
await expect(page.locator("#statusLog")).toHaveValue(/WARN section Bridge does not exist\. Normalize a section map containing that label or choose a listed custom section\./);
22672349
await page.locator("#instrumentGridLoopStartSelect").selectOption("loop");
22682350
await page.locator("#instrumentGridLoopEndSelect").selectOption("intro");
22692351
await page.locator("#playLoopButton").click();
2270-
await expect(page.locator("#statusLog")).toHaveValue(/FAIL Instrument grid loop rejected: Invalid loop region: loop starts after intro\./);
2352+
await expect(page.locator("#statusLog")).toHaveValue(/WARN Loop region unavailable: Invalid loop region: loop starts after intro\./);
22712353
} finally {
22722354
await workspaceV2CoverageReporter.stop(page);
22732355
await server.close();

tools/midi-studio-v2/js/MidiStudioV2App.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -631,11 +631,11 @@ export class MidiStudioV2App {
631631

632632
async handleInstrumentGridTransport(action, detail = {}) {
633633
if (action === "invalid-section") {
634-
this.statusLog.fail(`Instrument grid section not found: ${detail.label}. Normalize a section map containing that label or choose a listed custom section.`);
634+
this.statusLog.warn(`section ${detail.label} does not exist. Normalize a section map containing that label or choose a listed custom section.`);
635635
return;
636636
}
637637
if (action === "invalid-loop") {
638-
this.statusLog.fail(`Instrument grid loop rejected: ${detail.message}`);
638+
this.statusLog.warn(`Loop region unavailable: ${detail.message}`);
639639
return;
640640
}
641641
if (action === "play-section") {
@@ -664,6 +664,10 @@ export class MidiStudioV2App {
664664
this.statusLog.info(`Timing section selected: ${detail.section.label}.`);
665665
return;
666666
}
667+
if (action === "set-loop-region") {
668+
this.statusLog.info(`Loop region set: ${detail.startSection.label} -> ${detail.endSection.label}.`);
669+
return;
670+
}
667671
if (action === "stop-preview") {
668672
const stoppedCount = this.previewSynth.stop();
669673
this.statusLog.ok(`Preview playback stopped. Cleared ${stoppedCount} scheduled oscillator${stoppedCount === 1 ? "" : "s"}.`);
@@ -698,6 +702,7 @@ export class MidiStudioV2App {
698702
this.statusLog.warn(`Preview Synth warnings: ${result.warnings.join("; ")}`);
699703
}
700704
this.instrumentGrid.setPreviewPlaybackLanes(result.activeLanes);
705+
this.statusLog.info(`Playing ${mode}: ${label}.`);
701706
this.statusLog.ok(`Preview Synth started for ${mode} ${label} with ${result.eventCount} playable event${result.eventCount === 1 ? "" : "s"}.`);
702707
this.statusLog.warn("Preview Synth is an approximate Web Audio audition; SoundFont and real instrument playback are not implemented.");
703708
this.updateAudioDiagnostics();

0 commit comments

Comments
 (0)