fix(hog-charts): fix tooltip scroll behaviour and tighten horizontal padding#66366
Closed
sampennington wants to merge 4 commits into
Closed
fix(hog-charts): fix tooltip scroll behaviour and tighten horizontal padding#66366sampennington wants to merge 4 commits into
sampennington wants to merge 4 commits into
Conversation
Now that DefaultTooltip scrolls to and bolds the closest series, promoting it to row 0 is redundant and disrupts the stable sort order. Keeping the series in their original position and letting scroll+bold do the work is better UX. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- DefaultTooltip: add subtle background highlight (8% white) + border-radius to the closest-series row so it reads clearly at a glance, not just bold - sqlLineGraphAdapter: bar charts now use buildSqlTooltipConfig() instead of a minimal inline config, giving them per-column formatting, total row, and value sorting to match line and combo charts Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…padding Reset scroll to top when hovering a new data point so the list always starts at the top. Only scroll the closest row into view when it's actually outside the visible area, and account for the 20% fade gradient at each end so rows aren't left sitting inside the fade zone. Add row padding (py-0.5 px-1.5) and reduce the surface paddingInline from 0.75rem to 0.5rem to close the excess horizontal gap. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Size Change: +7.96 kB (+0.01%) Total Size: 64.3 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
Reviews (1): Last reviewed commit: "fix(hog-charts): fix tooltip scroll beha..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
sampennington
commented
Jun 26, 2026
sampennington
left a comment
sampennington
left a comment
Contributor
Author
There was a problem hiding this comment.
🤖 sp-ship · 🚫 Blockers
Clean, well-scoped tooltip fix — the narrowSeriesByCursor simplification (filter-only, let→const/revalued) is a genuine improvement and a net reduction in hover-path work. Two things block merge.
(1) Broken existing tests → CI will be red. Removing the "promote hovered segment to seriesData[0]" reorder breaks tests in packages/quill/packages/charts/src/charts/BarChart/BarChart.test.tsx that still pin the old behaviour: it('stacked tooltip bubbles the cursor-resolved segment to seriesData[0]') (asserts seriesData[0].series.key at lines 258/267) and the parameterized tooltip narrows to the visible segment … (lines 429-430). That file isn't in this PR's diff, so it isn't flagged inline — but it must be updated to the new filter-only contract (rows keep their original series order; the hovered segment is identified via data-closest/key, not index 0).
(2) The closest-row highlight is invisible on a light-theme tooltip (see inline comment).
Minor / non-blocking: the two added comments on the scroll effects (around lines 68 and 76) largely narrate the adjacent code and could be dropped.
Replace the hardcoded `rgba(255,255,255,0.08)` background with `bg-current/8` (8% opacity of the text color) and `rounded-sm` (the `--radius-sm` token) so the highlight is visible on both light and dark tooltip surfaces. Generated-By: PostHog Code Task-Id: 965ea287-3864-46cb-bbc3-cf5c295f8392
Contributor
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
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
In stacked bar charts, the tooltip's closest-row scroll was firing on every hover position change as the cursor moved up/down within a bar. Because the tooltip list order doesn't match the bar's visual stacking order, this caused the list to constantly jump mid-scroll, cutting off rows at the top. Separately, rows near the bottom of a scrollable tooltip could sit inside the 20% fade gradient and appear unreadable even though they were technically within bounds.
Changes
TooltipSurfacepaddingInlinefrom0.75remto0.5remto close excess horizontal gappy-0.5 px-1.5padding to tooltip rows for the highlighted row background to have breathing roomHow did you test this code?
I'm an agent — no manual testing done. No automated tests cover this scroll behaviour.
Publish to changelog?