Skip to content

Commit b3c3f84

Browse files
adewaleclaude
andcommitted
fix: Enable scrolling on published sessions while blocking editing
Published sessions had pointer-events: none on .sequencer-content which blocked all interactions including scrolling. Users couldn't see beyond the first ~30 steps on long sequences (e.g., 128-step tracks). Fix: Re-enable pointer-events on .tracks (allows scroll) while keeping pointer-events: none on .track-row (blocks editing cells). Added 7 integration tests to prevent regression. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 652fba6 commit b3c3f84

File tree

2 files changed

+124
-0
lines changed

2 files changed

+124
-0
lines changed

app/src/components/StepSequencer.css

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,13 @@
162162

163163
.step-sequencer.published .tracks {
164164
position: relative;
165+
/* Re-enable pointer events for scrolling, but children still blocked */
166+
pointer-events: auto;
167+
}
168+
169+
/* Block interactions on track rows (editing) while allowing container scroll */
170+
.step-sequencer.published .track-row {
171+
pointer-events: none;
165172
}
166173

167174
/* Glass/scrim overlay effect - subtle, doesn't block visibility */
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/**
2+
* Published Session UX Integration Tests
3+
*
4+
* These tests verify that published (immutable) sessions maintain proper UX:
5+
* - Scrolling should work (users need to see full grid)
6+
* - Editing should be blocked (read-only mode)
7+
* - Play/pause should work
8+
*
9+
* Bug fix: pointer-events:none on .sequencer-content was blocking scroll.
10+
* Fix: Re-enable pointer-events on .tracks while blocking on .track-row
11+
*/
12+
13+
import { describe, it, expect } from 'vitest';
14+
import * as fs from 'fs';
15+
import * as path from 'path';
16+
17+
// =============================================================================
18+
// CSS Rule Tests
19+
// =============================================================================
20+
21+
describe('Published Session CSS Rules', () => {
22+
// Load the CSS file
23+
const cssPath = path.join(__dirname, '../../src/components/StepSequencer.css');
24+
const cssContent = fs.readFileSync(cssPath, 'utf-8');
25+
26+
describe('Scroll behavior', () => {
27+
it('should block pointer events on sequencer-content for published sessions', () => {
28+
// This is the base rule that blocks editing
29+
expect(cssContent).toContain('.step-sequencer.published .sequencer-content');
30+
expect(cssContent).toMatch(/\.step-sequencer\.published\s+\.sequencer-content\s*\{[^}]*pointer-events:\s*none/);
31+
});
32+
33+
it('should re-enable pointer events on .tracks for scrolling', () => {
34+
// Critical fix: .tracks must have pointer-events: auto to allow scrolling
35+
expect(cssContent).toMatch(/\.step-sequencer\.published\s+\.tracks\s*\{[^}]*pointer-events:\s*auto/);
36+
});
37+
38+
it('should block pointer events on track-row to prevent editing', () => {
39+
// Track rows (containing step cells) should not be interactive
40+
expect(cssContent).toMatch(/\.step-sequencer\.published\s+\.track-row\s*\{[^}]*pointer-events:\s*none/);
41+
});
42+
});
43+
44+
describe('Transport controls', () => {
45+
it('should allow pointer events on transport for play/pause', () => {
46+
// Play/pause must work on published sessions
47+
expect(cssContent).toMatch(/\.step-sequencer\.published\s+\.transport\s*\{[^}]*pointer-events:\s*auto/);
48+
});
49+
50+
it('should block pointer events on tempo/swing controls', () => {
51+
// Tempo and swing should not be editable
52+
expect(cssContent).toContain('.step-sequencer.published .transport .tempo-control');
53+
expect(cssContent).toContain('.step-sequencer.published .transport .swing-control');
54+
expect(cssContent).toMatch(/pointer-events:\s*none/);
55+
});
56+
});
57+
});
58+
59+
// =============================================================================
60+
// Pointer Events Hierarchy Test
61+
// =============================================================================
62+
63+
describe('Pointer Events Hierarchy', () => {
64+
/**
65+
* CSS pointer-events inheritance:
66+
* - Parent with pointer-events: none blocks all children
67+
* - Child with pointer-events: auto re-enables for that subtree
68+
* - Grandchild with pointer-events: none blocks that branch
69+
*
70+
* For published sessions:
71+
* .sequencer-content (none) -> blocks all
72+
* .tracks (auto) -> re-enables scrolling
73+
* .track-row (none) -> blocks editing cells
74+
* .transport (auto) -> re-enables play button
75+
* .tempo-control (none) -> blocks tempo slider
76+
*/
77+
78+
it('documents the pointer-events hierarchy for published sessions', () => {
79+
const hierarchy = {
80+
'.sequencer-content': 'none', // Base: block everything
81+
'.tracks': 'auto', // Allow: scrolling the grid
82+
'.track-row': 'none', // Block: editing step cells
83+
'.transport': 'auto', // Allow: play/pause button
84+
'.tempo-control': 'none', // Block: tempo editing
85+
'.swing-control': 'none', // Block: swing editing
86+
};
87+
88+
// This test documents the expected hierarchy
89+
// If any of these change, this test should be updated
90+
expect(Object.keys(hierarchy).length).toBe(6);
91+
expect(hierarchy['.tracks']).toBe('auto'); // Critical for scroll fix
92+
});
93+
});
94+
95+
// =============================================================================
96+
// Regression Prevention
97+
// =============================================================================
98+
99+
describe('Published Session Regression Tests', () => {
100+
it('should not have .tracks with pointer-events: none', () => {
101+
// Regression: If .tracks ever gets pointer-events: none, scrolling breaks
102+
const cssPath = path.join(__dirname, '../../src/components/StepSequencer.css');
103+
const cssContent = fs.readFileSync(cssPath, 'utf-8');
104+
105+
// Extract the .step-sequencer.published .tracks rule
106+
const tracksRuleMatch = cssContent.match(
107+
/\.step-sequencer\.published\s+\.tracks\s*\{([^}]*)\}/
108+
);
109+
110+
expect(tracksRuleMatch).toBeTruthy();
111+
const tracksRule = tracksRuleMatch![1];
112+
113+
// Ensure it has pointer-events: auto (not none)
114+
expect(tracksRule).toContain('pointer-events: auto');
115+
expect(tracksRule).not.toMatch(/pointer-events:\s*none/);
116+
});
117+
});

0 commit comments

Comments
 (0)