refactor(eve): derive trace viewer geometry once - #1468
Merged
Conversation
The conversation viewer's line layout — each card occupies its rendered lines plus one separator row — was hand-rolled in four places: the scroll offset of the selected card, the row-to-card lookup for clicks, and two byte-identical total-line loops (#1446 added one that #1403's wheel handler already had). They had also drifted on the edge case where the measured line counts lag the card list: two loops walked the counts array and two walked the cards with a one-line default. All four now come from `cardLineOffsets`, which walks the cards and keeps the default, so a frame measured before a repaint maps rows to cards the same way everywhere. The nine-key "reset the view" literal was spelled out in `applyTraceList`, `switchTrace`, and `createTraceViewerState`, and had drifted too: `textSelection` was added to two of them by #1446. One `clearedTraceView()` factory now serves all three. The pruned-trace branch keeps its own key list, since it deliberately preserves expansion state and selection. In the renderer, `renderEmptyState` and `center` both built a body-height array and centred content in it, so `center` was `renderEmptyState` with one line; both already returned exactly `bodyRows` entries, which made `padBody` the identity function at both of its call sites. `center` now takes the lines to centre and `padBody` is gone. `joinPanels` loses its `separator` parameter, which had one caller passing a constant the function's own width math already assumed. Also types the session's `#metrics` field as the `TraceViewerKeyEnvironment` it restates field-for-field and passes straight to the reducer. Signed-off-by: Chad Hietala <chadhietala@gmail.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
chadhietala
marked this pull request as ready for review
July 31, 2026 15:46
AndrewBarba
approved these changes
Jul 31, 2026
ctgowrie
approved these changes
Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found
trace-viewer-state.ts, two of them byte-identical, and they had split two-and-two on the stale-measurement default — so a click could map to a different card than the scroll math assumed.textSelectionto two of them.trace-view.ts,padBodyis the identity function at both call sites,centerduplicatesrenderEmptyState, andjoinPanels'separatorparameter cannot vary.Did
One
cardLineOffsetsprefix-sum feeds all four sites, oneclearedTraceView()factory serves the three resets,padBodyis gone and the parameter dropped. −81/+78.Validated
1010 CLI unit tests and
tui-traces(all 9 smoke scenarios: expand/collapse, click, drag-copy, trace switching, empty state) pass. No test file touched.tsc --noEmit, lint, fmt,guard:invariantsclean.