Repository navigation
fix(studio): keep narrow timeline headers readable - #4232
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
Merged at 23:49:29Z, about 14 seconds after the review request, so this is a comment rather than an approval — reviewDecision is REVIEW_REQUIRED with zero reviews, so an APPROVE now would be the only review on the PR and would read as the stamp that opened a gate nothing was holding. Verdict on the code: this is the right change, and for a better reason than the PR body gives.
Content check: all 4 paths byte-identical between the head named in the request (a97164f9) and the squash (b4787082). Positive control at the PR's first commit 31620fd7, where all 4 differ — so "identical" here is a measurement, not a blind comparison.
The change closes a contradiction that was already in the tree
The PR reads as a taste call — wrapping vs. truncating — and it deletes three comments that argue against truncating:
Wraps rather than truncating: a truncated name needs a hover to be read at all, and a tooltip is no use to a name you are scanning a column of.
Adopting title={name} is exactly the hover those comments reject, so on the face of it this reverses a documented decision and removes the record of it. But at the merge base, TimelineTrackPlainHeader.tsx already contradicted itself — nine lines apart:
:87-88— "the name already truncates on its own (min-w-0plustruncate)":98-101— "Wraps rather than truncating …wrap-break-word", with:103actually renderingwrap-break-word
So :87-88 was false at base, and it wasn't decorative: it's the premise for collapsing the old two-line split into one line ("the controls are shrink-0, so they hold the edge and the name gives way instead"). The name never gave way, which is precisely why a long name shoved the header around. This PR makes the code match the claim the file was already making. That's a stronger argument than "it truncates now", and it's worth stating, because as written the diff invites someone to revert it on the authority of the comments it deleted.
Three non-blocking notes
1. Identical change in three components, one test. LayerDisclosureRow.tsx, TimelineGroupHeader.tsx and TimelineTrackPlainHeader.tsx all move from wrap-break-word to truncate + title, and only the third is pinned. TimelineGroupHeader.test.tsx already exists, so covering it was a few lines. LayerDisclosureRow has no test file at all — it's the one with flex-1 on the name, so its layout behaviour is the least like the other two and the most worth a test. Two-thirds of the regression this PR fixes can silently come back.
2. Half the new test pins the diff rather than the behaviour. expect(label?.getAttribute("title")).toBe(name) is the valuable assertion. expect(label?.className).not.toContain("wrap-break-word") has no failure mode that corresponds to a user-visible bug — it only fails if someone names the old utility again, and it would need editing if truncation were ever reached another way. Asserting the class rather than measured overflow is a fair compromise given happy-dom does no layout, so toContain("truncate") is fine; it's the negative assertion that's pure diff-pinning.
3. The rationale is gone from two of the three files. TimelineTrackPlainHeader keeps an explanation by accident — the pre-existing :85-89 comment now happens to be true. LayerDisclosureRow and TimelineGroupHeader are left with a bare title={…} truncate and no note of the tradeoff, in files where every other layout decision carries its reasoning. The tradeoff is real and worth one line: the full name stays in the DOM, so screen readers are unaffected by the visual truncation, but title doesn't open on keyboard focus and doesn't exist on touch — so those users get the ellipsis and nothing else. That's the cost the deleted comments were pointing at, and it's still true; it's just now outweighed by the header no longer being shoved around.
— Rames
Summary
The timeline header names now stay on one line with ellipsis truncation and a full-name title. This reverses the earlier deliberate
wrap-break-wordchoice because Miguel's screenshot showed long unbroken names stacking one letter per line. The same rule applies to plain track names, group names, and disclosure-row names.Before
After
Verification
TimelineTrackPlainHeader.test.tsxandTimelineTrackHeader.test.tsx, 41 passed.format:check: passed.lint: passed.UX checklist