Skip to content

Commit 41b0a68

Browse files
committed
refactor(studio): give resolveTimelineMove a row-based vertical axis
Rows stopped sharing one pixel height when lanes gained expansion, so the only production caller was passing cumulative row coordinates with trackHeight 1 and both scrollTops zeroed. The parameter names described units the values no longer carried. The vertical axis is now a row index and the caller keeps ownership of folding scroll and per-row heights into it.
1 parent 9f761c6 commit 41b0a68

3 files changed

Lines changed: 29 additions & 42 deletions

File tree

packages/studio/src/player/components/timelineClipDragPreview.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,22 +139,18 @@ export function computeDragPreview(
139139
ctx.rowHeights,
140140
);
141141
const currentRow = getTimelineRowFromY(clientY - scrollRectTop + scrollTop, ctx.rowHeights);
142-
// resolveTimelineMove's vertical axis is expressed in track-height units.
143-
// Feeding cumulative row coordinates with a unit height preserves its existing
144-
// threshold/create-track behavior while supporting variable pixel heights.
142+
// resolveTimelineMove's vertical axis is row indices, which is why the pointer
143+
// and scroll pixels are folded into originRow/currentRow above.
145144
const nextMove = resolveTimelineMove(
146145
{
147146
start: drag.element.start,
148147
track: drag.element.track,
149148
duration: drag.element.duration,
150149
originClientX: drag.originClientX,
151-
originClientY: originRow,
150+
originRow,
152151
originScrollLeft: drag.originScrollLeft,
153-
originScrollTop: 0,
154152
currentScrollLeft: scroll?.scrollLeft ?? drag.originScrollLeft,
155-
currentScrollTop: 0,
156153
pixelsPerSecond: pps,
157-
trackHeight: 1,
158154
maxStart: dragMaxStart,
159155
trackOrder,
160156
},

packages/studio/src/player/components/timelineEditing.test.ts

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,13 @@ describe("resolveTimelineMove", () => {
2727
track: 2,
2828
duration: 2,
2929
originClientX: 100,
30-
originClientY: 200,
30+
originRow: 0,
3131
pixelsPerSecond: 100,
32-
trackHeight: 72,
3332
maxStart: 8,
3433
trackOrder: [0, 1, 2, 3, 4],
3534
},
3635
245,
37-
200,
36+
0,
3837
),
3938
).toEqual({ start: 2.7, track: 2 });
4039
});
@@ -47,14 +46,13 @@ describe("resolveTimelineMove", () => {
4746
track: 1,
4847
duration: 3,
4948
originClientX: 200,
50-
originClientY: 200,
49+
originRow: 0,
5150
pixelsPerSecond: 100,
52-
trackHeight: 72,
5351
maxStart: 10,
5452
trackOrder: [0, 1, 5, 9],
5553
},
5654
150,
57-
390,
55+
190 / 72,
5856
),
5957
).toEqual({ start: 1.5, track: 9 });
6058
});
@@ -67,14 +65,13 @@ describe("resolveTimelineMove", () => {
6765
track: 0,
6866
duration: 4,
6967
originClientX: 300,
70-
originClientY: 200,
68+
originRow: 0,
7169
pixelsPerSecond: 100,
72-
trackHeight: 72,
7370
maxStart: 6,
7471
trackOrder: [0, 10, 20],
7572
},
7673
-100,
77-
-200,
74+
-400 / 72,
7875
),
7976
).toEqual({ start: 0, track: -1 });
8077

@@ -85,14 +82,13 @@ describe("resolveTimelineMove", () => {
8582
track: 10,
8683
duration: 4,
8784
originClientX: 300,
88-
originClientY: 200,
85+
originRow: 0,
8986
pixelsPerSecond: 100,
90-
trackHeight: 72,
9187
maxStart: 6,
9288
trackOrder: [0, 10, 20],
9389
},
9490
500,
95-
200,
91+
0,
9692
),
9793
).toEqual({ start: 6, track: 10 });
9894
});
@@ -105,14 +101,13 @@ describe("resolveTimelineMove", () => {
105101
track: 0,
106102
duration: 2,
107103
originClientX: 100,
108-
originClientY: 200,
104+
originRow: 0,
109105
pixelsPerSecond: 100,
110-
trackHeight: 72,
111106
maxStart: 8,
112107
trackOrder: [0, 10, 20],
113108
},
114109
100,
115-
150,
110+
-50 / 72,
116111
),
117112
).toEqual({ start: 1, track: -1 });
118113
});
@@ -125,38 +120,36 @@ describe("resolveTimelineMove", () => {
125120
track: 20,
126121
duration: 2,
127122
originClientX: 100,
128-
originClientY: 200,
123+
originRow: 0,
129124
pixelsPerSecond: 100,
130-
trackHeight: 72,
131125
maxStart: 8,
132126
trackOrder: [0, 10, 20],
133127
},
134128
100,
135-
250,
129+
50 / 72,
136130
),
137131
).toEqual({ start: 1, track: 21 });
138132
});
139133

140-
it("accounts for scroll displacement while dragging", () => {
134+
it("accounts for horizontal scroll displacement while dragging", () => {
141135
expect(
142136
resolveTimelineMove(
143137
{
144138
start: 1,
145139
track: 0,
146140
duration: 2,
147141
originClientX: 100,
148-
originClientY: 200,
142+
originRow: 0,
149143
originScrollLeft: 0,
150-
originScrollTop: 0,
151144
currentScrollLeft: 100,
152-
currentScrollTop: 144,
153145
pixelsPerSecond: 100,
154-
trackHeight: 72,
155146
maxStart: 8,
147+
// Vertical scroll never reaches here: the drag preview folds it into the
148+
// row index it passes, because rows no longer share one pixel height.
156149
trackOrder: [0, 1, 2, 3],
157150
},
158151
100,
159-
200,
152+
2,
160153
),
161154
).toEqual({ start: 2, track: 2 });
162155
});
@@ -195,9 +188,8 @@ describe("resolveTimelineMove", () => {
195188
track: 1,
196189
duration: 2,
197190
originClientX: 0,
198-
originClientY: 0,
191+
originRow: 0,
199192
pixelsPerSecond: 100,
200-
trackHeight: 72,
201193
maxStart: 8,
202194
trackOrder: [0, 1],
203195
layerOrder: layers.map((layer) => layer.id),
@@ -206,7 +198,7 @@ describe("resolveTimelineMove", () => {
206198
stackingElements,
207199
},
208200
0,
209-
-72,
201+
-1,
210202
);
211203

212204
expect(result).toEqual({

packages/studio/src/player/components/timelineEditing.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,11 @@ export interface TimelineMoveInput {
4646
track: number;
4747
duration: number;
4848
originClientX: number;
49-
originClientY: number;
49+
/** Vertical position as a track-row index, not pixels: rows vary in height. */
50+
originRow: number;
5051
originScrollLeft?: number;
51-
originScrollTop?: number;
5252
currentScrollLeft?: number;
53-
currentScrollTop?: number;
5453
pixelsPerSecond: number;
55-
trackHeight: number;
5654
maxStart: number;
5755
trackOrder: number[];
5856
layerOrder?: TimelineLayerId[];
@@ -108,7 +106,7 @@ export function resolveTimelineAutoScroll(
108106
export function resolveTimelineMove(
109107
input: TimelineMoveInput,
110108
clientX: number,
111-
clientY: number,
109+
currentRow: number,
112110
): {
113111
start: number;
114112
track: number;
@@ -117,11 +115,12 @@ export function resolveTimelineMove(
117115
stackingReorder?: TimelineStackingReorderIntent | null;
118116
} {
119117
const scrollDeltaX = (input.currentScrollLeft ?? 0) - (input.originScrollLeft ?? 0);
120-
const scrollDeltaY = (input.currentScrollTop ?? 0) - (input.originScrollTop ?? 0);
121118
const deltaTime =
122119
(clientX - input.originClientX + scrollDeltaX) / Math.max(input.pixelsPerSecond, 1);
123-
const trackDeltaRaw =
124-
(clientY - input.originClientY + scrollDeltaY) / Math.max(input.trackHeight, 1);
120+
// Rows, so vertical scroll and per-row heights are the caller's problem: rows
121+
// have varied in height since lanes expand, and a single trackHeight can't
122+
// describe them.
123+
const trackDeltaRaw = currentRow - input.originRow;
125124
const deltaTrack = Math.round(trackDeltaRaw);
126125
const nextStart = clamp(
127126
roundToCentiseconds(input.start + deltaTime),

0 commit comments

Comments
 (0)