Polish MIDI Studio V2 instrument row icons, width, layering, and scroll stability - PR_26146_030-midi-studio-v2-instrument-row-polish

Author: ToolboxAid

docs/dev/reports/PR_26146_030-midi-studio-v2-instrument-row-polish_validation.md

@@ -0,0 +1,52 @@
+# PR_26146_030 MIDI Studio V2 Instrument Row Polish Validation
+
+Status: PASS
+
+## Scope
+
+- Continued from PR_26146_029.
+- Replaced visible Mute/Solo text controls with CSS-drawn icon controls.
+- Kept Mute, Solo, Eye visibility, and Delete/X controls on one horizontal row.
+- Increased the MIDI Studio V2 left column by 350px and verified instrument controls fit without wrapping or clipping.
+- Applied gray styling to non-selected instrument notes.
+- Gave selected instrument note cells a higher draw layer than dimmed/non-selected note cells.
+- Preserved PR029 octave density and multi-note/chord behavior.
+- Preserved GM Type + Instrument dropdown behavior.
+- Preserved playback, Play/Stop, and Stop All Audio behavior.
+
+## Changed Files
+
+- `tools/midi-studio-v2/js/controls/InstrumentGridControl.js`
+- `tools/midi-studio-v2/styles/midiStudioV2.css`
+- `tests/playwright/tools/MidiStudioV2.spec.mjs`
+- `docs/dev/reports/PR_26146_030-midi-studio-v2-instrument-row-polish_validation.md`
+
+## Validation
+
+- PASS: `node --check tools/midi-studio-v2/js/controls/InstrumentGridControl.js`
+- PASS: `node --check tests/playwright/tools/MidiStudioV2.spec.mjs`
+- PASS: `npx playwright test tests/playwright/tools/MidiStudioV2.spec.mjs -g "octave timeline editor is the default editable and playable Studio workflow|octave grid density supports icon controls and simultaneous chord editing" --reporter=list --workers=1 --timeout=60000`
+  - 2 passed.
+- PASS: `git diff --check`
+  - No whitespace errors. Git emitted LF-to-CRLF working-copy warnings for touched text files.
+
+## Required Proof Points
+
+- PASS: Mute and Solo render as icon controls with no visible text.
+- PASS: mute/solo/eye/delete controls stay on one horizontal row.
+- PASS: widened left column is at least the expected PR030 width and instrument controls fit without wrapping or clipping.
+- PASS: non-selected instrument notes use gray color/background treatment.
+- PASS: selected instrument notes use a higher z-layer than dimmed notes.
+- PASS: clicking mute, solo, eye visibility, and delete controls preserves instrument-area scroll positions.
+- PASS: show/hide behavior still removes and restores hidden-lane notes.
+- PASS: GM Type + Instrument dropdown behavior remains covered by the carried octave workflow.
+- PASS: multi-note/chord editing still adds sibling notes without removing existing notes.
+- PASS: drum simultaneous events still work.
+- PASS: Play and Stop still work.
+- PASS: Stop All Audio still returns playback controls to idle.
+- PASS: all JS/CSS remains external; no HTML file was modified.
+
+## Notes
+
+- Full samples smoke test was not run, per BUILD request.
+- Initial targeted Playwright run exposed a Songs/MIDI Import overlap after widening the left column; the song list now sizes to its contents and the final targeted run passed.

tests/playwright/tools/MidiStudioV2.spec.mjs

@@ -372,6 +372,51 @@ function instrumentSelect(page, lane) {
   return instrumentRow(page, lane).locator("[data-lane-instrument-select]");
 }
 
+function instrumentToggle(page, lane, kind) {
+  return instrumentRow(page, lane).locator(`.midi-studio-v2__lane-toggle--${kind}`);
+}
+
+async function instrumentScrollSnapshot(page) {
+  return page.evaluate(() => {
+    const leftPanel = document.querySelector(".tool-starter__panel--left");
+    const instrumentPanel = document.querySelector(".midi-studio-v2__instrument-list-panel");
+    return {
+      instrumentLeft: instrumentPanel?.scrollLeft || 0,
+      instrumentTop: instrumentPanel?.scrollTop || 0,
+      leftLeft: leftPanel?.scrollLeft || 0,
+      leftTop: leftPanel?.scrollTop || 0,
+      windowX: window.scrollX,
+      windowY: window.scrollY
+    };
+  });
+}
+
+async function prepareInstrumentScrollSentinel(page) {
+  await page.evaluate(() => {
+    const leftPanel = document.querySelector(".tool-starter__panel--left");
+    const instrumentPanel = document.querySelector(".midi-studio-v2__instrument-list-panel");
+    if (leftPanel) {
+      leftPanel.style.maxHeight = "260px";
+      leftPanel.style.overflow = "auto";
+      leftPanel.scrollLeft = 0;
+    }
+    if (instrumentPanel) {
+      instrumentPanel.style.maxHeight = "170px";
+      instrumentPanel.style.overflow = "auto";
+      instrumentPanel.scrollLeft = 0;
+    }
+  });
+}
+
+async function expectInstrumentClickKeepsScroll(page, locator) {
+  await prepareInstrumentScrollSentinel(page);
+  await locator.evaluate((element) => element.scrollIntoView({ block: "center", inline: "nearest" }));
+  const before = await instrumentScrollSnapshot(page);
+  await locator.click();
+  await page.waitForTimeout(50);
+  await expect.poll(() => instrumentScrollSnapshot(page)).toEqual(before);
+}
+
 function laneHeader(page, lane) {
   return page.locator(`.midi-studio-v2__lane-header-cell[data-lane="${lane}"]`);
 }
@@ -535,6 +580,15 @@ test.describe("MIDI Studio V2", () => {
       await expect(leadVisibility).toHaveText("");
       await expect(leadVisibility).toHaveAttribute("aria-pressed", "true");
       await expect(leadVisibility).toHaveAttribute("aria-label", /Hide Lead/);
+      await expect(instrumentToggle(page, "lead", "mute")).toHaveText("");
+      await expect(instrumentToggle(page, "lead", "solo")).toHaveText("");
+      await expect(instrumentToggle(page, "lead", "mute").locator(".midi-studio-v2__lane-toggle-icon")).toHaveCount(1);
+      await expect(instrumentToggle(page, "lead", "solo").locator(".midi-studio-v2__lane-toggle-icon")).toHaveCount(1);
+      await expect(instrumentRow(page, "lead").locator('[aria-label="Mute Lead"]')).not.toBeChecked();
+      await instrumentToggle(page, "lead", "mute").click();
+      await expect(instrumentRow(page, "lead").locator('[aria-label="Mute Lead"]')).toBeChecked();
+      await instrumentToggle(page, "lead", "mute").click();
+      await expect(instrumentRow(page, "lead").locator('[aria-label="Mute Lead"]')).not.toBeChecked();
       const controlAlignment = await instrumentRow(page, "lead").locator(".midi-studio-v2__instrument-control-row").evaluate((row) => {
         const controls = [
           row.querySelector('[aria-label="Mute Lead"]')?.closest("label"),
@@ -556,6 +610,20 @@ test.describe("MIDI Studio V2", () => {
       });
       expect(controlAlignment.missing).toBe(false);
       expect(controlAlignment.maxCenterDelta).toBeLessThanOrEqual(4);
+      const leftColumnFit = await instrumentRow(page, "lead").locator(".midi-studio-v2__instrument-control-row").evaluate((row) => {
+        const leftPanel = document.querySelector(".tool-starter__panel--left").getBoundingClientRect();
+        const rowRect = row.getBoundingClientRect();
+        const controls = Array.from(row.children).map((control) => control.getBoundingClientRect());
+        return {
+          controlsFit: controls.every((rect) => rect.left >= rowRect.left - 1 && rect.right <= rowRect.right + 1 && rect.right <= leftPanel.right + 1),
+          leftWidth: leftPanel.width,
+          rowClientWidth: row.clientWidth,
+          rowScrollWidth: row.scrollWidth
+        };
+      });
+      expect(leftColumnFit.leftWidth).toBeGreaterThanOrEqual(560);
+      expect(leftColumnFit.controlsFit).toBe(true);
+      expect(leftColumnFit.rowScrollWidth).toBeLessThanOrEqual(leftColumnFit.rowClientWidth + 1);
 
       const octaveCellHeight = await octaveCell(page, "C5", 0).evaluate((cell) => cell.getBoundingClientRect().height);
       expect(octaveCellHeight).toBeLessThanOrEqual(32);
@@ -590,6 +658,33 @@ test.describe("MIDI Studio V2", () => {
       expect(leadChordValues).toEqual(expect.arrayContaining(initialLeadValues));
       await expect(octaveCell(page, "C5", chordStep)).toHaveClass(/midi-studio-v2__grid-cell--lane-selected/);
       await expect(octaveNoteBlock(page, "chords").first()).toHaveClass(/midi-studio-v2__grid-cell--lane-dimmed/);
+      const noteLayering = await page.evaluate(() => {
+        const selected = document.querySelector('.midi-studio-v2__octave-note-cell[data-row-token="C5"].midi-studio-v2__grid-cell--lane-selected');
+        const dimmed = document.querySelector(".midi-studio-v2__octave-note-cell.midi-studio-v2__grid-cell--lane-dimmed");
+        const selectedStyles = getComputedStyle(selected);
+        const dimmedStyles = getComputedStyle(dimmed);
+        return {
+          dimmedBackground: dimmedStyles.backgroundColor,
+          dimmedColor: dimmedStyles.color,
+          dimmedZ: Number(dimmedStyles.zIndex),
+          selectedZ: Number(selectedStyles.zIndex)
+        };
+      });
+      expect(noteLayering.dimmedColor).toBe("rgb(148, 163, 184)");
+      expect(noteLayering.dimmedBackground).toMatch(/rgba?\(71,\s*85,\s*105/);
+      expect(noteLayering.selectedZ).toBeGreaterThan(noteLayering.dimmedZ);
+
+      await expectInstrumentClickKeepsScroll(page, instrumentToggle(page, "lead", "mute"));
+      await expectInstrumentClickKeepsScroll(page, instrumentToggle(page, "lead", "mute"));
+      await expectInstrumentClickKeepsScroll(page, instrumentToggle(page, "lead", "solo"));
+      await expectInstrumentClickKeepsScroll(page, instrumentToggle(page, "lead", "solo"));
+      await expectInstrumentClickKeepsScroll(page, leadVisibility);
+      await expectInstrumentClickKeepsScroll(page, leadVisibility);
+      await page.locator("#addInstrumentRowButton").click();
+      await expect(instrumentRow(page, "instrument-1")).toBeVisible();
+      await expectInstrumentClickKeepsScroll(page, instrumentRow(page, "instrument-1").locator("[data-delete-instrument-row='instrument-1']"));
+      await expect(instrumentRow(page, "instrument-1")).toHaveCount(0);
+      await selectInstrumentRow(page, "lead");
 
       await leadVisibility.click();
       await expect(octaveNoteBlock(page, "lead")).toHaveCount(0);
@@ -649,6 +744,11 @@ test.describe("MIDI Studio V2", () => {
       await expect(page.locator(`.midi-studio-v2__grid-cell--timing-header.midi-studio-v2__grid-cell--playhead-active[data-step-index="${activeStep}"]`)).toHaveCount(1);
       await page.locator("#stopButton").click();
       await expect(page.locator("#playButton")).toBeEnabled();
+      await page.locator("#playButton").click();
+      await expect(page.locator("#stopButton")).toBeEnabled();
+      await page.locator("#stopAllAudioButton").click();
+      await expect(page.locator("#stopButton")).toBeDisabled();
+      await expect(page.locator("#playButton")).toBeEnabled();
     } finally {
       await workspaceV2CoverageReporter.stop(page);
       await server.close();

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

@@ -687,7 +687,10 @@ export class InstrumentGridControl {
     button.className = "midi-studio-v2__lane-icon-button midi-studio-v2__visibility-button";
     button.type = "button";
     button.dataset.toggleInstrumentVisibility = lane;
-    button.addEventListener("click", () => this.toggleInstrumentVisibility(lane));
+    this.preventPointerFocusScroll(button);
+    button.addEventListener("click", () => {
+      this.runInstrumentControlAction(() => this.toggleInstrumentVisibility(lane));
+    });
     this.updateVisibilityButton(button, lane);
     return button;
   }
@@ -725,7 +728,7 @@ export class InstrumentGridControl {
   }
 
   createOctaveTimelineCell({ result, row, stepIndex }) {
-    const events = this.visibleEventsForCell({ result, row, stepIndex });
+    const events = this.orderedEventsForCell(this.visibleEventsForCell({ result, row, stepIndex }));
     const cell = document.createElement("div");
     cell.className = this.octaveCellClass(events).join(" ");
     cell.dataset.octaveRow = row.value;
@@ -775,6 +778,14 @@ export class InstrumentGridControl {
     }))).join(" ");
   }
 
+  orderedEventsForCell(events) {
+    return events.slice().sort((left, right) => {
+      const leftSelected = left.lane === this.selectedLane ? 1 : 0;
+      const rightSelected = right.lane === this.selectedLane ? 1 : 0;
+      return leftSelected - rightSelected;
+    });
+  }
+
   visibleEventsForCell({ result, row, stepIndex }) {
     return (result.timeline || []).filter((event) => {
       if (event.stepIndex !== stepIndex || this.previewLaneState[event.lane]?.visible === false) {
@@ -1119,29 +1130,36 @@ export class InstrumentGridControl {
   createLaneToggle(lane, kind) {
     const input = document.createElement("input");
     const label = document.createElement("label");
-    const text = document.createElement("span");
+    const icon = document.createElement("span");
     const controlLabel = kind === "mute" ? "Mute" : "Solo";
     input.id = `preview${controlLabel}${laneId(lane)}Toggle`;
     input.type = "checkbox";
     input.checked = kind === "mute" ? this.previewLaneState[lane]?.muted === true : this.previewLaneState[lane]?.soloed === true;
     input.dataset[kind === "mute" ? "previewMuteLane" : "previewSoloLane"] = lane;
     input.setAttribute("aria-label", `${controlLabel} ${laneLabel(lane)}`);
-    label.className = "tool-starter__toggle midi-studio-v2__lane-toggle";
+    label.className = `tool-starter__toggle midi-studio-v2__lane-toggle midi-studio-v2__lane-toggle--${kind}`;
+    label.classList.toggle("is-active", input.checked);
+    label.dataset.laneControlKind = kind;
     label.htmlFor = input.id;
-    text.textContent = controlLabel;
+    icon.className = "midi-studio-v2__lane-toggle-icon";
+    icon.setAttribute("aria-hidden", "true");
+    this.preventPointerFocusScroll(label);
     input.addEventListener("change", () => {
-      if (kind === "mute") {
-        this.previewLaneState[lane].muted = input.checked;
-      } else {
-        this.previewLaneState[lane].soloed = input.checked;
-      }
-      this.onLaneSettingChange?.(kind, {
-        enabled: input.checked,
-        lane,
-        laneLabel: laneLabel(lane)
+      this.runInstrumentControlAction(() => {
+        label.classList.toggle("is-active", input.checked);
+        if (kind === "mute") {
+          this.previewLaneState[lane].muted = input.checked;
+        } else {
+          this.previewLaneState[lane].soloed = input.checked;
+        }
+        this.onLaneSettingChange?.(kind, {
+          enabled: input.checked,
+          lane,
+          laneLabel: laneLabel(lane)
+        });
       });
     });
-    label.append(input, text);
+    label.append(input, icon);
     return { input, label };
   }
 
@@ -1199,10 +1217,64 @@ export class InstrumentGridControl {
     button.setAttribute("aria-label", `Delete instrument row ${laneLabel(lane)}`);
     button.title = "Delete instrument row";
     button.textContent = "x";
-    button.addEventListener("click", () => this.deleteInstrumentRow(lane));
+    this.preventPointerFocusScroll(button);
+    button.addEventListener("click", () => {
+      this.runInstrumentControlAction(() => this.deleteInstrumentRow(lane));
+    });
     return button;
   }
 
+  preventPointerFocusScroll(element) {
+    element.addEventListener("pointerdown", (event) => {
+      event.preventDefault();
+    });
+  }
+
+  runInstrumentControlAction(action) {
+    const scrollState = this.captureInstrumentScrollState();
+    action();
+    this.restoreInstrumentScrollState(scrollState);
+  }
+
+  captureInstrumentScrollState() {
+    const leftPanel = this.instrumentList?.closest(".tool-starter__panel--left") || null;
+    const instrumentPanel = this.instrumentList?.closest(".midi-studio-v2__instrument-list-panel") || null;
+    return {
+      instrumentPanel,
+      instrumentScrollLeft: instrumentPanel?.scrollLeft || 0,
+      instrumentScrollTop: instrumentPanel?.scrollTop || 0,
+      leftPanel,
+      leftScrollLeft: leftPanel?.scrollLeft || 0,
+      leftScrollTop: leftPanel?.scrollTop || 0,
+      windowScrollX: this.window.scrollX || 0,
+      windowScrollY: this.window.scrollY || 0
+    };
+  }
+
+  restoreInstrumentScrollState(scrollState) {
+    this.applyInstrumentScrollState(scrollState);
+    this.window.requestAnimationFrame?.(() => this.applyInstrumentScrollState(scrollState));
+  }
+
+  applyInstrumentScrollState(scrollState) {
+    if (!scrollState) {
+      return;
+    }
+    if (scrollState.leftPanel) {
+      scrollState.leftPanel.scrollTop = scrollState.leftScrollTop;
+      scrollState.leftPanel.scrollLeft = scrollState.leftScrollLeft;
+    }
+    if (scrollState.instrumentPanel) {
+      scrollState.instrumentPanel.scrollTop = scrollState.instrumentScrollTop;
+      scrollState.instrumentPanel.scrollLeft = scrollState.instrumentScrollLeft;
+    }
+    this.window.scrollTo(scrollState.windowScrollX, scrollState.windowScrollY);
+    const activeElement = this.window.document.activeElement;
+    if (activeElement?.closest?.(".midi-studio-v2__instrument-control-row")) {
+      activeElement.blur();
+    }
+  }
+
   addInstrumentRow() {
     const lane = this.nextInstrumentLaneName();
     this.extraLaneSources[lane] = "";