Skip to content

feat: add tooltip visibility control on scroll - #278

Open
timothygachengo wants to merge 8 commits into
mainfrom
fix/make-tooltips-centered
Open

feat: add tooltip visibility control on scroll#278
timothygachengo wants to merge 8 commits into
mainfrom
fix/make-tooltips-centered

Conversation

@timothygachengo

@timothygachengo timothygachengo commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Description

Problem
Chart tooltips (Bar, Line, Pie, Sankey charts) had critical issues:

  • Off-screen tooltips — positioning logic didn't properly account for viewport boundaries, especially near edges
  • Scroll persistence — tooltips remained visible/stuck when users scrolled, creating jarring UX
  • Styling drops — tooltips rendered outside the theme's scoped DOM ancestry, losing background, borders, padding, and z-index styling
  • Animation lag — Nivo charts animate their tooltip wrapper with react-spring transforms, but the old positioning logic only tracked resize/scroll events, missing frame-by-frame movement

Solution
Migrated to @floating-ui/react with proper scroll detection and theme-scoped portaling:

Core Changes

  1. New useTooltipPosition() Hook (chart/Tooltip.jsx)
  • frame-by-frame tracking for animated wrappers
  1. Scroll Detection useHideTooltipOnScroll()
  • Passive listeners on scroll, touchmove, and global click (capturing phase)
  • Hides tooltip instantly when user scrolls or taps elsewhere
  • Never blocks scrolling (passive listeners)

Benefits

  • Tooltips stay within viewport bounds automatically (no more off-screen)
  • Tooltips hide immediately on scroll (smooth UX, no janky persistence)
  • Tooltips maintain full styling on all themes (background, border, padding, fonts, z-index)
  • Proper frame-by-frame positioning for animated chart wrappers
  • Touch-friendly (show on tap, don't dismiss with emulated mouse events)

NOTE

Projects will have to update their chart CSS where in the scss/default/index.scss or scss/theme-name/index.scss
where the chart SCSS file import should be moved outside of the .edit-post-visual-editor, #root {} block since the tooltip is now rendered on a portal.

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (BREAKING CHANGE:)
  • Refactor / chore (refactor: / chore:)
  • Documentation update (docs:)

Affected package(s)

  • @devgateway/dvz-ui-react
  • @devgateway/wp-react-lib
  • example only (no changeset needed)

Checklist

  • PR title follows Conventional Commits format
  • A changeset has been added (pnpm changeset) for any change to a published package
  • pnpm build passes locally
  • pnpm --filter @devgateway/* typecheck passes
  • No hardcoded credentials, internal URLs, client names, or PII introduced
  • Any new dependency has an Apache-2.0-compatible license

@pkg-pr-new

pkg-pr-new Bot commented Aug 13, 2026

Copy link
Copy Markdown

Open in StackBlitz

pnpm add https://pkg.pr.new/@devgateway/dvz-ui-react@c2abedc
pnpm add https://pkg.pr.new/@devgateway/wp-react-lib@c2abedc

commit: c2abedc

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates chart tooltips to Floating UI for viewport-aware positioning, scoped styling, and scroll dismissal.

Changes:

  • Adds shared tooltip positioning and visibility hooks.
  • Updates tooltip styling and chart integrations.
  • Adds dependencies and reduces broad Lodash imports.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.changeset/green-schools-poke.md Documents the tooltip feature release.
packages/dvz-ui/package.json Adds Floating UI and focused utility dependencies.
pnpm-lock.yaml Locks new dependency versions.
packages/dvz-ui/src/embeddable/chart/Tooltip.jsx Implements tooltip positioning and visibility control.
packages/dvz-ui/src/embeddable/common/ChartTooltip.jsx Applies shared positioning to additional charts.
packages/dvz-ui/src/scss/themes/default/_charts.scss Adds viewport-safe tooltip styling.
packages/dvz-ui/src/utils/semanticUtils.ts Replaces Lodash iteration with native APIs.
packages/dvz-ui/src/embeddable/chart/Radar.jsx Removes an unused Lodash import.
packages/dvz-ui/src/embeddable/big-filter/BigNumberGroup.jsx Uses the focused debounce package.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Suppressed comments (1)

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:248

  • onMouseOut bubbles when the pointer moves from this element onto any descendant. Since both rendering branches always add child elements, moving within the tooltip can set isHovered to false and hide it. onMouseLeave already handles actually leaving the tooltip without this bubbling behavior.
          onMouseOut={hide}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/dvz-ui/src/embeddable/chart/Tooltip.jsx Outdated
Comment thread packages/dvz-ui/src/embeddable/chart/Tooltip.jsx
Comment thread packages/dvz-ui/src/embeddable/chart/Tooltip.jsx
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 13, 2026 20:54
@pkg-pr-new

pkg-pr-new Bot commented Aug 13, 2026

Copy link
Copy Markdown

Open in StackBlitz

pnpm add https://pkg.pr.new/@devgateway/dvz-ui-react@cd85aa0
pnpm add https://pkg.pr.new/@devgateway/wp-react-lib@cd85aa0

commit: cd85aa0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Suppressed comments (4)

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:235

  • portalRoot is null on this first render, so FloatingPortal creates its portal under document.body. Once Floating UI has created that portal node, changing root does not relocate it, meaning the scoped .chart.tooltip rules can still be lost. Wait until the anchor callback has resolved the intended root before mounting the portal.
      <FloatingPortal root={portalRoot}>

packages/dvz-ui/src/embeddable/common/ChartTooltip.jsx:69

  • This portal is mounted while portalRoot is still null, causing Floating UI to create it under document.body; the later root update does not move an existing portal node. As a result, Sankey tooltips can still render outside the scoped theme ancestry. Mount the portal only after the anchor ref resolves its root.
                <FloatingPortal root={portalRoot}>

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:250

  • onMouseOut also fires when the pointer moves from this container into its inner <div> or any Markdown child. That normal movement hides the tooltip even though the pointer is still inside it. onMouseLeave already handles exiting the whole tooltip, so remove the bubbling handler.
          onMouseOut={hide}

.changeset/green-schools-poke.md:14

  • The PR description says consumers must relocate their chart SCSS import and classifies this as a breaking change, but this changeset declares no breaking change and requests only a minor release. A required consumer integration change is semver-major for this 1.x package; update both the changeset bump and its migration note.
**Breaking changes:** None. Internal implementation detail; public API unchanged.

Copilot AI review requested due to automatic review settings August 14, 2026 05:03
@pkg-pr-new

pkg-pr-new Bot commented Aug 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

pnpm add https://pkg.pr.new/@devgateway/dvz-ui-react@193689c
pnpm add https://pkg.pr.new/@devgateway/wp-react-lib@193689c

commit: 193689c

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Suppressed comments (5)

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:89

  • The global pointerdown listener also fires for presses inside the tooltip itself. Because it sets hideOnScroll before the tooltip's touch/mouse handlers run, tooltip links or other content cannot be clicked/tapped and handleTouchStart cannot make the tooltip visible again. The stated behavior is to dismiss on taps elsewhere, so the handler needs to ignore events whose target is inside the floating tooltip/reference.
    window.addEventListener("scroll", hide, { capture: true, passive: true });
    window.addEventListener("touchmove", hide, { capture: true, passive: true });
    window.addEventListener("pointerdown", hide, { capture: true, passive: true });

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:237

  • Hiding via CSS leaves the floating element mounted, so the autoUpdate(..., { animationFrame: true }) loop continues doing frame-by-frame measurements after scrolling. A sticky touch tooltip can remain mounted indefinitely, turning a dismissed tooltip into a permanent 60-fps layout cost. Detach the floating ref while hidden (or otherwise stop autoUpdate) and restore it when visible.
          ref={floatingRef}

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:250

  • onMouseOut bubbles when the pointer moves between descendants, so a Markdown tooltip will hide as soon as the cursor enters a paragraph, link, or other child. onMouseLeave already handles actually leaving the tooltip; remove the bubbling handler.
          onMouseOut={hide}

.changeset/green-schools-poke.md:2

  • The PR description marks this as a breaking change and explicitly requires consumers to move their chart SCSS import, but this changeset requests a minor release and says there are no breaking changes. Since the package is currently 1.9.7, either record a major bump and document the required migration here, or remove the migration requirement if compatibility is guaranteed.
"@devgateway/dvz-ui-react": minor

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:220

  • Calling preventDefault() for every tooltip touchend suppresses the synthesized click, so Markdown/raw-HTML links in the tooltip cannot be activated on touch devices. Avoid cancelling the default touch action; if emulated mouse events must be filtered, track the input modality and ignore only those mouse handlers instead.
  const handleTouchEnd = (event) => {
    // Prevent the emulated mouse events that follow a tap from immediately
    // hiding the tooltip we just opened via touch.
    event.preventDefault();

Copilot AI review requested due to automatic review settings August 18, 2026 10:57
@pkg-pr-new

pkg-pr-new Bot commented Aug 18, 2026

Copy link
Copy Markdown

Open in StackBlitz

pnpm add https://pkg.pr.new/@devgateway/dvz-ui-react@6985ba7
pnpm add https://pkg.pr.new/@devgateway/wp-react-lib@6985ba7

commit: 6985ba7

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Suppressed comments (7)

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:89

  • This capture listener also handles pointer presses inside the portaled tooltip. On touch, it sets isHidden before the tooltip's own onTouchStart, and that handler cannot clear isHidden, so tapping tooltip content immediately dismisses it. Ignore pointerdowns originating within the tooltip (while retaining scroll/touchmove dismissal).
    window.addEventListener("pointerdown", hide, { capture: true, passive: true });

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:245

  • After scroll dismissal this floating element remains mounted with only visibility: hidden. Because useTooltipPosition() enabled autoUpdate(..., { animationFrame: true }), the hidden/stuck Nivo tooltip continues measuring and updating every animation frame until Nivo eventually unmounts it. Unmount the FloatingPortal whenever isVisible is false (as ChartTooltip already does) or stop the auto-update subscription while hidden.
          style={{
            ...floatingStyles,
            visibility: isVisible ? "visible" : "hidden",
            pointerEvents: isVisible ? "auto" : "none",
          }}

packages/dvz-ui/src/embeddable/tooltip/index.tsx:66

  • The new floating element only has title-popup, while the removed Semantic UI Popup supplied the ui popup visible mini classes that provide its border, shadow, typography, and z-index. The new SCSS rule only restores background and padding, so this tooltip loses most of its prior presentation. Preserve the base popup classes (or reproduce all of those styles explicitly).
            className="title-popup"

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:250

  • mouseout bubbles when the pointer moves between descendants, so markdown containing links, tables, or other nested elements hides the tooltip even though the pointer is still inside it. mouseleave already handles actually exiting the tooltip; remove this handler.
          onMouseOut={hide}

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:221

  • Calling preventDefault() on the tooltip container's bubbling touchend also cancels the synthesized click for links or other interactive HTML rendered by Markdown/rehypeRaw. Limit cancellation to touches directly on the container so child controls remain usable.
  const handleTouchEnd = (event) => {
    // Prevent the emulated mouse events that follow a tap from immediately
    // hiding the tooltip we just opened via touch.
    event.preventDefault();

.changeset/green-schools-poke.md:14

  • The PR explicitly requires consumers to relocate chart SCSS imports and marks the change as breaking, but this changeset declares no breaking change and requests only a minor release. That would publish the integration break under a non-breaking version. Record the consumer action and use a major changeset.
"@devgateway/dvz-ui-react": minor

packages/dvz-ui/src/embeddable/tooltip/index.tsx:61

  • The interaction props are attached to the <span>, but keyboard focus is placed on Semantic UI's icon, which renders as an aria-hidden decorative element. Keyboard and screen-reader users therefore focus an unnamed element rather than the tooltip reference. Make the reference itself focusable and named.
      <span ref={setReference} {...getReferenceProps()}>
        <Icon name="question circle" tabIndex={0} />

Copilot AI review requested due to automatic review settings August 18, 2026 11:09
@timothygachengo
timothygachengo force-pushed the fix/make-tooltips-centered branch from 6985ba7 to 193689c Compare August 18, 2026 11:09
@pkg-pr-new

pkg-pr-new Bot commented Aug 18, 2026

Copy link
Copy Markdown

Open in StackBlitz

pnpm add https://pkg.pr.new/@devgateway/dvz-ui-react@193689c
pnpm add https://pkg.pr.new/@devgateway/wp-react-lib@193689c

commit: 193689c

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Suppressed comments (5)

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:89

  • The capture listener hides the tooltip for every pointer press, including presses inside the portaled tooltip itself. Because isHidden can only reset when d/str changes, tapping tooltip content (including links rendered from Markdown/HTML) dismisses it before activation and onTouchStart cannot make it visible again. Treat this as an outside press by excluding the floating element from the event path instead of hiding on all pointerdown events.
    window.addEventListener("scroll", hide, { capture: true, passive: true });
    window.addEventListener("touchmove", hide, { capture: true, passive: true });
    window.addEventListener("pointerdown", hide, { capture: true, passive: true });

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:250

  • onMouseOut fires when the pointer moves from this wrapper into any nested Markdown/HTML element because it bubbles. That immediately sets isHovered to false and disables pointer events, so the tooltip disappears while the user moves within its content. onMouseLeave already handles leaving the tooltip without firing for descendant transitions.
          onMouseOut={hide}

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:220

  • Calling preventDefault() on a bubbling touchend also cancels the synthesized click/default action for anchors or other interactive HTML rendered inside the tooltip. This makes Markdown/HTML links unusable on touch devices; suppress the specific unwanted mouse transition without cancelling the content's default touch action.
  const handleTouchEnd = (event) => {
    // Prevent the emulated mouse events that follow a tap from immediately
    // hiding the tooltip we just opened via touch.
    event.preventDefault();

.changeset/green-schools-poke.md:14

  • This conflicts with the PR's explicit downstream migration note and its breaking-change classification: requiring consumers to relocate their chart SCSS import is a breaking integration change even though the JavaScript API is unchanged. Either document that migration here and release accordingly, or remove the migration requirement if the scoped portal root makes it unnecessary.
**Breaking changes:** None. Internal implementation detail; public API unchanged.

.changeset/green-schools-poke.md:12

  • The GroupedBars implementation only imports formatContent from this module and does not render ChartTooltip, so it does not receive the new Floating UI positioning or scroll-dismissal behavior. Remove GroupedBars from this release-note claim unless its tooltip integration is also updated.
- Applied changes to Tooltip.jsx (Bar, Line, Pie charts) and ChartTooltip.jsx (Sankey, GroupedBars)

Copilot AI review requested due to automatic review settings August 19, 2026 09:45
@pkg-pr-new

pkg-pr-new Bot commented Aug 19, 2026

Copy link
Copy Markdown

Open in StackBlitz

pnpm add https://pkg.pr.new/@devgateway/dvz-ui-react@ccea00f
pnpm add https://pkg.pr.new/@devgateway/wp-react-lib@ccea00f

commit: ccea00f

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Suppressed comments (3)

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:89

  • This capture listener also runs when the user touches the tooltip itself, setting isHidden to true before handleTouchStart runs. That handler only resets isHovered, so the tooltip remains hidden despite the new touch handling. Ignore pointer downs originating inside the tooltip (the PR description only calls for dismissing taps elsewhere).
    window.addEventListener("pointerdown", hide, { capture: true, passive: true });

packages/dvz-ui/src/embeddable/chart/Tooltip.jsx:248

  • mouseout bubbles when the pointer moves between descendants. A Markdown/HTML tooltip with multiple elements therefore sets isHovered to false while the pointer is still inside it, and the resulting visibility: hidden prevents another enter event from restoring it. onMouseLeave already handles actually leaving the tooltip, so remove this handler.
          onMouseOut={hide}

.changeset/green-schools-poke.md:14

  • This contradicts the PR description, which marks the change as breaking and requires consumers to move their chart SCSS import. Reconcile the release note and changeset level with that migration requirement; if consumers really must update integration code, this 1.x package needs a major rather than minor changeset.
**Breaking changes:** None. Internal implementation detail; public API unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants