feat(log-viewer): rebuild the timeline hover card, and one swatch for every colour key - #961
Open
lukecotter wants to merge 19 commits into
Open
feat(log-viewer): rebuild the timeline hover card, and one swatch for every colour key#961lukecotter wants to merge 19 commits into
lukecotter wants to merge 19 commits into
Conversation
Four places drew a small colour key, in three shapes. --lana-radius-sm is var(--vscode-cornerRadius-small, 4px), so at 8px the frame tooltip's swatch rounded to a circle, the timeline legend asked for one outright, and only the stacked bar and the metric strip drew the squircle. Replace all four with <color-swatch>, a custom element so the canvas tooltips — which build their panels imperatively and cannot adopt a Lit stylesheet — draw the same shape as the Lit components. Size and radius come from new --lana-swatch-* tokens, which also takes the last literal 8px/50% out of TimelineKey.
…d-tooltip # Conflicts: # log-viewer/src/components/StackedTimeBar.ts
The legend carried its own category-to-theme-key map, duplicating the one Themes.ts calls the single source of truth, and its own getTheme() call — so it resolved colours by a path nothing else in the app used. toTimelineKeys now takes the colour function, and TimelineView passes categoryPalette(), the same resolver the inspector's rows read. That collapses the legacy and modern legend branches into one: categoryPalette already maps a category to its legacy group, so the legend under the legacy chart now names the same categories as the modern one, with the colours that chart drew and the per-category self times it never used to show.
The metric list, the used/limit formatting and the byte format lived inside EventVitals, so the timeline tooltip carried a second hand-written copy that had already drifted: lower-cased labels, no percentage, and the wrong SOSL row denominator. Move them to core/metrics/eventMetrics.ts. usageParts returns the reading as parts plus the fraction of the limit, so a caller can render it as text or as a meter; EventVitals' usage() is now a wrapper over it and renders as before.
The legend read the modern categories through the legacy colour map, and Apex and Callout both map to Method — so it drew two chips in one colour, and named neither of them what the legacy chart's own key says. It now folds the categories that share a group and sums their self time, naming the six groups the chart draws. The fold is stated rather than inverted from LEGACY_CATEGORY_MAP: the draw order and the non-empty membership are facts a derivation loses, and inverting it would pull the canvas service — and the window it reads as it loads — into a pure util. A chip carries the categories it stands for, so data-category names something a reader can resolve. Under legacy the label is a group name and matches no category. A group missing from lana.timeline.colors also left a swatch unpainted, where setColors leaves the chart on its built-in colour. The legend reads that default too.
The card measured every reading against a governor limit. The hover is for deciding where to look, so it now measures a branch against the log's own total — 3 of 30, spelled "of" so it cannot read as a cap. Risk against limits stays with the governor strip and the Inspector. frameTooltipCard.ts holds what the card says and in what order, with no DOM, so it reads in a test without laying a panel out. The renderer keeps the DOM, the timers, the placement and the query budget. Rows are three columns: label, reading, and the frame's own figure. A self reading of zero stays — it is what says the work happened in a descendant. The duration and the byte readings state no denominator, because the unit follows the value and the pair would not compare. Six metric rows at most, the rest counted in the footer and ranked by the branch's share, so the choice holds as the pointer moves. A heap peak is unranked: it composes by max, so every frame spanning the transaction's peak reports the root's figure and would outrank every summed metric. A frame's name is its description block; category, type, namespace and call site read on one identity line. The row that the hover was for carries the card's emphasis, so a frame with no duration no longer puts it on the wall clock. The marker rail derives from MARKER_COLORS instead of restating it, the self reading is spelled once in eventMetrics, and usageParts drops the fraction nothing consumed once the card stopped metering.
lineNumber is where the frame was called from in the containing code, not where it is defined. "Line" reads as the definition, so the inspector now says "Called from". A line the parser could not resolve still reads as EXTERNAL on its own, matching the timeline's identity line.
The metric strip reserved a 12px track for a swatch that is now 8px, and stated a size the token already owns. Both rows read --lana-swatch-size, and the row style is written once rather than twice. The swatch painted itself two ways — a --row-hue fallback in :host and a style written in updated(). No caller leaves the colour out, and --row-hue is only ever set by a reveal row, which draws no swatch, so the colour property is now the one mechanism. Its label became a title on an aria-hidden host, naming what the text beside it already said; the accessible name belongs in light DOM, as the reveal rows do it. eventMetrics owned the only import from features/ anywhere in core/, for a constant that is a governor fact rather than a Database-tab one. SOSL_ROWS_PER_QUERY_LIMIT moves down; the Database tab re-exports it so its own consumers are unchanged. StatementType was declared twice inside core/, so the metric list now owns it and the event bus re-exports. hasSelf was read only as `=== false`, where absent and true meant the same thing. It reads as noSelf. EventVitals' timing row spells the self reading through selfLabel, like every other view.
…ardown Toggling the timeline between modern and legacy threw `Cannot read properties of undefined (reading 'push')` from `TexturePoolClass.returnTexture`. `app.destroy(true, ...)` releases Pixi's global resources, and TexturePool is one of them. A timeline runs three apps, so the first destroy emptied the pool the other two still return their text textures to. One `destroyTimelineApp` now owns the call, so the rule holds for the next app as well as these three.
The chart takes its height from a flex row that a `lana.timeline.legacy` toggle re-lays-out around it. Measuring before that settled read 0, and `init` rejects that outright: "Container must have non-zero dimensions". Wait for a size first, so the user sees a chart rather than an error about a container that is fine.
…g changes The classifier allocates its points once per `processData` and hands back the same object for every pointer position inside one time segment. Identity therefore tells a re-position from a new reading. Sweeping a segment no longer re-parses the panel or upgrades a swatch element per row, on a mousemove that is not throttled.
Moving `StatementType` and `SOSL_ROWS_PER_QUERY_LIMIT` left three doc blocks documenting `import` statements.
…rive
`categoryPalette(timeline && {...})` collapses to `null` while no settings have
been pushed, so the previewed theme was dropped and the legend painted the
default palette while the chart drew the chosen one.
The palette now takes the previewed theme as its own argument, which also
removes the spread that was papering over it.
…vive `data-category` joined on a space, but `Code Unit` is one category that contains a space, so a consumer splitting the list read two categories.
… part The rule parts what the frame is from what it measured. It went on the first group whatever the card held, so on a marker card — which has no identity line — it parted nothing.
lcottercertinia
previously approved these changes
Aug 25, 2026
The renderer overrode `positionTooltip` to sit below the strip, and in doing so reimplemented the base's maths without its `requestAnimationFrame` batching. It read `offsetWidth` right after writing `display`, so every raw mousemove forced a layout flush. `below-anchor` puts the rule in the base class instead: cursor X with the same flip, a fixed offset below the anchor, and never flipping up over the band the panel must not cover.
…into The wait for a size sat in the Lit component, guarding one call while `FlameChart.init` still threw on zero for every other caller. It was also a second ResizeObserver on a container the chart already observes, and an unbounded one: a container that never gained a size hung instead of reporting. The chart now waits for its own container, bounded by a frame count, so the fault still surfaces. That covers every zero-size trigger — a hidden tab, a collapsed panel — not just the settings toggle, and drops the component's duplicate of the predicate `init` throws on.
The panel was rebuilt from an HTML string, so every segment the pointer crossed reparsed it and upgraded a swatch custom element per row. The rows are elements now, held and written into. `selectRows` chooses and orders the readings, and says nothing about the DOM.
Two files conflicted where this branch had already changed them. `StackedTimeBar` — our side replaced the `.legend__swatch` span with `<color-swatch>`, so the rule has no user; kept upstream's added `.tip__part-value` selector. `EventVitals` — combined our shared `EVENT_METRICS` and `noSelf` with upstream's split `sumTotal`/`sumSelf`. The `SOSL_ROWS_PER_QUERY_LIMIT` import stays dropped: the constant now lives in `core/metrics/eventMetrics.ts`.
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.
Two timeline features shipped this release — the legend below the tabs and the frame hover card — were built in isolation, so they diverged from conventions the rest of the app had already settled. This brings them in line and rebuilds the card.
The card
It measured every reading against a governor limit. The hover is for deciding where to look, and "is the work here" varies frame to frame where "is the transaction near a limit" does not — so a reading is now measured against the log's own tree total:
3 of 30. Spelled "of", not "/", so it cannot read as consumption of a cap. Risk against limits stays with the governor strip and the Inspector.frameTooltipCard.tsholds what the card says and in what order, with no DOM, so the content reads in a test without laying a panel out. The renderer keeps the DOM, timers, placement and the query budget.58.2 ms of 24.6 sor100 bytes of 6 MB.lineNumberis the call site in the containing code, not the definition line. The card saysfrom line 42; the inspector's row is relabelledCalled from.The legend
toTimelineKeysnow takes a colour function andTimelineViewpassescategoryPalette()— the resolver the inspector rows already use — instead of re-declaring the category-to-theme-key map thatThemes.tscalls the single source of truth.Under
lana.timeline.legacythat exposed a defect:LEGACY_CATEGORY_MAPsends bothApexandCallouttoMethod, so the legend drew two chips in one colour and named neither of them what the legacy chart's own key says. It now folds the categories that share a group and sums their self time, naming the six groups the chart draws. A group missing fromlana.timeline.colorsalso left a swatch unpainted, wheresetColorsleaves the chart on its built-in colour; the legend reads that default too.One swatch
Five places drew a small colour key in three shapes.
--lana-radius-smisvar(--vscode-cornerRadius-small, 4px), and 4px on an 8px box is a full circle — so two of the "squircles" were circles. One<color-swatch>now, at every site.Commits
refactor: one swatch for every colour key<color-swatch>+--lana-swatch-*; four call sitesupstream/mainrefactor: read the timeline legend from the shared palettecategoryPalette()replaces the legend's own resolverrefactor: share the frame metric listcore/metrics/eventMetrics.tsextracted fromEventVitalsfix: key the legacy timeline by the groups it drawsfeat: rebuild the timeline hover cardrefactor: name the line number as the call siteLine→Called fromrefactor: settle the swatch and the shared metric vocabularyThe last commit fixes what review found in the earlier ones: the metric strip reserved a 12px track for a swatch that is now 8px; the swatch painted itself two ways, and its
labelbecame atitleon anaria-hiddenhost naming what the text beside it already said;eventMetricsheld the only import fromfeatures/anywhere incore/, for a constant that is a governor fact rather than a Database-tab one; andStatementTypewas declared twice insidecore/.Fixes after review
fix: keep Pixi's global texture pool across a timeline teardownTexturePoolClass.returnTexturefix: wait for the chart's container before drawing into itperf: rebuild the metric-strip hover only when the reading changesmousemoverebuild listed below as out of scopedocs: reattach the docs the metric move orphanedimportstatementsfix: follow a previewed timeline theme before settings arrivefix: join the legend's categories on a delimiter they survivedata-categoryjoined on a space, andCode Unitcontains onefix: rule the hover card only where it has an identity to partperf: batch the metric-strip hover's placement into a framepositionTooltipforced a layout per raw mousemoverefactor: let the chart own the container it cannot draw intoFlameChart, not in one Lit componentperf: write the metric-strip hover into rows it already hasapp.destroy(true, ...)releases Pixi's global resources, andTexturePoolis one of them. Atimeline runs three apps, so the first destroy emptied the pool the other two still return their
text textures to. A
destroyTimelineApphelper now owns the call, so the rule holds for a fourthapp as well as these three — the only three
PIXI.Applicationsites in the repo.The theme one is a regression against the code it replaced:
categoryPalette(timeline && {...})collapses to
nullbefore the first settings push, droppingactiveTheme, so a quick-pick previewthat landed first went unseen by the legend. The palette now takes the previewed theme as its own
argument. Each of the three ships a test that fails on the code it replaces.
One finding left alone: replacing
lana.timeline.colorsleaves a stale hue, becausesetColorsmutates
keyMapin place and never resets a group the new setting omits. That is pre-existinglegacy-timeline behaviour, chart and legend agree either way, and the legacy timeline goes in 1.22.0.
The last three close the review's structural findings rather than working around them.
BaseTooltipRenderergained abelow-anchormode, so the metric strip no longer reimplementsplacement without the base's frame batching.
FlameChartnow waits for its own container —bounded, so a container that never gains a size still reports the fault — which covers a hidden
tab and a collapsed panel too, not just the settings toggle, and removes both the second
ResizeObserver and the component's copy of the predicate
initthrows on. And the hover's rowsare elements now, written into rather than reparsed.
Verification
tsc -b,eslint,prettier --checkclean; 140 suites / 1899 tests pass. Checked in the dev host onsample-app/debug-logs/, in a light and a dark theme, against bothlana.timeline.legacysettings and after a theme switch.Deliberately not in scope
features/timeline/services/Timeline.ts) has the same SOSL-rows defect the card used to have. Left alone: the legacy timeline goes in 1.22.0.eventText.ts(the Copy Details text) still owns a fourth hand-written metric list, so it disagrees with the card and the inspector on order and omits Throws and heap entirely. Worth a follow-up now thatEVENT_METRICSexists.EventMetriccould carry more of its own rules —combine: 'sum' | 'max'would remove "a peak composes by max" being spelled twice in two idioms in two consumers;unitwould let the "state a denominator only when the units match" rule apply to Time and bytes from one branch instead of two;statementTypewould replaceEventVitals'metric.label !== \${type.toUpperCase()} Rows`` string reconstruction.usageParts/formatByteshave one caller and structurally cannot gain a second (the card formats bytes compactly and spells the denominator differently), so they may belong back inEventVitals.descriptionCacheis aWeakMapkeyed onLogEvent, so nothing is collected while a log is loaded and each hovered query retains a detached subtree.Validationis excluded from the modern legend behind a parser NOTE but folds intoSystem Methodon the legacy side, so the two disagree once the parser starts emitting it.keyMap's immutable defaults could split from the mapsetColorsmutates, which would let the legend and the chart read one resolved colour.