Repository navigation
fix(app): keep the heatmap tooltip from overflowing its tile #3618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
17c32c1
a8b5417
2a427ac
77558d6
2ffe44f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@hyperdx/app': patch | ||
| --- | ||
|
|
||
| fix: heatmap hover tooltip no longer adds scrollbars or shifts the tile; it now | ||
| sits above the chart instead of inside it |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; | |
| import cx from 'classnames'; | ||
| import uPlot from 'uplot'; | ||
| import UplotReact from 'uplot-react'; | ||
| import { useElementSize } from '@mantine/hooks'; | ||
| import { useElementSize, useMergedRef } from '@mantine/hooks'; | ||
|
|
||
| import { useStableValue } from '@/hooks/useStableValue'; | ||
| import { NumberFormat } from '@/types'; | ||
|
|
@@ -73,6 +73,16 @@ export function HeatmapPlot({ | |
| HighlightedPoint | undefined | ||
| >(undefined); | ||
|
|
||
| // The tooltip is fixed to the viewport, so a scroll under a still cursor | ||
| // would leave it behind; hide it until the next mouse move. | ||
| const hasHighlight = highlightedPoint != null; | ||
| useEffect(() => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — Copies the scroll-dismiss listener that already exists for the pinned time-chart tooltip
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| if (!hasHighlight) return; | ||
| const hide = () => setHighlightedPoint(undefined); | ||
| window.addEventListener('scroll', hide, { capture: true, passive: true }); | ||
| return () => window.removeEventListener('scroll', hide, { capture: true }); | ||
| }, [hasHighlight]); | ||
|
|
||
| // Gate tooltip display on actual mouse interaction. uPlot fires setCursor | ||
| // on init (before user hovers), which would show the tooltip on page load. | ||
| const mouseInsideRef = useRef(false); | ||
|
|
@@ -109,6 +119,8 @@ export function HeatmapPlot({ | |
| }); | ||
|
|
||
| const { ref, width, height } = useElementSize(); | ||
| const [container, setContainer] = useState<HTMLDivElement | null>(null); | ||
| const mergedRef = useMergedRef(ref, setContainer); | ||
|
|
||
| // Reapply the URL-backed selection whenever the bounds prop changes | ||
| // (e.g. a fresh drag-select arrives via the round-trip through the | ||
|
|
@@ -329,7 +341,7 @@ export function HeatmapPlot({ | |
|
|
||
| return ( | ||
| <div | ||
| ref={ref} | ||
| ref={mergedRef} | ||
| className={cx('heatmap-selection-container', className)} | ||
| style={{ width: '100%', height: '100%', position: 'relative' }} | ||
| onClick={() => { | ||
|
|
@@ -374,8 +386,7 @@ export function HeatmapPlot({ | |
| <HeatmapTooltip | ||
| point={highlightedPoint} | ||
| cell={highlightedCell} | ||
| width={width} | ||
| height={height} | ||
| containerRect={container?.getBoundingClientRect()} | ||
| showDragHint={!!onFilter} | ||
| /> | ||
| )} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 minor — Dismiss-on-scroll effect repeats the pinned tooltip's scroll listener in DBTimeChart
packages/app/src/components/DBTimeChart.tsx:246-262already registers a capture-phase, passivewindowscroll listener to close a fixed-position chart tooltip when anything scrolls. This effect is a second copy of that logic. Move it into a shared hook next touseChartTooltipZIndexincomponents/charts/ChartTooltip.tsx, for exampleuseDismissOnScroll(active, onDismiss, ignoreRef?), and use it in both places so the two body-portaled tooltips can't drift apart.Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving this out of this PR: sharing the listener means editing DBTimeChart.tsx and ChartTooltip.tsx, which this fix doesn't otherwise touch. Worth a follow-up.