Skip to content

Commit 1e9c64f

Browse files
authored
fix: Avoid scrolling the entire page for charts with many series (#134)
* fix: Avoid scrolling the entire page for charts with many series * Add screenshot test for scroll-into-view logic.
1 parent 80443d9 commit 1e9c64f

File tree

4 files changed

+80
-13
lines changed

4 files changed

+80
-13
lines changed

src/internal/components/chart-legend/index.tsx

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
import Box, { BoxProps } from "@cloudscape-design/components/box";
1717
import { InternalChartTooltip } from "@cloudscape-design/components/internal/do-not-use/chart-tooltip";
1818

19+
import { scrollIntoViewNearestContainer } from "../../utils/dom";
1920
import { DebouncedCall } from "../../utils/utils";
2021
import { GetLegendTooltipContentProps, LegendItem, LegendTooltipContent } from "../interfaces";
2122

@@ -110,20 +111,11 @@ export const ChartLegend = ({
110111
if (isMouseInContainer.current) {
111112
return;
112113
}
113-
const container = containerRef.current;
114114
const element = elementsByIndexRef.current?.[highlightedIndex];
115-
if (!container || !element) {
115+
if (!element) {
116116
return;
117117
}
118-
const elementRect = element.getBoundingClientRect();
119-
const containerRect = container.getBoundingClientRect();
120-
const isVisible = elementRect.top >= containerRect.top && elementRect.bottom <= containerRect.bottom;
121-
if (!isVisible) {
122-
const elementCenter = elementRect.top + elementRect.height / 2;
123-
const containerCenter = containerRect.top + containerRect.height / 2;
124-
const top = container.scrollTop + (elementCenter - containerCenter);
125-
container.scrollTo({ top, behavior: "smooth" });
126-
}
118+
scrollIntoViewNearestContainer(element, { behavior: "smooth", scrollMode: "if-needed" });
127119
}, SCROLL_DELAY);
128120
}, [items, scrollIntoViewControl]);
129121

src/internal/components/series-details/index.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { BaseComponentProps } from "@cloudscape-design/components/internal/base-
99
import { InternalExpandableSection } from "@cloudscape-design/components/internal/do-not-use/expandable-section";
1010

1111
import { getDataAttributes } from "../../base-component/get-data-attributes";
12+
import { scrollIntoViewNearestContainer } from "../../utils/dom";
1213

1314
import styles from "./styles.css.js";
1415
import testClasses from "./test-classes/styles.css.js";
@@ -59,8 +60,8 @@ function ChartSeriesDetails(
5960

6061
const selectedIndex = details.findIndex((d) => d.highlighted);
6162
useEffect(() => {
62-
if (selectedIndex !== -1 && selectedRef.current && "scrollIntoView" in selectedRef.current) {
63-
selectedRef.current.scrollIntoView({ behavior: "smooth", block: "center" });
63+
if (selectedIndex !== -1 && selectedRef.current) {
64+
scrollIntoViewNearestContainer(selectedRef.current, { behavior: "smooth", scrollMode: "always" });
6465
}
6566
}, [selectedIndex]);
6667

src/internal/utils/dom.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
/**
5+
* Polyfill for `element.scrollIntoView({ container: "nearest", scrollMode: "..." })`
6+
* For ease of implementation, behaves like `{ block: "center" }` is provided.
7+
*
8+
* @see https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView (for `container`)
9+
* @see https://caniuse.com/wf-scroll-into-view-container (for `container` browser support)
10+
* @see https://github.com/w3c/csswg-drafts/pull/5677 (for discussion of `if-needed`)
11+
*/
12+
export function scrollIntoViewNearestContainer(
13+
element: HTMLElement,
14+
options?: ScrollOptions & { scrollMode?: "always" | "if-needed" },
15+
) {
16+
// Scroll methods (scrollTo, scrollIntoView, etc) are not supported in test environments like JSDOM.
17+
if (!("scrollTo" in element)) {
18+
return;
19+
}
20+
21+
// Get element boundaries to measure scroll offsets.
22+
// In the future, could be replaced with a simple `element.scrollParent`.
23+
// https://drafts.csswg.org/cssom-view/#dom-htmlelement-scrollparent
24+
const scrollParent = getScrollParent(element) ?? element.ownerDocument.documentElement;
25+
const { top: elementTop, bottom: elementBottom, height: elementHeight } = element.getBoundingClientRect();
26+
const { top: scrollTop, bottom: scrollBottom, height: scrollHeight } = scrollParent.getBoundingClientRect();
27+
28+
if (options?.scrollMode === "if-needed") {
29+
// Technically, getBoundingClientRect() returns the border-box, which includes the border width.
30+
// We should subtract the border width to get the padding box, but all that computation isn't worth it.
31+
const isNeeded = elementTop < scrollTop || elementBottom > scrollBottom;
32+
if (!isNeeded) {
33+
return;
34+
}
35+
}
36+
37+
const topRelativeToParent = elementTop + scrollParent.scrollTop - scrollTop;
38+
const blockCenterOffset = scrollHeight / 2 - elementHeight / 2;
39+
scrollParent.scrollTo({ top: topRelativeToParent - blockCenterOffset, ...options });
40+
}
41+
42+
function getScrollParent(element: HTMLElement): HTMLElement | null {
43+
for (
44+
let node: HTMLElement | null = element.parentElement;
45+
node !== null && node !== element.ownerDocument.body;
46+
node = node.parentElement
47+
) {
48+
const overflow = getComputedStyle(node).overflow;
49+
if (overflow !== "visible" && overflow !== "hidden") {
50+
return node;
51+
}
52+
}
53+
return null;
54+
}

test/visual/scroll.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import { expect, test } from "vitest";
5+
6+
import { setupScreenshotTest } from "../utils";
7+
8+
test(
9+
"scrolls highlighted element into view in tooltip and legend",
10+
setupScreenshotTest(
11+
"/#/01-cartesian-chart/many-series?screenshotMode=true&legendBottomMaxHeight=32",
12+
async (page) => {
13+
await page.waitForVisible(".screenshot-area");
14+
await page.waitForJsTimers(100);
15+
await page.hoverElement('[aria-label*="series 18"] [aria-label*="Sep 25\\a 01:15"]');
16+
const pngString = await page.fullPageScreenshot();
17+
expect(pngString).toMatchImageSnapshot();
18+
},
19+
),
20+
);

0 commit comments

Comments
 (0)