Skip to content

Commit c8a7218

Browse files
committed
Move MIDI Studio V2 song notes to full-width detail row and restore timeline instruments accordion - PR_26146_063-midi-studio-v2-song-notes-and-timeline-instruments-layout
1 parent ba7c040 commit c8a7218

6 files changed

Lines changed: 222 additions & 21 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# PR_26146_063 MIDI Studio V2 Song Notes And Timeline Instruments Layout Validation
2+
3+
## Scope
4+
5+
- Continued from `PR_26146_062`.
6+
- Moved editable Notes to the bottom of Song Setup > Song Details.
7+
- Kept Notes as the only field on the bottom Song Details row and spanning the full Song Details width.
8+
- Restored Octave Timeline quick Instruments in the left column as an accordion.
9+
- Kept Octave Timeline quick controls limited to select, mute, solo, and hide/show.
10+
- Preserved GM Type and GM Instrument/Patch ownership in the Instruments tab.
11+
- Preserved selectedInstrumentId sync, canvas-backed note editing, and Play/Stop.
12+
13+
## Validation
14+
15+
PASS - `node --check tools/midi-studio-v2/js/controls/SongDetailsControl.js`
16+
17+
PASS - `node --check tools/midi-studio-v2/js/bootstrap.js`
18+
19+
PASS - `node --check tools/midi-studio-v2/js/MidiStudioV2App.js`
20+
21+
PASS - `node --check tools/midi-studio-v2/js/controls/SongSheetControl.js`
22+
23+
PASS - `node --check tools/midi-studio-v2/js/services/MidiStudioStateSerializer.js`
24+
25+
PASS - `node --check tools/midi-studio-v2/js/controls/InstrumentGridControl.js`
26+
27+
PASS - `node --check tests/playwright/tools/MidiStudioV2.spec.mjs`
28+
29+
PASS - HTML restriction check found no inline script blocks, style blocks, inline event handlers, or inline style attributes in `tools/midi-studio-v2/index.html`.
30+
31+
WARN - `npx playwright test tests/playwright/tools/MidiStudioV2.spec.mjs --project=chromium -g "PR063|PR062|PR061|PR060"` was not a valid local project name. The repo exposes `playwright`.
32+
33+
PASS - `npx playwright test tests/playwright/tools/MidiStudioV2.spec.mjs --project=playwright -g "PR063|PR062|PR061|PR060"`
34+
35+
Result: 4 passed.
36+
37+
PASS - `git diff --check`
38+
39+
PASS - Playwright V8 coverage report generated at `docs/dev/reports/playwright_v8_coverage_report.txt`.
40+
41+
PASS - Changed runtime JS coverage guardrail generated at `docs/dev/reports/coverage_changed_js_guardrail.txt`.
42+
43+
## Playwright Evidence
44+
45+
- Notes renders below the Song Details core fields.
46+
- Notes is the only bottom-row Song Details field.
47+
- Notes spans the available Song Details width.
48+
- Tempo/BPM, Key, and Style remain editable in Song Details.
49+
- Song Sheet remains structure-only and does not duplicate Tempo/BPM, Key, or Style.
50+
- Octave Timeline left column contains the Instruments accordion.
51+
- Octave Timeline Instruments contains quick select, mute, solo, and hide/show controls.
52+
- GM Type and GM Instrument/Patch dropdowns are absent from Octave Timeline and visible in the Instruments tab.
53+
- Selecting an instrument in Octave Timeline syncs `selectedInstrumentId` to the Instruments tab.
54+
- Selecting an instrument in Instruments syncs back to Octave Timeline quick controls.
55+
- Play and Stop still transition correctly.
56+
57+
## Samples Decision
58+
59+
SKIP - Full samples smoke test was not run per PR instruction. This PR is scoped to MIDI Studio V2 layout and targeted tool behavior.
60+

tests/playwright/tools/MidiStudioV2.spec.mjs

Lines changed: 105 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2166,7 +2166,7 @@ test.describe("MIDI Studio V2", () => {
21662166
await expect(page.locator("#songSheetStyleInput")).toHaveValue("public-domain-reel");
21672167
await expect(page.locator("#songDetails [data-song-detail-field='tags']")).toHaveCount(0);
21682168
await expect(page.locator("#songDetails [data-song-detail-field='usage']")).toHaveCount(0);
2169-
await expect(page.locator("#songDetails [data-song-detail-field='notes']")).toHaveValue("Traditional Camptown Races style public-domain test arrangement with lead, bass, chords/pad, and drums.");
2169+
await expect(page.locator("#songDetailNotes [data-song-detail-field='notes']")).toHaveValue("Traditional Camptown Races style public-domain test arrangement with lead, bass, chords/pad, and drums.");
21702170
await expect(page.locator("#songSectionsLoopDetails [data-song-detail-field='sections']")).toHaveCount(0);
21712171
await expect(page.locator("#songSheetSectionsInput")).toHaveValue(/intro: G C G D/);
21722172
await expect(page.locator("#songSheetLoopSectionsInput")).toHaveValue("loop");
@@ -2189,7 +2189,7 @@ test.describe("MIDI Studio V2", () => {
21892189
await page.locator("#songSheetTempoInput").fill("156");
21902190
await page.locator("#songSheetKeyInput").selectOption("C major");
21912191
await page.locator("#songSheetStyleInput").selectOption("chip");
2192-
await page.locator("#songDetails [data-song-detail-field='notes']").fill("Edited notes from Song Setup.");
2192+
await page.locator("#songDetailNotes [data-song-detail-field='notes']").fill("Edited notes from Song Setup.");
21932193
await page.locator("#songSectionsLoopDetails [data-song-detail-field='loopEnabled']").setChecked(false);
21942194
expect(await page.evaluate(() => {
21952195
const song = window.__midiStudioV2App.selectedSong();
@@ -2358,7 +2358,7 @@ test.describe("MIDI Studio V2", () => {
23582358
songTempo: "#songSheetTempoInput",
23592359
songKey: "#songSheetKeyInput",
23602360
songStyle: "#songSheetStyleInput",
2361-
songNotes: "#songDetails [data-song-detail-field='notes']"
2361+
songNotes: "#songDetailNotes [data-song-detail-field='notes']"
23622362
};
23632363
const ownership = await page.evaluate((selectors) => Object.fromEntries(Object.entries(selectors).map(([name, selector]) => {
23642364
const elements = Array.from(document.querySelectorAll(selector));
@@ -2949,6 +2949,108 @@ test.describe("MIDI Studio V2", () => {
29492949
}
29502950
});
29512951

2952+
test("keeps PR063 notes layout and Octave Timeline quick instruments scoped to the left column", async ({ page }) => {
2953+
const server = await openMidiStudioForImport(page);
2954+
try {
2955+
await page.locator("#toolImportManifestInput").setInputFiles(uatManifestPath);
2956+
await selectMidiStudioTab(page, "song-setup");
2957+
2958+
const notesLayout = await page.locator("#songDetailsContent").evaluate((container) => {
2959+
const editor = container.querySelector(".midi-studio-v2__song-detail-editor");
2960+
const core = container.querySelector(".midi-studio-v2__song-detail-core-fields");
2961+
const notesList = container.querySelector("#songDetailNotes");
2962+
const notesRow = notesList?.querySelector(".midi-studio-v2__editable-detail");
2963+
const notesField = notesList?.querySelector("[data-song-detail-field='notes']");
2964+
const detailRows = Array.from(container.querySelectorAll("#songDetails .midi-studio-v2__editable-detail, .midi-studio-v2__song-detail-core-fields .tool-starter__field, #songDetailNotes .midi-studio-v2__editable-detail"));
2965+
const editorRect = editor.getBoundingClientRect();
2966+
const coreRect = core.getBoundingClientRect();
2967+
const notesRect = notesRow.getBoundingClientRect();
2968+
const notesFieldRect = notesField.getBoundingClientRect();
2969+
const laterRows = detailRows.filter((row) => row.getBoundingClientRect().top > notesRect.top + 1);
2970+
const sameRowPeers = detailRows.filter((row) => {
2971+
const rect = row.getBoundingClientRect();
2972+
return row !== notesRow && Math.abs(rect.top - notesRect.top) <= 2;
2973+
});
2974+
return {
2975+
editorWidth: editorRect.width,
2976+
laterRows: laterRows.length,
2977+
notesAfterCore: notesRect.top >= coreRect.bottom - 2,
2978+
notesFieldWidth: notesFieldRect.width,
2979+
notesListWidth: notesList.getBoundingClientRect().width,
2980+
notesRowWidth: notesRect.width,
2981+
sameRowPeers: sameRowPeers.length
2982+
};
2983+
});
2984+
expect(notesLayout.laterRows).toBe(0);
2985+
expect(notesLayout.notesAfterCore).toBe(true);
2986+
expect(notesLayout.sameRowPeers).toBe(0);
2987+
expect(notesLayout.notesListWidth).toBeGreaterThanOrEqual(notesLayout.editorWidth - 2);
2988+
expect(notesLayout.notesRowWidth).toBeGreaterThanOrEqual(notesLayout.editorWidth - 4);
2989+
expect(notesLayout.notesFieldWidth).toBeGreaterThan(notesLayout.editorWidth * 0.85);
2990+
2991+
await expect(page.locator("#songDetailsContent #songSheetTempoInput")).toBeVisible();
2992+
await expect(page.locator("#songDetailsContent #songSheetKeyInput")).toBeVisible();
2993+
await expect(page.locator("#songDetailsContent #songSheetStyleInput")).toBeVisible();
2994+
expect(await page.locator("#songSheetTempoInput").evaluate((input) => input.readOnly || input.disabled)).toBe(false);
2995+
expect(await page.locator("#songSheetKeyInput").evaluate((select) => select.disabled)).toBe(false);
2996+
expect(await page.locator("#songSheetStyleInput").evaluate((select) => select.disabled)).toBe(false);
2997+
await page.locator("#songSheetTempoInput").fill("151");
2998+
await page.locator("#songSheetKeyInput").selectOption("C major");
2999+
await page.locator("#songSheetStyleInput").selectOption("chip");
3000+
expect(await page.evaluate(() => {
3001+
const arrangement = window.__midiStudioV2App.selectedSong().studioArrangement;
3002+
return {
3003+
key: arrangement.key,
3004+
style: arrangement.style,
3005+
tempo: arrangement.tempo
3006+
};
3007+
})).toEqual({ key: "C major", style: "chip", tempo: "151" });
3008+
3009+
await expect(page.locator("#songSheetContent #songSheetTempoInput, #songSheetContent #songSheetKeyInput, #songSheetContent #songSheetStyleInput")).toHaveCount(0);
3010+
await expect(page.locator("#songSheetSectionsInput")).toBeVisible();
3011+
await expect(page.locator("#songSheetLoopSectionsInput")).toBeVisible();
3012+
await expect(page.locator("#songSheetSummary [data-song-sheet-summary-field='bars'] [data-song-sheet-computed='true']")).toHaveCount(1);
3013+
await expect(page.locator("#songSheetSummary [data-song-sheet-summary-field='warnings'] [data-song-sheet-diagnostics='true']")).toHaveCount(1);
3014+
3015+
await selectMidiStudioTab(page, "studio");
3016+
await waitForCanvasRender(page);
3017+
const quickAccordion = page.locator('.tool-starter__panel--left [aria-controls="timelineInstrumentQuickContent"]');
3018+
await expect(quickAccordion).toBeVisible();
3019+
await expect(quickAccordion).toContainText("Instruments");
3020+
await expect(page.locator('.tool-starter__panel--left #timelineInstrumentQuickContent')).toBeVisible();
3021+
await expect(page.locator(".tool-starter__panel--center #timelineInstrumentQuickList")).toHaveCount(0);
3022+
await expect(timelineQuickInstrumentRow(page, "lead")).toBeVisible();
3023+
await expect(timelineQuickInstrumentRow(page, "lead").locator("[data-timeline-quick-mute='lead']")).toBeVisible();
3024+
await expect(timelineQuickInstrumentRow(page, "lead").locator("[data-timeline-quick-solo='lead']")).toBeVisible();
3025+
await expect(timelineQuickInstrumentRow(page, "lead").locator("[data-toggle-instrument-visibility='lead']")).toBeVisible();
3026+
await expect(page.locator("#timelineInstrumentQuickList select, #timelineInstrumentQuickList input:not([type='checkbox']), #timelineInstrumentQuickList [data-lane-instrument-type-select], #timelineInstrumentQuickList [data-lane-instrument-select]")).toHaveCount(0);
3027+
3028+
await timelineQuickInstrumentRow(page, "bass").click();
3029+
expect(await page.evaluate(() => window.__midiStudioV2App.instrumentGrid.selectedInstrumentId)).toBe("bass");
3030+
await selectMidiStudioTab(page, "instruments");
3031+
await expect(instrumentRow(page, "bass")).toHaveClass(/is-selected/);
3032+
await expect(instrumentTypeSelect(page, "bass")).toBeVisible();
3033+
await expect(instrumentSelect(page, "bass")).toBeVisible();
3034+
expect(await instrumentTypeSelect(page, "bass").evaluate((select) => select.closest("[data-midi-studio-tab-panel]")?.dataset.midiStudioTabPanel)).toBe("instruments");
3035+
expect(await instrumentSelect(page, "bass").evaluate((select) => select.closest("[data-midi-studio-tab-panel]")?.dataset.midiStudioTabPanel)).toBe("instruments");
3036+
3037+
await selectInstrumentRow(page, "lead");
3038+
expect(await page.evaluate(() => window.__midiStudioV2App.instrumentGrid.selectedInstrumentId)).toBe("lead");
3039+
await selectMidiStudioTab(page, "studio");
3040+
await expect(timelineQuickInstrumentRow(page, "lead")).toHaveClass(/is-selected/);
3041+
3042+
await page.locator("#playButton").click();
3043+
await expect(page.locator("#playButton")).toBeDisabled();
3044+
await expect(page.locator("#stopButton")).toBeEnabled();
3045+
await page.locator("#stopButton").click();
3046+
await expect(page.locator("#stopButton")).toBeDisabled();
3047+
await expect(page.locator("#playButton")).toBeEnabled();
3048+
} finally {
3049+
await workspaceV2CoverageReporter.stop(page);
3050+
await server.close();
3051+
}
3052+
});
3053+
29523054
test("derives primary song, instrument, grid, playback, and diagnostics views from the canonical selected song", async ({ page }) => {
29533055
const server = await openMidiStudioForImport(page);
29543056
try {

tools/midi-studio-v2/index.html

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,15 @@ <h2 class="tools-platform-frame__eyebrow">First-Class Tools Surface V2</h2>
7878
<div id="songList" class="midi-studio-v2__song-list" role="listbox" aria-label="MIDI songs"></div>
7979
</div>
8080
</section>
81+
<section class="accordion-v2 tool-starter__accordion is-open" data-accordion-v2-open="true" data-midi-studio-tab-panel="studio">
82+
<button class="accordion-v2__header" type="button" aria-expanded="true" aria-controls="timelineInstrumentQuickContent">
83+
<span>Instruments</span>
84+
<span class="accordion-v2__icon" aria-hidden="true">+</span>
85+
</button>
86+
<div id="timelineInstrumentQuickContent" class="accordion-v2__content midi-studio-v2__timeline-instruments">
87+
<div id="timelineInstrumentQuickList" class="midi-studio-v2__timeline-instrument-list" aria-label="Octave Timeline quick instrument controls"></div>
88+
</div>
89+
</section>
8190
<section class="accordion-v2 tool-starter__accordion is-open" data-accordion-v2-open="true" data-midi-studio-tab-panel="midi-import">
8291
<button class="accordion-v2__header" type="button" aria-expanded="true" aria-controls="midiImportContent">
8392
<span>MIDI Import</span>
@@ -178,6 +187,7 @@ <h3 id="songDetailsHeading">Song Details</h3>
178187
</select>
179188
</label>
180189
</div>
190+
<dl id="songDetailNotes" class="midi-studio-v2__details midi-studio-v2__song-detail-notes" aria-label="Song notes"></dl>
181191
</div>
182192
</div>
183193
</section>
@@ -306,10 +316,6 @@ <h2 id="instrumentGridHeading">Octave Timeline</h2>
306316
<button id="instrumentGridZoomInButton" class="midi-studio-v2__zoom-button" type="button" aria-label="Zoom in octave grid" title="Zoom in octave grid">+</button>
307317
</div>
308318
</div>
309-
<section class="midi-studio-v2__timeline-instruments" data-midi-studio-tab-panel="studio" aria-labelledby="timelineQuickInstrumentsHeading">
310-
<h3 id="timelineQuickInstrumentsHeading">Instruments</h3>
311-
<div id="timelineInstrumentQuickList" class="midi-studio-v2__timeline-instrument-list" aria-label="Octave Timeline quick instrument controls"></div>
312-
</section>
313319
<div class="midi-studio-v2__grid-settings" data-midi-studio-tab-panel="auto-create-parts">
314320
<label class="tool-starter__field" for="instrumentGridSectionsInput">
315321
<span>Grid sections from Song Sheet</span>

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ window.addEventListener("DOMContentLoaded", () => {
6969
details: requireElement("#songDetails"),
7070
instrumentSetField: requireElement("#instrumentSetField"),
7171
inspector: requireElement("#inspectorOutput"),
72+
notesDetails: requireElement("#songDetailNotes"),
7273
renderedTargets: requireElement("#renderedTargets"),
7374
sectionsLoopDetails: requireElement("#songSectionsLoopDetails"),
7475
sourceField: requireElement("#songSourceField")

tools/midi-studio-v2/js/controls/SongDetailsControl.js

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,17 @@ function displayValue(value) {
1515
}
1616

1717
function editableRows(song) {
18-
const rows = [
18+
return [
1919
{ field: "name", label: "Name", type: "text", value: song.name },
2020
{ field: "id", label: "Id", readonly: true, type: "text", value: song.id }
2121
];
22+
}
23+
24+
function notesRows(song) {
2225
if (song.director?.notes) {
23-
rows.push({ field: "notes", label: "Notes", rows: 2, type: "textarea", value: song.director.notes });
26+
return [{ field: "notes", label: "Notes", rows: 3, type: "textarea", value: song.director.notes }];
2427
}
25-
return rows;
28+
return [];
2629
}
2730

2831
function sectionsLoopRows(song) {
@@ -34,10 +37,11 @@ function sectionsLoopRows(song) {
3437
}
3538

3639
export class SongDetailsControl {
37-
constructor({ details, instrumentSetField, inspector, renderedTargets, sectionsLoopDetails, sourceField }) {
40+
constructor({ details, instrumentSetField, inspector, notesDetails, renderedTargets, sectionsLoopDetails, sourceField }) {
3841
this.details = details;
3942
this.instrumentSetField = instrumentSetField;
4043
this.inspector = inspector;
44+
this.notesDetails = notesDetails;
4145
this.onChange = () => {};
4246
this.renderedTargets = renderedTargets;
4347
this.sectionsLoopDetails = sectionsLoopDetails;
@@ -46,7 +50,7 @@ export class SongDetailsControl {
4650

4751
mount({ onChange = () => {} } = {}) {
4852
this.onChange = onChange;
49-
[this.details, this.sectionsLoopDetails].filter(Boolean).forEach((list) => {
53+
[this.details, this.notesDetails, this.sectionsLoopDetails].filter(Boolean).forEach((list) => {
5054
list.addEventListener("input", (event) => this.handleFieldChange(event));
5155
list.addEventListener("change", (event) => this.handleFieldChange(event));
5256
});
@@ -81,6 +85,7 @@ export class SongDetailsControl {
8185
renderEmptyDetails(payload) {
8286
this.details.replaceChildren();
8387
this.details.classList.remove("midi-studio-v2__editable-details");
88+
this.renderEmptyNotesDetails();
8489
this.renderEmptySectionsLoopDetails();
8590
if (this.details.hidden) {
8691
this.renderDefinitionList(this.renderedTargets, [["WAV", "No rendered WAV target selected."], ["MP3", "No rendered MP3 target selected."], ["OGG", "No rendered OGG target selected."]]);
@@ -110,6 +115,20 @@ export class SongDetailsControl {
110115
this.details.replaceChildren();
111116
this.details.classList.add("midi-studio-v2__editable-details");
112117
this.renderEditableRows(this.details, editableRows(song));
118+
this.renderNotesDetails(song);
119+
}
120+
121+
renderNotesDetails(song) {
122+
if (!this.notesDetails) {
123+
return;
124+
}
125+
this.notesDetails.replaceChildren();
126+
const rows = notesRows(song);
127+
this.notesDetails.hidden = !rows.length;
128+
this.notesDetails.classList.toggle("midi-studio-v2__editable-details", rows.length > 0);
129+
if (rows.length) {
130+
this.renderEditableRows(this.notesDetails, rows);
131+
}
113132
}
114133

115134
renderSectionsLoopDetails(song) {
@@ -133,6 +152,15 @@ export class SongDetailsControl {
133152
this.sectionsLoopDetails.append(empty);
134153
}
135154

155+
renderEmptyNotesDetails() {
156+
if (!this.notesDetails) {
157+
return;
158+
}
159+
this.notesDetails.replaceChildren();
160+
this.notesDetails.classList.remove("midi-studio-v2__editable-details");
161+
this.notesDetails.hidden = true;
162+
}
163+
136164
renderEditableRows(list, rows) {
137165
rows.forEach((row) => {
138166
const wrapper = document.createElement("div");
@@ -174,7 +202,8 @@ export class SongDetailsControl {
174202
}
175203

176204
updateFieldValue(field, value) {
177-
const control = this.details.querySelector(`[data-song-detail-field="${field}"]`);
205+
const control = this.details.querySelector(`[data-song-detail-field="${field}"]`)
206+
|| this.notesDetails?.querySelector(`[data-song-detail-field="${field}"]`);
178207
if (control && control.type !== "checkbox") {
179208
control.value = displayValue(value) === "not declared" ? "" : displayValue(value);
180209
}

0 commit comments

Comments
 (0)