From eead27dbd8870ea01de47b063244a400598a781d Mon Sep 17 00:00:00 2001
From: Xiaohan-Tian <157918347+Xiaohan-Tian@users.noreply.github.com>
Date: Thu, 7 May 2026 23:54:08 -0700
Subject: [PATCH] fix: misc UI updates and fixes

---
 src/components/Toolbar.tsx                    |  10 +-
 src/components/piano-roll/PianoRoll.css       |  51 +++----
 src/components/piano-roll/PianoRoll.tsx       |  14 +-
 .../piano-roll/PianoRollAutomationLane.tsx    | 137 +++++++++++-------
 .../piano-roll/PianoRollContent.tsx           |  39 ++---
 src/index.css                                 |   8 +-
 6 files changed, 150 insertions(+), 109 deletions(-)

diff --git a/src/components/Toolbar.tsx b/src/components/Toolbar.tsx
index c65bc63..aabae6c 100644
--- a/src/components/Toolbar.tsx
+++ b/src/components/Toolbar.tsx
@@ -48,7 +48,7 @@ const Toolbar: React.FC = () => {
     barWidthMultiplier, setBarWidthMultiplier,
     isLooping, toggleLoop,
     canUndo, canRedo, undoDescription, redoDescription, undo, redo,
-    toggleChatBox, toggleSettings, toggleKGOnePanel, toggleListEventPanel, showKGOnePanel, showListEventPanel, showChatBox, cleanupProjectState, toggleMetronome, isMetronomeEnabled,
+    toggleChatBox, toggleSettings, toggleKGOnePanel, toggleListEventPanel, showKGOnePanel, showListEventPanel, showChatBox, showSettings, cleanupProjectState, toggleMetronome, isMetronomeEnabled,
     isRecording, startRecording, stopRecording,
     // Piano roll state/actions
     showPianoRoll, setShowPianoRoll, activeRegionId, setActiveRegionId,
@@ -1065,7 +1065,13 @@ const Toolbar: React.FC = () => {
           </div>
         </div>
         <button title="Import" onClick={handleImportProject}><FaUpload /></button>
-        <button title="Settings" onClick={handleSettingsClick}><FaCog /></button>
+        <button
+          title="Settings"
+          className={`tool-button ${showSettings ? 'active' : ''}`}
+          onClick={handleSettingsClick}
+        >
+          <FaCog />
+        </button>
         <div className="toolbar-separator"></div>
         <button title="Undo" onClick={handleUndoClick}><FaUndo /></button>
         <button title="Redo" onClick={handleRedoClick}><FaRedo /></button>
diff --git a/src/components/piano-roll/PianoRoll.css b/src/components/piano-roll/PianoRoll.css
index ee26466..6ec135b 100644
--- a/src/components/piano-roll/PianoRoll.css
+++ b/src/components/piano-roll/PianoRoll.css
@@ -180,6 +180,14 @@
   --toolbar-button-bg: #d6d6d6;
 }
 
+.piano-roll-toolbar button:focus,
+.piano-roll-toolbar button:focus-visible,
+.piano-roll-toolbar .tool-button:focus,
+.piano-roll-toolbar .tool-button:focus-visible {
+  outline: none;
+  box-shadow: none;
+}
+
 /* Override pointer-events for piano roll toolbar sections */
 .piano-roll-toolbar .toolbar-left,
 .piano-roll-toolbar .toolbar-right {
@@ -310,8 +318,7 @@
   display: flex;
   flex-direction: column;
   height: 100%;
-  overflow-x: auto;
-  overflow-y: hidden;
+  overflow: hidden;
 }
 
 .piano-roll-main-section {
@@ -319,7 +326,7 @@
   flex-direction: column;
   flex: 1;
   min-height: 0;
-  min-width: calc(var(--max-number-of-bars) * var(--region-grid-bar-width) + var(--region-piano-key-width));
+  min-width: 0;
 }
 
 .piano-roll-main-section.with-automation {
@@ -329,34 +336,12 @@
 .piano-roll-note-scroll {
   flex: 1;
   min-height: 0;
-  overflow-y: auto;
-  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;
+  overflow: auto;
 }
 
 .piano-roll-automation-section {
   flex: 0 0 50%;
   min-height: 0;
-  min-width: calc(var(--max-number-of-bars) * var(--region-grid-bar-width) + var(--region-piano-key-width));
   border-top: 1px solid #3a3a3a;
   overflow: hidden;
 }
@@ -416,7 +401,11 @@
   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: 15;
 }
 
 .piano-grid-container {
@@ -523,6 +512,13 @@
   min-width: calc(var(--max-number-of-bars) * var(--region-grid-bar-width) + var(--region-piano-key-width));
 }
 
+.piano-roll-automation-scroll-layer {
+  position: absolute;
+  inset: 0;
+  will-change: transform;
+  z-index: 0;
+}
+
 .piano-roll-automation-track::before {
   content: '';
   position: absolute;
@@ -540,6 +536,7 @@
   left: 0;
   width: var(--region-piano-key-width);
   background: linear-gradient(180deg, #343434 0%, #2e2e2e 100%);
+  z-index: 2;
 }
 
 .piano-roll-automation-grid {
@@ -561,7 +558,7 @@
   position: absolute;
   top: 8px;
   left: 10px;
-  z-index: 1;
+  z-index: 3;
   font-size: 10px;
   font-weight: 700;
   letter-spacing: 0.04em;
diff --git a/src/components/piano-roll/PianoRoll.tsx b/src/components/piano-roll/PianoRoll.tsx
index e506525..7fd5065 100644
--- a/src/components/piano-roll/PianoRoll.tsx
+++ b/src/components/piano-roll/PianoRoll.tsx
@@ -671,7 +671,7 @@ const PianoRoll: React.FC<PianoRollProps> = ({
   const handleZoomChange = useCallback((nextZoom: number) => {
     if (nextZoom === pianoRollZoom) return;
 
-    const container = pianoRollContentRef.current;
+    const container = pianoRollNoteScrollRef.current;
     pendingZoomAnchorBeatRef.current = null;
     if (container) {
       const keysWidth = parseInt(
@@ -742,7 +742,7 @@ const PianoRoll: React.FC<PianoRollProps> = ({
 
   // Detect manual horizontal scroll during playback
   useEffect(() => {
-    const container = pianoRollContentRef.current;
+    const container = pianoRollNoteScrollRef.current;
     if (!container) return;
 
     const handleScroll = () => {
@@ -761,7 +761,7 @@ const PianoRoll: React.FC<PianoRollProps> = ({
   useEffect(() => {
     if (!isPlaying || !autoScrollEnabled) return;
 
-    const container = pianoRollContentRef.current;
+    const container = pianoRollNoteScrollRef.current;
     if (!container) return;
 
     const beatWidth = parseInt(
@@ -787,7 +787,7 @@ const PianoRoll: React.FC<PianoRollProps> = ({
   useEffect(() => {
     if (pianoRollScrollRequest === null) return;
 
-    const container = pianoRollContentRef.current;
+    const container = pianoRollNoteScrollRef.current;
     if (!container) return;
 
     const beatWidth = parseInt(
@@ -820,7 +820,7 @@ const PianoRoll: React.FC<PianoRollProps> = ({
     }
 
     const anchorBeat = pendingZoomAnchorBeatRef.current;
-    const container = pianoRollContentRef.current;
+    const container = pianoRollNoteScrollRef.current;
     if (anchorBeat !== null && container) {
       const keysWidth = parseInt(
         getComputedStyle(document.documentElement).getPropertyValue('--region-piano-key-width')
@@ -845,7 +845,7 @@ const PianoRoll: React.FC<PianoRollProps> = ({
 
   // Scroll horizontally to the active region's starting bar
   useEffect(() => {
-    if (pianoRollContentRef.current && activeRegion) {
+    if (pianoRollNoteScrollRef.current && activeRegion) {
       // Get the starting beat of the region
       const startBeat = activeRegion.getStartFromBeat();
       
@@ -866,7 +866,7 @@ const PianoRoll: React.FC<PianoRollProps> = ({
       const scrollPosition = barNumber * barWidth;
       
       // Scroll to the calculated position
-      pianoRollContentRef.current.scrollLeft = Math.max(0, scrollPosition);
+      pianoRollNoteScrollRef.current.scrollLeft = Math.max(0, scrollPosition);
     }
   }, [activeRegion, timeSignature]);
 
diff --git a/src/components/piano-roll/PianoRollAutomationLane.tsx b/src/components/piano-roll/PianoRollAutomationLane.tsx
index 10acde9..ee4ef85 100644
--- a/src/components/piano-roll/PianoRollAutomationLane.tsx
+++ b/src/components/piano-roll/PianoRollAutomationLane.tsx
@@ -26,6 +26,8 @@ interface PianoRollAutomationLaneProps {
   timeSignature: { numerator: number; denominator: number };
   bpm?: number;
   redrawVersion?: number;
+  horizontalScrollLeft?: number;
+  onHorizontalWheel?: (delta: number) => void;
 }
 
 const AUTOMATION_COLOR = '#87CEFA';
@@ -38,6 +40,8 @@ const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
   maxBars,
   timeSignature,
   redrawVersion = 0,
+  horizontalScrollLeft = 0,
+  onHorizontalWheel,
 }) => {
   const laneRef = useRef<HTMLDivElement | null>(null);
   const [laneHeight, setLaneHeight] = useState(MIN_LANE_HEIGHT);
@@ -66,6 +70,28 @@ const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
     };
   }, []);
 
+  useEffect(() => {
+    const element = laneRef.current;
+    if (!element) {
+      return;
+    }
+
+    const handleWheel = (event: WheelEvent) => {
+      const horizontalDelta = Math.abs(event.deltaX) > 0 ? event.deltaX : (event.shiftKey ? event.deltaY : 0);
+      if (horizontalDelta === 0) {
+        return;
+      }
+
+      event.preventDefault();
+      onHorizontalWheel?.(horizontalDelta);
+    };
+
+    element.addEventListener('wheel', handleWheel, { passive: false });
+    return () => {
+      element.removeEventListener('wheel', handleWheel);
+    };
+  }, [onHorizontalWheel]);
+
   const points = useMemo<AutomationPoint[]>(() => {
     if (!activeRegion) {
       return [];
@@ -160,62 +186,67 @@ const PianoRollAutomationLane: React.FC<PianoRollAutomationLaneProps> = ({
     >
       <div className="piano-roll-automation-track" style={{ width: totalWidth }}>
         <div className="piano-roll-automation-gutter" />
-        <div className="piano-roll-automation-grid" />
         <div className="piano-roll-automation-lane-label">{laneLabel}</div>
-        <svg
-          className="piano-roll-automation-svg"
-          width="100%"
-          height={laneHeight}
-          viewBox={`0 0 ${beatWidth * totalBeats + keyWidth} ${laneHeight}`}
-          preserveAspectRatio="none"
+        <div
+          className="piano-roll-automation-scroll-layer"
+          style={{ transform: `translateX(-${horizontalScrollLeft}px)` }}
         >
-          {interpolationMode === 'linear' && points.length > 0 && (
-            <polyline
-              className="piano-roll-automation-line"
-              fill="none"
-              stroke={AUTOMATION_COLOR}
-              strokeWidth="2"
-              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(laneHeight - 6, point.y - 10));
+          <div className="piano-roll-automation-grid" />
+          <svg
+            className="piano-roll-automation-svg"
+            width="100%"
+            height={laneHeight}
+            viewBox={`0 0 ${beatWidth * totalBeats + keyWidth} ${laneHeight}`}
+            preserveAspectRatio="none"
+          >
+            {interpolationMode === 'linear' && points.length > 0 && (
+              <polyline
+                className="piano-roll-automation-line"
+                fill="none"
+                stroke={AUTOMATION_COLOR}
+                strokeWidth="2"
+                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(laneHeight - 6, point.y - 10));
 
-            return (
-              <g key={point.id}>
-                <circle
-                  className="piano-roll-automation-point"
-                  cx={point.x}
-                  cy={point.y}
-                  r="5"
-                  fill={AUTOMATION_COLOR}
-                  stroke="#1d2428"
-                  strokeWidth="2"
-                />
-                <text
-                  className="piano-roll-automation-value"
-                  x={point.x + 6}
-                  y={labelY}
-                  fill={AUTOMATION_COLOR}
-                >
-                  {point.label}
-                </text>
-              </g>
-            );
-          })}
-        </svg>
+              return (
+                <g key={point.id}>
+                  <circle
+                    className="piano-roll-automation-point"
+                    cx={point.x}
+                    cy={point.y}
+                    r="5"
+                    fill={AUTOMATION_COLOR}
+                    stroke="#1d2428"
+                    strokeWidth="2"
+                  />
+                  <text
+                    className="piano-roll-automation-value"
+                    x={point.x + 6}
+                    y={labelY}
+                    fill={AUTOMATION_COLOR}
+                  >
+                    {point.label}
+                  </text>
+                </g>
+              );
+            })}
+          </svg>
+        </div>
         {points.length === 0 && (
           <div className="piano-roll-automation-empty-state">
             No {laneLabel} events in this region
diff --git a/src/components/piano-roll/PianoRollContent.tsx b/src/components/piano-roll/PianoRollContent.tsx
index 4d34618..fa3ce4f 100644
--- a/src/components/piano-roll/PianoRollContent.tsx
+++ b/src/components/piano-roll/PianoRollContent.tsx
@@ -76,7 +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 [noteScrollLeft, setNoteScrollLeft] = useState(0);
   const handleSpectrogramLoadingChange = useCallback((loading: boolean) => {
     setSpectrogramLoading(loading);
   }, []);
@@ -87,6 +87,15 @@ const PianoRollContent: React.FC<PianoRollContentProps> = ({
   // Recording state
   const isRecording = useProjectStore(s => s.isRecording);
   const recordingNotes = useProjectStore(s => s.recordingNotes);
+  const forwardAutomationHorizontalWheel = useCallback((delta: number) => {
+    const container = noteScrollRef.current;
+    if (!container) {
+      return;
+    }
+
+    container.scrollLeft += delta;
+    setNoteScrollLeft(container.scrollLeft);
+  }, [noteScrollRef]);
 
   // Use the note operations hook for resize and drag functionality
   const {
@@ -290,23 +299,14 @@ const PianoRollContent: React.FC<PianoRollContentProps> = ({
         data-testid={showAutomationLane ? 'piano-roll-content-split' : 'piano-roll-content-single'}
       >
         <div className={`piano-roll-main-section ${showAutomationLane ? 'with-automation' : ''}`}>
-          <PianoGridHeader maxBars={maxBars} timeSignature={timeSignature} />
-
-          <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">
+          <div
+            className="piano-roll-note-scroll"
+            ref={noteScrollRef}
+            onScroll={(event) => setNoteScrollLeft(event.currentTarget.scrollLeft)}
+          >
+            <PianoGridHeader maxBars={maxBars} timeSignature={timeSignature} />
+            <div className="piano-roll-body">
+              <PianoKeys activeRegion={activeRegion} />
               <PianoGrid
                 gridRef={pianoGridRef}
                 onDoubleClick={isSpectrogram ? () => {} : handleGridDoubleClick}
@@ -331,7 +331,6 @@ const PianoRollContent: React.FC<PianoRollContentProps> = ({
                 {memoizedNotes}
                 {!isSpectrogram && recordingNoteOverlays}
               </PianoGrid>
-              </div>
             </div>
           </div>
         </div>
@@ -344,6 +343,8 @@ const PianoRollContent: React.FC<PianoRollContentProps> = ({
               timeSignature={timeSignature}
               bpm={bpm}
               redrawVersion={automationRedrawVersion}
+              horizontalScrollLeft={noteScrollLeft}
+              onHorizontalWheel={forwardAutomationHorizontalWheel}
             />
           </div>
         )}
diff --git a/src/index.css b/src/index.css
index a25c276..4bb29a6 100644
--- a/src/index.css
+++ b/src/index.css
@@ -50,6 +50,12 @@ button {
   transition: border-color 0.25s;
 }
 
+button:focus,
+button:focus-visible {
+  outline: none;
+  box-shadow: none;
+}
+
 textarea {
   font-family: 'Segoe UI', Tahoma, Geneva, Verdana, sans-serif;
   font-size: 12px;
@@ -73,4 +79,4 @@ input[type="range"]::-webkit-slider-thumb {
   background: #e0e0e0;
   border-radius: 50%;
   cursor: pointer;
-}
\ No newline at end of file
+}