fix(insights): align hog-charts retention bar with legacy visual#60159
Merged
sampennington merged 2 commits intoMay 26, 2026
Conversation
- Drop the hatched in-progress bars in `RetentionBarChart` by not passing `incompletenessOffsetFromEnd` into `buildRetentionSeries`. The partial-period stripe stays on the line view, matching legacy chart.js behavior. - Always frame the plot area in `drawGrid`: vertical mode now draws both left and right value-axis lines, horizontal mode draws top and bottom. Retention bar (and every other hog-charts consumer) reads as a closed rectangle instead of an L-shape. - Update `canvas-renderer.test.ts` to assert both near and far edges are framed in each orientation. Generated-By: PostHog Code Task-Id: 2e54f6c4-6cbc-426d-9938-d9a1fec57f81
Contributor
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
Contributor
|
Reviews (1): Last reviewed commit: "fix(insights): align hog-charts retentio..." | Re-trigger Greptile |
Contributor
|
Size Change: 0 B Total Size: 80.1 MB ℹ️ View Unchanged
|
Contributor
|
✅ Visual changes approved by @sampennington — baseline updated in 21 changed. |
21 updated Run: 6e8af058-aef3-406d-8179-5ee337cc642a Co-authored-by: sampennington <56024559+sampennington@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No showstoppers. This is a low-risk frontend visual fix — adds a closing frame line to the canvas renderer and removes an optional in-progress dashing parameter from the retention bar chart to match legacy appearance. All snapshot updates are consistent with the rendering change.
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 retention bar chart looks different depending on whether the
product-analytics-hog-charts-retentionflag is on. Compared to the legacychart.js version, the hog-charts version:
in-progress (partial) period tail. The legacy chart only uses the
incompleteness indicator on the line view — the bar view shows nothing
special.
"L-shape" rather than a closed rectangle.
This is a re-PR of #60129 (closed because the branch went stale against master).
The substantive code change is identical; storybook baselines will be
regenerated by the visual-review bot on this branch.
Changes
RetentionBarChartstops forwardingincompletenessOffsetFromEndtobuildRetentionSeries, so nostroke.partialis set and the bar chart nolonger hatches in-progress bars. The line chart still receives the offset
and continues to dash the in-progress tail.
drawGridalways frames the plot area: vertical mode now draws both leftand right value-axis lines; horizontal mode draws top and bottom. Retention
bar (and every other hog-charts consumer) now reads as a closed rectangle
instead of an L-shape.
How did you test this code?
I'm an agent (Claude Code). Manual UI verification was not performed.
Automated tests run locally on the original branch:
Visual coverage comes from existing storybook stories — `Scenes-App/Insights/Retention - RetentionBar`
(gated on the hog-charts retention flag) plus the hog-charts bar/line/timeseries
stories that now read the framed plot area.
Publish to changelog?
no
Docs update
No docs change needed.
🤖 Agent context
Re-PR of #60129 authored by Claude Code (Opus 4.7) at the request of @SammyP.
The previous branch was closed after a stale-checkout incident produced a
broken commit (later reverted in-branch); rather than continue on that
history, this branch is freshly cut from master with the substantive code
commits cherry-picked. The bot-generated snapshot baseline commit from the
old PR was intentionally dropped because the baselines now conflict with
unrelated master changes — the visual-review bot will regenerate them here.
Decisions worth flagging:
flag. That keeps the API smaller and matches the legacy chart.js box-style
grid for every hog-charts consumer (trends bar, funnel steps, line charts,
retention) in one move. The original PR briefly went through an opt-in
shape before consolidating on this default — the consolidated version is
what ships here.
parameter on `buildRetentionSeries` to suppress the partial stroke. Same
outcome, smaller surface area, and the line chart keeps the existing
behavior.