chore(hog-charts): group BarChart bar options under bars config#60820
Merged
Conversation
Flatten `barCornerRadius`, `barTrack`, `barShadow`, `divergingStack`, `maxBandRange`, `bandPadding`, and `minBandSize` into a single `bars: BarsConfig` object on `BarChartConfig`. `barLayout` stays top-level as the primary discriminator. Behavior-preserving refactor: internal `BarChart.tsx` keeps its existing variable names via destructuring aliases, so the render path is unchanged. All consumers (`TimeSeriesBarChart`, the funnel charts, and stories) move to the grouped shape. `TimeSeriesBarChart`'s public component props are untouched. Split off ahead of the horizontal-funnel rework so the config regrouping reviews on its own, separate from the new pill/track/cursor behavior. Generated-By: PostHog Code Task-Id: c1e8f45e-0005-4c7d-8cd4-86ffc88ba81a
Contributor
|
Reviews (1): Last reviewed commit: "chore(hog-charts): group BarChart bar op..." | Re-trigger Greptile |
Contributor
|
Size Change: 0 B Total Size: 80.8 MB ℹ️ View Unchanged
|
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
sampennington
added a commit
that referenced
this pull request
May 31, 2026
…ooltip Self-contained BarChart building blocks, split off ahead of the horizontal-funnel rework so they review on their own: - `cornersFor` gains a `shouldRoundBaseline` arg and `computeSeriesBars`/`computeBarAtIndex` gain per-index `capRounded`/`baseRounded` overrides, so a stack can round both ends (funnel-style pill) resolved per band. Fully covered by new bar-layout unit tests. - `clipToRoundedRects` and `drawSolidBarTracks` canvas-renderer helpers for masking a stack to a rounded band and painting a single solid "remainder" track per band. - `tooltip.placement: 'cursor'` so the tooltip follows the mouse (chart.js-style). These are reusable primitives + an additive tooltip option; the orchestration that wires them up lives in the follow-up funnel PR. Stacked on #60820. Generated-By: PostHog Code Task-Id: c1e8f45e-0005-4c7d-8cd4-86ffc88ba81a
2 tasks
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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
1 participant
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.
Problem
The horizontal-funnel rework (#60762) grew large because it bundles a behaviour-preserving config refactor together with genuinely new rendering behaviour. This PR carves off the mechanical part so it can review on its own and land first, shrinking the follow-up to just the new funnel behaviour.
Changes
Group the flat
BarChartConfigbar options under a singlebars: BarsConfigobject:BarChartConfig.*)BarChartConfig.bars.*)barCornerRadiuscornerRadiusbarTracktrackbarShadowshadowdivergingStackdivergingStackmaxBandRangemaxBandRangebandPaddingbandPaddingminBandSizeminBandSizebarLayoutstays top-level as the primary discriminator.No behaviour change.
BarChart.tsxkeeps its existing internal variable names via destructuring aliases, so the render path is byte-for-byte equivalent. All consumers move to the grouped shape:TimeSeriesBarChart,FunnelHistogramChart,FunnelStepsBarChart,FunnelBarHorizontalChart, and theBarChartstories.TimeSeriesBarChart's public component props (barCornerRadius,divergingStack) are intentionally unchanged, soTrendsLifecycleChartneeds no edit.This is a pure regrouping —
rounding, track colour, and cursor-tooltip placement are new capabilities that stay in the follow-up funnel PR.How did you test this code?
I'm an agent (Claude Code, Opus 4.8). This sandbox has no
node_modules, so I could not run jest/tsc locally — CI will typecheck and run the existing hog-charts and funnel suites. The change is a mechanical rename verified by an exhaustive repo-wide grep for every old flat key to confirm no consumer was missed and the only remaining references are the internal destructuring aliases.Automatic notifications
🤖 Agent context
Authored with Claude Code (Opus 4.8) at the request of the PR author, who wanted to split a reviewable chunk off #60762 and land it first. I chose the
barsconfig grouping as the split target because it is behaviour-preserving (fast, low-risk review) and is the foundation the rest of the work sits on, so merging it strips the largest block of mechanical noise out of the feature PR. The smaller cursor-tooltip addition was considered as an alternative but shrinks the feature PR less.Agent-assisted; requires human review.