chore(quill): revert unused HighlightedRange overlay and per-bar hatch - #70252
chore(quill): revert unused HighlightedRange overlay and per-bar hatch#70252sampennington wants to merge 6 commits into
Conversation
sampennington
added
skip-agent-review
PostHog
🤖 CI report
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.21 MiB · 22 files | no change | ███░░░░░░░ 28.2% of 4.29 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.10 MiB · 2,974 files | no change | █████████░ 87.5% of 9.25 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| 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.3 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1283.93 MiB · 🔺 +1.3 KiB (+0.0%)
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
Run oxfmt on docs/internal/quill-migration-sparkline.md — aligns the population-B table columns to satisfy the markdown format check. Generated-By: PostHog Code Task-Id: 2b83bd23-a83a-4c91-982e-b1e0e9c356ee
9712cd1 to
7e86be6
Compare
Generated-By: PostHog Code Task-Id: 7f697c76-e9e2-4af6-93ff-c9fc9f90e932
|
Docs from this PR will be published at posthog.com
Preview will be ready in ~10 minutes. Click Preview link above to access docs at |
🦔 Hogbox preview · ❌ build failedThe preview didn't come up for commit |
|
Reviews (1): Last reviewed commit: "chore(quill): fix MD018 heading lint in ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const hatch = dashedFrom !== null || dashedTo !== null ? getHatchPattern(ctx, series.color) : null | ||
|
|
||
| for (const bar of bars) { | ||
| if (bar.width <= 0 || bar.height <= 0) { | ||
| continue | ||
| } | ||
| const useHatch = | ||
| (dashedFrom !== null && bar.dataIndex >= dashedFrom) || | ||
| (dashedTo !== null && bar.dataIndex <= dashedTo) || | ||
| !!series.bars?.[bar.dataIndex]?.hatch | ||
| // The hatch keeps the bar's own resolved color (per-bar override included) so a | ||
| // flagged bar still reads as belonging to its series. Pattern lookups are cached. | ||
| const barColor = barColorAt(series, bar.dataIndex) | ||
| ctx.fillStyle = useHatch ? getHatchPattern(ctx, barColor) : makeBarFill(ctx, barColor, bar, fillStyle) | ||
| hatch !== null && | ||
| ((dashedFrom !== null && bar.dataIndex >= dashedFrom) || (dashedTo !== null && bar.dataIndex <= dashedTo)) | ||
| ctx.fillStyle = useHatch ? hatch : makeBarFill(ctx, barColorAt(series, bar.dataIndex), bar, fillStyle) |
There was a problem hiding this comment.
When a series uses stroke.partial together with bars[i].color, the hatched bars now all use series.color instead of the resolved per-bar color. That makes partial bars with color overrides render in the wrong color while non-hatched bars in the same series still use their overrides.
Generated-By: PostHog Code Task-Id: 7f697c76-e9e2-4af6-93ff-c9fc9f90e932
|
✅ Visual changes approved by @sampennington — baseline updated in 8 removed. |
0 updated, 8 removed Run: 6ea29762-fb9f-4b3d-8980-576b6313ab41 Co-authored-by: sampennington <56024559+sampennington@users.noreply.github.com>
|
Note 🤖 stamphog reviewed The diff isn't a clean revert: it also rewrites the pre-existing stroke.partial hatch color logic to always use the series-level color instead of the per-bar resolved color, a real rendering regression for bars combining stroke.partial with per-bar color overrides — flagged unresolved by Greptile and contradicting the PR's own claim that pre-existing hatch machinery is untouched.
Gate mechanics and policy version
Updated in place — this replaces 1 earlier stamphog review(s) on this PR. |
sampennington
added
the
stamphog
PostHog
1 participant
Problem
The
HighlightedRangeoverlay and the per-barbars[i].hatchfill were added to quill-charts in #70223 as step 1 of the shared Sparkline → quill migration, to close capability gaps for the Logs/Tracing volume-chart consumers.The plan for those consumers has since changed — they will move to quill's
TimeSeriesBarChartin a later phase, not the Sparkline preset. Nothing infrontend/orproducts/renders either capability today, so it is merged-but-unused public API plus its snapshot baselines.Changes
Revert #70223:
HighlightedRangeoverlay — component, tests, stories, and the package export.hatch?: booleanfield onSeries.bars[i]and its canvas render path.AGENTS.mddocs and storybook snapshot baselines.The pre-existing hatch machinery (
stroke.partialdashes,bars.tracktracks) is untouched. When that migration reaches Logs and Tracing, the overlay gets rebuilt against the realTimeSeriesBarChartrequirements.Note
The in-flight sparkline PR #70231 does not depend on this — it is based on master and the true-sparkline path uses none of these capabilities. #70236 (Sparkline bar/series/tooltip) is kept because #70231 consumes it.
How did you test this code?
I (Claude) verified by grep that nothing in
frontend/orproducts/imports the quillHighlightedRangeoverlay or setsbars[i].hatch; the only apphighlightedRangeusages flow through the legacy Chart.jsSparkline. The revert applied with no conflicts and leaves no dangling references. CI regenerates storybook snapshots for the removed stories. No manual UI testing, since nothing rendered these.Automatic notifications
Docs update
Charts
AGENTS.mdis updated in this PR to drop the removedhatchandHighlightedRangereferences. No user-facing docs are affected.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Sam directed this. While working through the shared Sparkline → quill migration with PostHog Code (Claude), we concluded the Logs/Tracing "sparklines" are really full time-series bar charts and belong on
TimeSeriesBarChart, not the Sparkline preset. That pushed the relevant migration phase out and left #70223'sHighlightedRangeoverlay and per-bar hatch as merged-but-unused capability, so we revert it now and rebuild later against clearer requirements.Before reverting, I (Claude) confirmed there are no consumers of either capability and kept #70236 because #70231 consumes it. An internal migration handover doc was drafted and then dropped from this PR to keep it a focused revert. No repo skills were invoked.
Created with PostHog Code