Skip to content

Commit 191786b

Browse files
committed
fix(studio): stop lane-header clicks from reaching the track row
The prev/next keyframe chevrons and the group toggle diamond let their click bubble to the ancestor track row, so seeking to a keyframe also reselected the track. The disclosure caret and the eye already stop it; these now match. Truncated labels (layer name, track label, group label, value readout) also carry a title so the full text is reachable on hover.
1 parent 5aa92ae commit 191786b

3 files changed

Lines changed: 58 additions & 7 deletions

File tree

‎packages/studio/src/player/components/LayerDisclosureRow.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ export function LayerDisclosureRow({
5050
>
5151
◇
5252
</span>
53-
<span className="min-w-0 flex-1 truncate font-medium">{name}</span>
53+
<span className="min-w-0 flex-1 truncate font-medium" title={name}>
54+
{name}
55+
</span>
5456
</div>
5557
);
5658
}

‎packages/studio/src/player/components/TimelineTrackHeader.test.tsx‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,37 @@ describe("TimelineTrackHeader", () => {
167167
act(() => view.root.unmount());
168168
});
169169

170+
// The lane header sits inside the track row, whose own click handler selects
171+
// the track. Every control in the label column has to own its click, or
172+
// seeking to a keyframe also reselects whatever is behind the header.
173+
it("keeps lane-header control clicks off the ancestor track row", () => {
174+
const onAncestorClick = vi.fn();
175+
const view = renderHeader({
176+
currentTime: 1,
177+
onSeek: vi.fn(),
178+
onTogglePropertyGroupKeyframe: vi.fn(),
179+
});
180+
// React 18 delegates from the root container, so an ancestor of it is where
181+
// a leaked click actually shows up.
182+
document.body.addEventListener("click", onAncestorClick);
183+
184+
// Every control in the lane's label column, found by row rather than by
185+
// label, so a wording change to one button can't silently drop it here.
186+
const controls = view.host.querySelectorAll<HTMLButtonElement>(
187+
'[data-property-group="position"] button',
188+
);
189+
expect(controls.length).toBeGreaterThanOrEqual(3);
190+
for (const button of controls) {
191+
act(() => {
192+
button.dispatchEvent(new MouseEvent("click", { bubbles: true }));
193+
});
194+
}
195+
196+
document.body.removeEventListener("click", onAncestorClick);
197+
expect(onAncestorClick).not.toHaveBeenCalled();
198+
act(() => view.root.unmount());
199+
});
200+
170201
it("fills the toggle diamond exactly at that group's keyframe", () => {
171202
const view = renderHeader({ currentTime: 0.5 });
172203
const positionToggle = view.host.querySelector<HTMLButtonElement>(

‎packages/studio/src/player/components/TimelineTrackHeader.tsx‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,11 @@ function LegacyTrackHeader({
197197
{isAudioTrack && (
198198
<Music size={12} weight="fill" aria-hidden="true" className="text-white/35" />
199199
)}
200-
{showTrackLabel && <span className="min-w-0 flex-1 truncate text-[11px]">{trackLabel}</span>}
200+
{showTrackLabel && (
201+
<span className="min-w-0 flex-1 truncate text-[11px]" title={trackLabel}>
202+
{trackLabel}
203+
</span>
204+
)}
201205
<VisibilityButton
202206
hidden={isTrackHidden}
203207
trackNumber={trackNumber}
@@ -343,7 +347,10 @@ function PropertyGroupNavigation({
343347
aria-label={`Previous ${label} keyframe`}
344348
disabled={!navigation.prevKeyframe}
345349
className="h-5 w-3 border-0 bg-transparent p-0 text-white/55 hover:text-white disabled:text-white/15"
346-
onClick={() => seekTo(navigation.prevKeyframe)}
350+
onClick={(event) => {
351+
event.stopPropagation();
352+
seekTo(navigation.prevKeyframe);
353+
}}
347354
>
348355
‹
349356
</button>
@@ -353,7 +360,10 @@ function PropertyGroupNavigation({
353360
aria-label={`Next ${label} keyframe`}
354361
disabled={!navigation.nextKeyframe}
355362
className="h-5 w-3 border-0 bg-transparent p-0 text-white/55 hover:text-white disabled:text-white/15"
356-
onClick={() => seekTo(navigation.nextKeyframe)}
363+
onClick={(event) => {
364+
event.stopPropagation();
365+
seekTo(navigation.nextKeyframe);
366+
}}
357367
>
358368
›
359369
</button>
@@ -427,7 +437,9 @@ function PropertyGroupHeaderRow({
427437
/>
428438
<span className="absolute left-1.5 top-1/2 h-px w-1.5 bg-white/15" />
429439
</span>
430-
<span className="w-[46px] shrink-0 truncate text-white">{label}</span>
440+
<span className="w-[46px] shrink-0 truncate text-white" title={label}>
441+
{label}
442+
</span>
431443
<PropertyGroupNavigation
432444
navigation={navigation}
433445
label={label}
@@ -439,7 +451,10 @@ function PropertyGroupHeaderRow({
439451
aria-label={`Toggle ${label} keyframe`}
440452
title={`${navigation.currentKeyframe ? "Remove" : "Add"} ${label} keyframe`}
441453
className="flex h-5 w-4 shrink-0 items-center justify-center border-0 bg-transparent p-0 text-[11px] text-[#3CE6AC] focus-visible:outline focus-visible:outline-1 focus-visible:outline-[#3CE6AC]"
442-
onClick={() => {
454+
onClick={(event) => {
455+
// Same as the disclosure caret and the eye: a control in the label
456+
// column owns its click, it does not also hit the track row behind it.
457+
event.stopPropagation();
443458
if (expandedElement && toggleTarget) {
444459
void onTogglePropertyGroupKeyframe?.(expandedElement, toggleTarget);
445460
}
@@ -448,7 +463,10 @@ function PropertyGroupHeaderRow({
448463
{navigation.currentKeyframe ? "◆" : "◇"}
449464
</button>
450465
</PropertyGroupNavigation>
451-
<span className="min-w-0 flex-1 truncate text-right tabular-nums text-white/45">
466+
<span
467+
className="min-w-0 flex-1 truncate text-right tabular-nums text-white/45"
468+
title={valueReadout(lane.group, values)}
469+
>
452470
{valueReadout(lane.group, values)}
453471
</span>
454472
<VisibilityButton

0 commit comments

Comments
 (0)