From 1e33caafc5e04d65373fdbaec934264df2aafe8d Mon Sep 17 00:00:00 2001
From: Xiaohan-Tian <157918347+Xiaohan-Tian@users.noreply.github.com>
Date: Thu, 7 May 2026 23:28:16 -0700
Subject: [PATCH] fix: few minor UI adjustments and fixes

---
 src/components/ListEventPanel.css             | 17 +++--
 src/components/piano-roll/PianoRoll.css       | 46 ++++++++++--
 src/components/piano-roll/PianoRoll.tsx       | 13 +++-
 .../PianoRollAutomationLane.test.tsx          | 26 +++++++
 .../piano-roll/PianoRollAutomationLane.tsx    | 73 +++++++++++++++++--
 .../piano-roll/PianoRollContent.tsx           | 21 +++++-
 .../piano-roll/pianoRollAutomation.ts         | 17 +++--
 src/core/state/KGPianoRollState.ts            | 20 ++++-
 8 files changed, 199 insertions(+), 34 deletions(-)

diff --git a/src/components/ListEventPanel.css b/src/components/ListEventPanel.css
index cf15413..e9ed29a 100644
--- a/src/components/ListEventPanel.css
+++ b/src/components/ListEventPanel.css
@@ -106,8 +106,13 @@
   width: 22px;
   height: 22px;
   border: 1px solid #444;
-  border-radius: 3px;
-  background-color: #3a3a3a;
+  border-right: 0;
+  border-top-right-radius: 0;
+  border-bottom-right-radius: 0;
+  border-top-left-radius: 3px;
+  border-bottom-left-radius: 3px;
+  background-color: #2f2f2f;
+  box-sizing: border-box;
   color: #e0e0e0;
   display: flex;
   align-items: center;
@@ -120,8 +125,8 @@
 }
 
 .list-event-add-button:hover {
-  background-color: #464646;
-  border-color: #5a5a5a;
+  background-color: #3b3b3b;
+  border-right: 0;
 }
 
 .list-event-dropdown-button,
@@ -142,6 +147,8 @@
 .list-event-type-button {
   min-width: 88px;
   margin-left: 0;
+  border-top-left-radius: 0;
+  border-bottom-left-radius: 0;
 }
 
 .list-event-delete-button {
@@ -242,4 +249,4 @@
   font-size: 11px;
   line-height: 16px;
   outline: none;
-}
\ No newline at end of file
+}
diff --git a/src/components/piano-roll/PianoRoll.css b/src/components/piano-roll/PianoRoll.css
index 5445b1e..ee26466 100644
--- a/src/components/piano-roll/PianoRoll.css
+++ b/src/components/piano-roll/PianoRoll.css
@@ -219,6 +219,21 @@
   font-weight: 700;
 }
 
+.piano-roll-toolbar .automation-toggle-button.active {
+  border-color: #e0e0e0;
+  border-right: 0;
+}
+
+.piano-roll-toolbar .automation-toggle-button.active:hover {
+  border-color: #f0f0f0;
+  border-right: 0;
+}
+
+.piano-roll-toolbar .automation-toggle-button.active:active {
+  border-color: #d6d6d6;
+  border-right: 0;
+}
+
 .piano-roll-toolbar .automation-type-dropdown {
   min-width: 92px;
   height: 20px;
@@ -318,6 +333,26 @@
   overflow-x: visible;
 }
 
+.piano-roll-body-shell {
+  display: flex;
+  min-height: 0;
+}
+
+.piano-roll-keys-viewport {
+  position: sticky;
+  left: 0;
+  z-index: 15;
+  width: var(--region-piano-key-width);
+  flex: 0 0 var(--region-piano-key-width);
+  overflow: hidden;
+  border-right: 1px solid #3a3a3a;
+  background-color: #252525;
+}
+
+.piano-roll-keys-offset {
+  will-change: transform;
+}
+
 .piano-roll-automation-section {
   flex: 0 0 50%;
   min-height: 0;
@@ -374,24 +409,19 @@
 .piano-roll-body {
   display: flex;
   min-height: calc(8 * 12 * var(--region-piano-key-height));
-  /* 8 octaves * 12 notes * piano key height */
-  width: calc(var(--max-number-of-bars) * var(--region-grid-bar-width) + var(--region-piano-key-width));
-  /* 32 bars * 160px width + piano keys width */
+  width: calc(var(--max-number-of-bars) * var(--region-grid-bar-width));
 }
 
 .piano-keys-container {
   width: var(--region-piano-key-width);
   flex-shrink: 0;
   overflow: hidden;
-  border-right: 1px solid #3a3a3a;
   background-color: #252525;
-  position: sticky;
-  left: 0;
-  z-index: 10;
 }
 
 .piano-grid-container {
   flex: 1;
+  width: 100%;
   min-width: calc(var(--max-number-of-bars) * var(--region-grid-bar-width));
   min-height: calc(8 * 12 * var(--region-piano-key-height));
   z-index: 5;
@@ -609,4 +639,4 @@
   color: #e0e0e0;
   min-width: 20px;
   text-align: center;
-}
\ No newline at end of file
+}
diff --git a/src/components/piano-roll/PianoRoll.tsx b/src/components/piano-roll/PianoRoll.tsx
index 852a5f9..e506525 100644
--- a/src/components/piano-roll/PianoRoll.tsx
+++ b/src/components/piano-roll/PianoRoll.tsx
@@ -205,6 +205,8 @@ const PianoRoll: React.FC<PianoRollProps> = ({
     // Sync tool state
     const currentTool = pianoRollState.getActiveTool() as 'pointer' | 'pencil';
     setActiveTool(currentTool);
+    setAutomationEnabled(pianoRollState.getAutomationViewEnabled());
+    setAutomationType(pianoRollState.getCurrentAutomationType() as PianoRollAutomationType);
     
     if (DEBUG_MODE.PIANO_ROLL) {
       console.log(`Synced piano roll state on mount - snap: ${currentSnap}, tool: ${currentTool}`);
@@ -691,11 +693,16 @@ const PianoRoll: React.FC<PianoRollProps> = ({
   }, [pianoRollZoom]);
 
   const handleAutomationToggle = useCallback(() => {
-    setAutomationEnabled(current => !current);
+    setAutomationEnabled(current => {
+      const next = !current;
+      KGPianoRollState.instance().setAutomationViewEnabled(next);
+      return next;
+    });
   }, []);
 
   const handleAutomationTypeChange = useCallback((value: PianoRollAutomationType) => {
     setAutomationType(value);
+    KGPianoRollState.instance().setCurrentAutomationType(value);
   }, []);
 
   // Calculate C4 position and scroll to it when piano roll opens
@@ -745,7 +752,9 @@ const PianoRoll: React.FC<PianoRollProps> = ({
     };
 
     container.addEventListener('scroll', handleScroll);
-    return () => container.removeEventListener('scroll', handleScroll);
+    return () => {
+      container.removeEventListener('scroll', handleScroll);
+    };
   }, []);
 
   // Auto-scroll to keep playhead centered during playback
diff --git a/src/components/piano-roll/PianoRollAutomationLane.test.tsx b/src/components/piano-roll/PianoRollAutomationLane.test.tsx
index ce34378..d9fe49c 100644
--- a/src/components/piano-roll/PianoRollAutomationLane.test.tsx
+++ b/src/components/piano-roll/PianoRollAutomationLane.test.tsx
@@ -74,4 +74,30 @@ describe('PianoRollAutomationLane', () => {
 
     expect(screen.getByText('No CC64 events in this region')).toBeInTheDocument();
   });
+
+  it('renders step-style hold segments for non-interpolatable automation', () => {
+    const controllerEventsByType: KGMidiControllerEvent[][] = Array.from({ length: 128 }, () => []);
+    controllerEventsByType[64] = [
+      createMockMidiControllerEvent({ id: 'cc64-1', beat: 0.5, value: 127 }),
+      createMockMidiControllerEvent({ id: 'cc64-2', beat: 2, value: 0 }),
+    ];
+
+    const region = createMockMidiRegion({
+      startFromBeat: 0,
+      controllerEventsByType,
+    });
+
+    const { container } = render(
+      <PianoRollAutomationLane
+        activeRegion={region}
+        automationType="cc-64"
+        maxBars={8}
+        timeSignature={{ numerator: 4, denominator: 4 }}
+      />
+    );
+
+    expect(container.querySelector('.piano-roll-automation-line')).not.toBeNull();
+    expect(container.querySelector('polyline.piano-roll-automation-line')).toBeNull();
+    expect(container.querySelectorAll('line.piano-roll-automation-line')).toHaveLength(2);
+  });
 });
diff --git a/src/components/piano-roll/PianoRollAutomationLane.tsx b/src/components/piano-roll/PianoRollAutomationLane.tsx
index 44bb398..10acde9 100644
--- a/src/components/piano-roll/PianoRollAutomationLane.tsx
+++ b/src/components/piano-roll/PianoRollAutomationLane.tsx
@@ -1,4 +1,4 @@
-import React, { useMemo } from 'react';
+import React, { useEffect, useMemo, useRef, useState } from 'react';
 import { KGMidiRegion } from '../../core/region/KGMidiRegion';
 import {
   MIDI_PITCH_BEND_MAX,
@@ -6,6 +6,7 @@ import {
   midiPitchBendToSignedValue,
 } from '../../util/midiUtil';
 import {
+  getAutomationInterpolationMode,
   getControllerNumberForAutomationType,
   PIANO_ROLL_AUTOMATION_OPTIONS,
   type PianoRollAutomationType,
@@ -28,8 +29,8 @@ interface PianoRollAutomationLaneProps {
 }
 
 const AUTOMATION_COLOR = '#87CEFA';
-const LANE_HEIGHT = 160;
 const LANE_PADDING_Y = 16;
+const MIN_LANE_HEIGHT = 160;
 
 const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
   activeRegion,
@@ -38,8 +39,33 @@ const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
   timeSignature,
   redrawVersion = 0,
 }) => {
+  const laneRef = useRef<HTMLDivElement | null>(null);
+  const [laneHeight, setLaneHeight] = useState(MIN_LANE_HEIGHT);
   const beatWidth = parseInt(getComputedStyle(document.documentElement).getPropertyValue('--region-grid-beat-width')) || 40;
   const keyWidth = parseInt(getComputedStyle(document.documentElement).getPropertyValue('--region-piano-key-width')) || 60;
+
+  useEffect(() => {
+    const element = laneRef.current;
+    if (!element) {
+      return;
+    }
+
+    const updateHeight = () => {
+      setLaneHeight(Math.max(Math.round(element.clientHeight), MIN_LANE_HEIGHT));
+    };
+
+    updateHeight();
+
+    const observer = new ResizeObserver(() => {
+      updateHeight();
+    });
+    observer.observe(element);
+
+    return () => {
+      observer.disconnect();
+    };
+  }, []);
+
   const points = useMemo<AutomationPoint[]>(() => {
     if (!activeRegion) {
       return [];
@@ -70,15 +96,16 @@ const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
 
   const selectedOption = PIANO_ROLL_AUTOMATION_OPTIONS.find(option => option.value === automationType);
   const laneLabel = selectedOption?.label ?? automationType;
+  const interpolationMode = getAutomationInterpolationMode(automationType);
   const totalBeats = maxBars * timeSignature.numerator;
   const totalWidth = 'calc(var(--max-number-of-bars) * var(--region-grid-bar-width) + var(--region-piano-key-width))';
 
   const toY = (value: number): number => {
     const minValue = automationType === 'pitch-bend' ? MIDI_PITCH_BEND_MIN : 0;
     const maxValue = automationType === 'pitch-bend' ? MIDI_PITCH_BEND_MAX : 127;
-    const usableHeight = LANE_HEIGHT - LANE_PADDING_Y * 2;
+    const usableHeight = laneHeight - LANE_PADDING_Y * 2;
     const normalized = (value - minValue) / (maxValue - minValue);
-    return LANE_HEIGHT - LANE_PADDING_Y - normalized * usableHeight;
+    return laneHeight - LANE_PADDING_Y - normalized * usableHeight;
   };
 
   const svgPoints = points.map(point => {
@@ -90,6 +117,10 @@ const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
   });
 
   const polylinePoints = (() => {
+    if (interpolationMode === 'step') {
+      return '';
+    }
+
     if (svgPoints.length === 0) {
       return '';
     }
@@ -107,11 +138,25 @@ const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
       .join(' ');
   })();
 
+  const stepSegments = (() => {
+    if (interpolationMode !== 'step' || svgPoints.length === 0) {
+      return [];
+    }
+
+    return svgPoints.map((point, index) => ({
+      id: `${point.id}-step`,
+      x1: point.x,
+      y1: point.y,
+      x2: index < svgPoints.length - 1 ? svgPoints[index + 1].x : beatWidth * totalBeats + keyWidth,
+    }));
+  })();
+
   return (
     <div
       className="piano-roll-automation-lane"
       data-testid="piano-roll-automation-lane"
       aria-label={`${laneLabel} automation lane`}
+      ref={laneRef}
     >
       <div className="piano-roll-automation-track" style={{ width: totalWidth }}>
         <div className="piano-roll-automation-gutter" />
@@ -120,11 +165,11 @@ const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
         <svg
           className="piano-roll-automation-svg"
           width="100%"
-          height={LANE_HEIGHT}
-          viewBox={`0 0 ${beatWidth * totalBeats + keyWidth} ${LANE_HEIGHT}`}
+          height={laneHeight}
+          viewBox={`0 0 ${beatWidth * totalBeats + keyWidth} ${laneHeight}`}
           preserveAspectRatio="none"
         >
-          {points.length > 0 && (
+          {interpolationMode === 'linear' && points.length > 0 && (
             <polyline
               className="piano-roll-automation-line"
               fill="none"
@@ -133,8 +178,20 @@ const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
               points={polylinePoints}
             />
           )}
+          {interpolationMode === 'step' && stepSegments.map(segment => (
+            <line
+              key={segment.id}
+              className="piano-roll-automation-line"
+              x1={segment.x1}
+              y1={segment.y1}
+              x2={segment.x2}
+              y2={segment.y1}
+              stroke={AUTOMATION_COLOR}
+              strokeWidth="2"
+            />
+          ))}
           {svgPoints.map(point => {
-            const labelY = Math.max(14, Math.min(LANE_HEIGHT - 6, point.y - 10));
+            const labelY = Math.max(14, Math.min(laneHeight - 6, point.y - 10));
 
             return (
               <g key={point.id}>
diff --git a/src/components/piano-roll/PianoRollContent.tsx b/src/components/piano-roll/PianoRollContent.tsx
index f03d30c..4d34618 100644
--- a/src/components/piano-roll/PianoRollContent.tsx
+++ b/src/components/piano-roll/PianoRollContent.tsx
@@ -76,6 +76,7 @@ const PianoRollContent: React.FC<PianoRollContentProps> = ({
   const isSpectrogram = mode === 'spectrogram';
   const showAutomationLane = automationEnabled && !isSpectrogram;
   const [spectrogramLoading, setSpectrogramLoading] = useState(false);
+  const [noteScrollTop, setNoteScrollTop] = useState(0);
   const handleSpectrogramLoadingChange = useCallback((loading: boolean) => {
     setSpectrogramLoading(loading);
   }, []);
@@ -291,10 +292,21 @@ const PianoRollContent: React.FC<PianoRollContentProps> = ({
         <div className={`piano-roll-main-section ${showAutomationLane ? 'with-automation' : ''}`}>
           <PianoGridHeader maxBars={maxBars} timeSignature={timeSignature} />
 
-          <div className="piano-roll-note-scroll" ref={noteScrollRef}>
-            <div className="piano-roll-body">
-              <PianoKeys activeRegion={activeRegion} />
-
+          <div className="piano-roll-body-shell">
+            <div className="piano-roll-keys-viewport">
+              <div
+                className="piano-roll-keys-offset"
+                style={{ transform: `translateY(-${noteScrollTop}px)` }}
+              >
+                <PianoKeys activeRegion={activeRegion} />
+              </div>
+            </div>
+            <div
+              className="piano-roll-note-scroll"
+              ref={noteScrollRef}
+              onScroll={(event) => setNoteScrollTop(event.currentTarget.scrollTop)}
+            >
+              <div className="piano-roll-body">
               <PianoGrid
                 gridRef={pianoGridRef}
                 onDoubleClick={isSpectrogram ? () => {} : handleGridDoubleClick}
@@ -319,6 +331,7 @@ const PianoRollContent: React.FC<PianoRollContentProps> = ({
                 {memoizedNotes}
                 {!isSpectrogram && recordingNoteOverlays}
               </PianoGrid>
+              </div>
             </div>
           </div>
         </div>
diff --git a/src/components/piano-roll/pianoRollAutomation.ts b/src/components/piano-roll/pianoRollAutomation.ts
index ae8c7e5..902b415 100644
--- a/src/components/piano-roll/pianoRollAutomation.ts
+++ b/src/components/piano-roll/pianoRollAutomation.ts
@@ -9,15 +9,16 @@ export type PianoRollAutomationType =
 export interface PianoRollAutomationOption {
   label: string;
   value: PianoRollAutomationType;
+  interpolationMode: 'linear' | 'step';
 }
 
 export const PIANO_ROLL_AUTOMATION_OPTIONS: PianoRollAutomationOption[] = [
-  { label: 'Pitch Bend', value: 'pitch-bend' },
-  { label: 'CC1', value: 'cc-1' },
-  { label: 'CC2', value: 'cc-2' },
-  { label: 'CC7', value: 'cc-7' },
-  { label: 'CC11', value: 'cc-11' },
-  { label: 'CC64', value: 'cc-64' },
+  { label: 'Pitch Bend', value: 'pitch-bend', interpolationMode: 'linear' },
+  { label: 'CC1', value: 'cc-1', interpolationMode: 'linear' },
+  { label: 'CC2', value: 'cc-2', interpolationMode: 'linear' },
+  { label: 'CC7', value: 'cc-7', interpolationMode: 'linear' },
+  { label: 'CC11', value: 'cc-11', interpolationMode: 'linear' },
+  { label: 'CC64', value: 'cc-64', interpolationMode: 'step' },
 ];
 
 export function getControllerNumberForAutomationType(type: PianoRollAutomationType): number | null {
@@ -37,3 +38,7 @@ export function getControllerNumberForAutomationType(type: PianoRollAutomationTy
       return null;
   }
 }
+
+export function getAutomationInterpolationMode(type: PianoRollAutomationType): 'linear' | 'step' {
+  return PIANO_ROLL_AUTOMATION_OPTIONS.find(option => option.value === type)?.interpolationMode ?? 'linear';
+}
diff --git a/src/core/state/KGPianoRollState.ts b/src/core/state/KGPianoRollState.ts
index bcef9b9..7afe3c5 100644
--- a/src/core/state/KGPianoRollState.ts
+++ b/src/core/state/KGPianoRollState.ts
@@ -13,6 +13,8 @@ export class KGPianoRollState {
   private currentSnap: string = "NO SNAP";
   private lastEditedNoteLength: number = 1; // Default to 1 beat
   private currentMode: string = "ionian"; // Default mode
+  private automationViewEnabled: boolean = false;
+  private currentAutomationType: string = "pitch-bend";
 
   // Chord guide state
   private currentSuitableChords: Record<string, string[]> = {}; // Map of chord symbols to note names (e.g., {"I": ["C", "E", "G"]})
@@ -65,6 +67,22 @@ export class KGPianoRollState {
     this.currentMode = mode;
   }
 
+  public getAutomationViewEnabled(): boolean {
+    return this.automationViewEnabled;
+  }
+
+  public setAutomationViewEnabled(enabled: boolean): void {
+    this.automationViewEnabled = enabled;
+  }
+
+  public getCurrentAutomationType(): string {
+    return this.currentAutomationType;
+  }
+
+  public setCurrentAutomationType(type: string): void {
+    this.currentAutomationType = type;
+  }
+
   public getCurrentSuitableChords(): Record<string, string[]> {
     return this.currentSuitableChords;
   }
@@ -104,4 +122,4 @@ export class KGPianoRollState {
   public setCurrentChordCursorPitch(pitch: number | null): void {
     this.currentChordCursorPitch = pitch;
   }
-}
\ No newline at end of file
+}