feat(quill): add HighlightedRange overlay and per-bar hatch fill to charts - #70223
Conversation
…harts Closes the quill-charts capability gaps needed to migrate the legacy Chart.js Sparkline: - New HighlightedRange overlay — a translucent box spanning an x-axis index/label range, for mirroring an external selection (e.g. a virtualized list's visible rows) onto a chart. - Per-bar hatch fill via Series.bars[i].hatch — flags individual not-final bars (e.g. buckets still being ingested) at arbitrary indices, complementing the contiguous stroke.partial range. - Documents that onDateRangeZoom is label-generic (categorical labels work, not just dates), backed by the existing categorical drag-zoom test. Generated-By: PostHog Code Task-Id: 62ad73cf-6a7b-49c5-9367-d275bcfae1b7
🤖 CI report✅ Bundle size — 🟢 -10 B (-0.0%)Uncompressed size of every built Total: 68.08 MiB · 🟢 -10 B (-0.0%) No file changed by more than 1000 B. Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report ✅ Eager graph — within budgetHow much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy
🟢 Largest files eagerly shipped from
|
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 668 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 278.2 KiB | ../node_modules/.pnpm/posthog-js@1.399.2/node_modules/posthog-js/dist/rrweb.js |
| 266.9 KiB | ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 234.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 221.5 KiB | ../node_modules/.pnpm/posthog-js@1.399.2/node_modules/posthog-js/dist/module.js |
| 164.0 KiB | src/queries/validators.js |
| 154.2 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.9 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
⚠️ Dist folder size — 🔺 +1.8 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1283.90 MiB · 🔺 +1.8 KiB (+0.0%)
|
Reviews (1): Last reviewed commit: "feat(quill): add HighlightedRange overla..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // unset on point-style (line) charts, where the box runs from point to point. | ||
| const startHalfBand = (scales.extent?.(startLabel) ?? 0) / 2 |
There was a problem hiding this comment.
When HighlightedRange is used on a combo chart that includes bar series, the combo scales do not provide extent, so this falls back to point-to-point geometry. The highlighted range is drawn from bar center to bar center instead of covering the selected endpoint buckets, leaving the overlay visibly narrower than the dragged or selected bar range.
There was a problem hiding this comment.
🤖 sp-triage
Verified — this is a real gap, but it lives in ComboChart's createScales (packages/quill/packages/charts/src/charts/ComboChart/ComboChart.tsx), which never surfaces an extent closure the way BarChart does, not in HighlightedRange itself (it correctly falls back to point-to-point when extent is absent, same as any other line-chart caller). Fixing it means deciding whether/how to expose per-bar band width from combo scales when bar series are present, which is a design change to a different component than what this PR adds. Deferring to the author/a follow-up rather than resolving as a non-issue, since it is a legitimate rough edge.
sampennington
left a comment
sampennington
left a comment
There was a problem hiding this comment.
🤖 sp-ship · 🚫 Blockers
One blocker: the new drawBars hatch test carries an if (original) guard in its body — branching logic in tests is disallowed; restore the descriptor unconditionally or use try/finally like the sibling test. The rest is minor polish: the reviewers converged on tightening the border-toggle assertion, and single suggestions cover theming the default highlight color, pinning the per-bar-override hatch color in a test, and two cosmetic cleanups. Overall the change is well-scoped — the hatch correctly resolves by bar.dataIndex (the known filtered-length bug shape), the overlay is O(1), layout-context-only, and properly exported.
- Remove branching restore in the new drawBars hatch test (try/finally like the sibling test) - Assert the border div's style directly in the HighlightedRange border-toggle test - Hoist barColorAt into a single const in drawBars Generated-By: PostHog Code Task-Id: 62ad73cf-6a7b-49c5-9367-d275bcfae1b7
sampennington
added
the
stamphog
PostHog
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additive frontend charting-library change (new overlay component + per-bar hatch option) outside risky territory, with solid new test coverage; the sole unresolved thread is an acknowledged, non-blocking edge-case limitation on combo charts deferred as follow-up, and the prior test-quality blocker was verified fixed in the current diff.
- Author wrote 91% of the modified lines and has 69 merged PRs in these paths (familiarity STRONG).
- sampennington reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 192L, 6F substantive, 345L/9F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (345L, 9F, single-area, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ dda7ff4 · reviewed head e1d832b |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Generated-By: PostHog Code Task-Id: 62ad73cf-6a7b-49c5-9367-d275bcfae1b7
|
Retaining stamphog approval — delta since last review classified as |
|
Retaining stamphog approval — delta since last review classified as |
|
✅ Visual changes approved by @sampennington — baseline updated in 8 new. New |
8 updated Run: a25167e9-548e-4cad-b518-120b8b085d25 Co-authored-by: sampennington <56024559+sampennington@users.noreply.github.com>
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Additive quill-charts overlay/hatch-fill feature with solid test coverage, no risky-territory surface (no auth/billing/API/migration/CI/deps); the sole unresolved thread (combo-chart band width edge case) was triaged as a legitimate pre-existing/out-of-scope follow-up and acknowledged by the agent reviewer, and the author has STRONG familiarity with this code.
- Author wrote 91% of the modified lines and has 429 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 208L, 7F substantive, 360L/10F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (360L, 10F, two-areas, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ edc31df · reviewed head b5d0e15 |
Revert #70223. The HighlightedRange overlay and the per-bar `bars[i].hatch` fill were added ahead of migrating the Logs/Tracing sparkline consumers, but that migration is being reframed onto TimeSeriesBarChart and has not started, so nothing renders either capability today. Removing the unused code now; it will be rebuilt against real requirements during the population-B migration. #70231 does not depend on this, and the pre-existing hatch machinery (stroke.partial dashes, bars.track) is untouched. Generated-By: PostHog Code Task-Id: 2b83bd23-a83a-4c91-982e-b1e0e9c356ee
Capture the two-population reframe (true sparklines vs charts-in-disguise), the consumer split, the needsLegacyFeatures framing, the maximumIndicator/tooltipRowCutoff/hoverColor decisions, and the step-4 direction (population B → TimeSeriesBarChart). Records why #70223's HighlightedRange overlay and per-bar hatch were reverted here and are deferred to the population-B migration. Generated-By: PostHog Code Task-Id: 2b83bd23-a83a-4c91-982e-b1e0e9c356ee
Revert #70223. The HighlightedRange overlay and the per-bar `bars[i].hatch` fill were added ahead of migrating the Logs/Tracing sparkline consumers, but that migration is being reframed onto TimeSeriesBarChart and has not started, so nothing renders either capability today. Removing the unused code now; it will be rebuilt against real requirements during the population-B migration. #70231 does not depend on this, and the pre-existing hatch machinery (stroke.partial dashes, bars.track) is untouched. Generated-By: PostHog Code Task-Id: 2b83bd23-a83a-4c91-982e-b1e0e9c356ee
Capture the two-population reframe (true sparklines vs charts-in-disguise), the consumer split, the needsLegacyFeatures framing, the maximumIndicator/tooltipRowCutoff/hoverColor decisions, and the step-4 direction (population B → TimeSeriesBarChart). Records why #70223's HighlightedRange overlay and per-bar hatch were reverted here and are deferred to the population-B migration. Generated-By: PostHog Code Task-Id: 2b83bd23-a83a-4c91-982e-b1e0e9c356ee
1 participant
Problem
The chart.js removal effort needs the shared
lib/components/Sparkline.tsx(Chart.js + chartjs-plugin-annotation) replaced with@posthog/quill-charts. The migration plan (docs/internal/quill-migration-sparkline.md) identified quill capability gaps that must close before any consumer can move: a highlighted-range overlay (needed by the logs viewer, tracing sparkline, and operation histogram), a per-bar hatch fill (theincompleteBarsequivalent), and confirmation that drag-to-zoom works on categorical labels.Why: step 1 of the Sparkline→quill migration plan — land the missing quill chart capabilities so the consumer waves can follow.
Changes
HighlightedRangeoverlay: a translucent box spanning an x-axis index/label range, composed as a chart child likeReferenceLine. Covers full bands on bar charts, point-to-point on line charts, clamps to the plot area, and renders null for unresolvable endpoints. Exported from the package index, with stories (including one mirroring a drag selection viaonDateRangeZoom).Series.bars[i].hatchfills that bar with the existing diagonal-hatch pattern in the bar's resolved color — flags individual not-final bars (e.g. buckets still being ingested) at arbitrary, possibly non-contiguous indices, complementing the contiguousstroke.partialrange. Added aHatchedBarsstory.onDateRangeZoomverified as label-generic (it resolves drags against label positions, so categorical labels like duration buckets work) and documented as such in AGENTS.md — the existing categorical drag-zoom test already covers it.How did you test this code?
HighlightedRange.test.tsx(8 tests): band-edge coverage vs point-to-point geometry, index/label endpoint resolution, reversed-endpoint normalization, plot clamping, null render for unresolvable endpoints, border omission atborderOpacity: 0— all new geometry with no prior coverage.drawBarstest asserting bars flagged viabars[i].hatchget the pattern fill at non-contiguous indices while unflagged bars stay solid — catches a regression where the new flag is ignored.BarCharttest already drags across categorical labels, so a duplicate would add no coverage.Automatic notifications
Docs update
Updated
packages/quill/packages/charts/AGENTS.md(and itsCLAUDE.mdsymlink) in the same PR, per the package's maintenance rule.🤖 Agent context
Autonomy: Fully autonomous
getHatchPatterntreatment (already used forstroke.partialdashed bars) for per-bar hatching rather than porting the legacy faded-fill pattern, keeping one consistent "not final" visual across the package; the hatch now resolves per-bar colors viabarColorAtso per-bar-colored series hatch in their own color.HighlightedRangeaccepts either data indices or labels so it can be fed straight fromonDateRangeZoomoutput or from label-based external state.Created with PostHog Code