feat: support debouncing of the chart core tooltip #129

HrvojeHemen · 2025-11-26T11:52:31Z

Description

Tooltip is being rendered on each hover which is slowing down the chart significantly.
By adding an option to debounce by using renderDebounceDuration we can stop it from being rendered each hover, but instead render it once the user stops moving their mouse, after the delay passed.

Covered by unit tests and manual testing.
Backwards compatible by making the prop fully optional

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

pan-kot · 2025-11-26T18:35:50Z

src/core/components/core-tooltip.tsx

  const [expandedSeries, setExpandedSeries] = useState<ExpandedSeriesState>({});
  const tooltip = useSelector(api.tooltipStore, (s) => s);
-  if (!tooltip.visible || tooltip.group.length === 0) {
+  const [debouncedTooltip, setDebouncedTooltip] = useState<ReactiveTooltipState | null>(null);


Can we e.g. move the code that subscribes to the tooltip data and does debouncing into a small utility hook above that component so to encapsulate the debouncing logic inside?

pan-kot · 2025-12-02T08:34:36Z

src/core/interfaces.ts

    enabled?: boolean;
    placement?: "middle" | "outside" | "target";
    size?: "small" | "medium" | "large";
+    renderDebounceDuration?: number;


idea: I would rename this prop to just "debounce" as the "render" and "duration" parts can kind of be implied here. Additionally, I would consider making it then a union number | boolean. That is because while we cant add the debounce interval by default, ideally we still want the debounce interval to be consistent. This can be then achieved by using debounce: true to let the internal implementation use the internal constant for it.

pan-kot · 2025-12-02T08:37:12Z

src/core/components/use-debounced-tooltip.tsx

+import { ChartAPI } from "../chart-api";
+import { ReactiveTooltipState } from "../chart-api/chart-extra-tooltip";
+
+export function useDebouncedTooltip(api: ChartAPI, renderDebounceDuration: number = 500) {


Can we pass tooltip as a prop here instead of the api? In that case we can probably also make the argument type generic:

export function useDebounce<T>(value: T, debounceDuration = DEFAULT_DEBOUNCE_DURATION): null | T { // ... }

Thats a great suggestion, I'll make it generic and reuseable

pan-kot · 2025-12-02T08:38:29Z

src/core/components/use-debounced-tooltip.tsx

+export function useDebouncedTooltip(api: ChartAPI, renderDebounceDuration: number = 500) {
+  const tooltip = useSelector(api.tooltipStore, (s) => s);
+  const [debouncedTooltip, setDebouncedTooltip] = useState<ReactiveTooltipState | null>(null);
+  const [shouldRender, setShouldRender] = useState(renderDebounceDuration <= 0);


Why do we need this to be a state? Do we expect the debounce duration to change during components lifetime?

pan-kot · 2025-12-02T08:41:25Z

src/core/components/core-tooltip.tsx

-  if (!tooltip.visible || tooltip.group.length === 0) {
+  const debouncedTooltip = useDebouncedTooltip(api, renderDebounceDuration);
+
+  if (!debouncedTooltip?.visible || debouncedTooltip?.group?.length === 0) {


Can we change that to !debouncedTooltip || !debouncedTooltip?.visible || debouncedTooltip?.group?.length === 0? This should allow unnecessary conditions around the debouncedTooltip var below.

Or can those be already removed?

They need to stay with the !debouncedTooltip check at the front, this makes sure that if it's not debounced yet it doesn't render, and then if it is debounced it keeps the same behaviour

pan-kot · 2025-12-02T08:43:45Z

src/core/components/core-tooltip.tsx

  const [expandedSeries, setExpandedSeries] = useState<ExpandedSeriesState>({});
-  const tooltip = useSelector(api.tooltipStore, (s) => s);
-  if (!tooltip.visible || tooltip.group.length === 0) {
+  const debouncedTooltip = useDebouncedTooltip(api, renderDebounceDuration);


What is the point of defaulting the renderDebounceDuration to 500 inside the useDebouncedTooltip, when it defaults to 0 in this function anyways?

Left it from when I was debugging. Nice catch!

pan-kot · 2025-12-02T08:44:01Z

src/core/__tests__/chart-core-tooltip.test.tsx

    });

-    expect(getTooltipContentMock).toHaveBeenCalledTimes(2);
+    expect(getTooltipContentMock).toHaveBeenCalledTimes(1);


Why this change?

pan-kot · 2025-12-02T08:45:00Z

src/core/__tests__/chart-core-tooltip.test.tsx

    });
  });
+
+  describe("debounce functionality", () => {


Very good tests 👍

HrvojeHemen · 2025-12-03T14:50:51Z

pages/06-visual-tests/cartesian-tooltip.page.tsx

        }}
      />
+
+      <h2>Debounced Tooltip (500ms)</h2>


Added a chart here so that debounce can be tested on a page with it enabled and disabled

Note that this is a page for visual regression tests. I would prefer not having anything there that is not meant for visual testing and might add noise and/or flakiness. Since this is a core-only feature, what about having a separate small page in pages/03-core?

…defined

jperals · 2025-12-04T07:48:49Z

src/internal/utils/use-debounce.tsx

+
+import { DebouncedCall } from "./utils";
+
+export function useDebounce<T>(component: T, duration: number) {


I would suggest calling this function something like useDebouncedComponent or useDebouncedRender, since it is specific to rendering a component and not other types of calls

I suggested (#129 (comment)) calling that useDebouncedValue because it can technically use any value as input, the component arg name is also misleading in that regard.

jperals · 2025-12-04T07:55:42Z

pages/06-visual-tests/cartesian-tooltip.page.tsx

        }}
      />
+
+      <h2>Debounced Tooltip (500ms)</h2>


Note that this is a page for visual regression tests. I would prefer not having anything there that is not meant for visual testing and might add noise and/or flakiness. Since this is a core-only feature, what about having a separate small page in pages/03-core?

pan-kot · 2025-12-04T13:46:07Z

src/core/components/core-tooltip.tsx

  const [expandedSeries, setExpandedSeries] = useState<ExpandedSeriesState>({});
  const tooltip = useSelector(api.tooltipStore, (s) => s);
-  if (!tooltip.visible || tooltip.group.length === 0) {
+  const debouncedTooltip = useDebouncedComponent(tooltip, debounce === true ? DEFAULT_DEBOUNCE : debounce || 0);


The debouncedTooltip is not a component but a state. Should we rename the util to useDebouncedValue to avoid confusion? Likewise, inside the util we can have const [debouncedValue, setDebouncedValue] = useState<T | null>(null);

HrvojeHemen requested review from jperals and pan-kot as code owners November 26, 2025 11:52

HrvojeHemen had a problem deploying to dev-pages November 26, 2025 18:24 — with GitHub Actions Failure

pan-kot reviewed Nov 26, 2025

View reviewed changes

feat: support debouncing of the chart core tooltip

c903809

HrvojeHemen force-pushed the debounce-tooltip-wip branch from 329a5ba to c903809 Compare November 28, 2025 14:55

Merge branch 'cloudscape-design:main' into debounce-tooltip-wip

9edf5c0

HrvojeHemen requested a review from pan-kot November 28, 2025 14:57

HrvojeHemen had a problem deploying to dev-pages December 1, 2025 09:03 — with GitHub Actions Failure

pan-kot reviewed Dec 2, 2025

View reviewed changes

feat: support debouncing of the chart core tooltip v3

a510085

HrvojeHemen commented Dec 3, 2025

View reviewed changes

Hrvoje Hemen added 3 commits December 3, 2025 14:52

remove testing changes

8d6b937

remove ? from debouncedTooltip because we're checking whether it's …

294381f

…defined

last ? remnants that aren't needed

24ab8bf

HrvojeHemen had a problem deploying to dev-pages December 3, 2025 16:29 — with GitHub Actions Failure

jperals reviewed Dec 4, 2025

View reviewed changes

Final touches

4222caa

HrvojeHemen requested a review from a team as a code owner December 4, 2025 11:30

HrvojeHemen requested review from ClaudioGSDB and removed request for a team December 4, 2025 11:30

HrvojeHemen had a problem deploying to dev-pages December 4, 2025 13:44 — with GitHub Actions Failure

pan-kot reviewed Dec 4, 2025

View reviewed changes

component -> value

c309573

pan-kot approved these changes Dec 4, 2025

View reviewed changes

pan-kot enabled auto-merge December 4, 2025 16:49

HrvojeHemen had a problem deploying to dev-pages December 4, 2025 16:50 — with GitHub Actions Failure

pan-kot added this pull request to the merge queue Dec 4, 2025

Merged via the queue into cloudscape-design:main with commit 805cc3c Dec 4, 2025
42 of 44 checks passed

HrvojeHemen deleted the debounce-tooltip-wip branch December 5, 2025 10:12


		import { DebouncedCall } from "./utils";

		export function useDebounce<T>(component: T, duration: number) {

feat: support debouncing of the chart core tooltip #129

feat: support debouncing of the chart core tooltip #129

Uh oh!

Conversation

HrvojeHemen commented Nov 26, 2025

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pan-kot Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pan-kot Dec 4, 2025 •

edited

Loading