Skip to content

Commit 7071af0

Browse files
fix: preserve page scrolling over popups (#4353)
1 parent 4e03d76 commit 7071af0

5 files changed

Lines changed: 146 additions & 0 deletions

File tree

‎src/components/ContentWithPopup/ContentWithPopup.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import React from 'react';
33
import type {PopupProps} from '@gravity-ui/uikit';
44
import {Popup} from '@gravity-ui/uikit';
55

6+
import {getPopupScrollContainer} from '../HoverPopup/getPopupScrollContainer';
7+
68
interface ContentWithPopupProps extends PopupProps {
79
content: React.ReactNode;
810
className?: string;
@@ -17,11 +19,13 @@ export const ContentWithPopup = ({
1719
pinOnClick,
1820
hasArrow = true,
1921
placement = ['top', 'bottom'],
22+
floatingStyles,
2023
...props
2124
}: ContentWithPopupProps) => {
2225
const [isPopupVisible, setIsPopupVisible] = React.useState(false);
2326
const [isPinned, setIsPinned] = React.useState(false);
2427
const anchor = React.useRef(null);
28+
const container = getPopupScrollContainer(anchor.current);
2529

2630
const showPopup = () => {
2731
setIsPopupVisible(true);
@@ -42,6 +46,9 @@ export const ContentWithPopup = ({
4246
return (
4347
<React.Fragment>
4448
<Popup
49+
container={container}
50+
strategy="fixed"
51+
floatingStyles={{fontFamily: 'var(--g-text-body-font-family)', ...floatingStyles}}
4552
anchorElement={anchor.current}
4653
open={isPinned || isPopupVisible}
4754
placement={placement}

‎src/components/HoverPopup/HoverPopup.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import debounce from 'lodash/debounce';
66

77
import {YDB_POPOVER_CLASS_NAME} from '../../utils/constants';
88

9+
import {getPopupScrollContainer} from './getPopupScrollContainer';
10+
911
const DEBOUNCE_TIMEOUT = 100;
1012

1113
function useVisibleAnchor(anchorElement: HTMLElement | null, open: boolean) {
@@ -148,6 +150,7 @@ export const HoverPopup = ({
148150
const anchorElement = anchorRef?.current || anchor.current;
149151
// Clipping a paired disk must not clear the shared hover state via onHidePopup.
150152
const isAnchorVisible = useVisibleAnchor(anchorElement, open);
153+
const container = getPopupScrollContainer(anchorElement);
151154

152155
return (
153156
<React.Fragment>
@@ -156,6 +159,9 @@ export const HoverPopup = ({
156159
</span>
157160
{anchorElement ? (
158161
<Popup
162+
container={container}
163+
// Keep portal typography when the page uses a different font.
164+
floatingStyles={{fontFamily: 'var(--g-text-body-font-family)'}}
159165
anchorElement={anchorElement}
160166
onOpenChange={(_open, _event, reason) => {
161167
if (reason === 'escape-key') {
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import {ThemeProvider} from '@gravity-ui/uikit';
2+
import {fireEvent, render, screen} from '@testing-library/react';
3+
4+
import {ContentWithPopup} from '../ContentWithPopup/ContentWithPopup';
5+
6+
import {getPopupScrollContainer} from './getPopupScrollContainer';
7+
8+
function makeScrollable(element: HTMLElement) {
9+
element.style.setProperty('overflow-y', 'auto');
10+
Object.defineProperties(element, {
11+
clientHeight: {value: 100},
12+
scrollHeight: {value: 200},
13+
});
14+
}
15+
16+
test('preserves document portals and keeps fullscreen portals inside the fullscreen boundary', () => {
17+
const outer = document.createElement('div');
18+
const fullscreen = document.createElement('div');
19+
const anchor = document.createElement('span');
20+
document.body.append(outer);
21+
outer.append(fullscreen);
22+
fullscreen.append(anchor);
23+
const original = Object.getOwnPropertyDescriptors(document);
24+
try {
25+
expect(getPopupScrollContainer(null)).toBeUndefined();
26+
expect(getPopupScrollContainer(anchor)).toBeUndefined();
27+
makeScrollable(outer);
28+
expect(getPopupScrollContainer(anchor)).toBe(outer);
29+
Object.defineProperty(document, 'scrollingElement', {configurable: true, value: outer});
30+
expect(getPopupScrollContainer(anchor)).toBeUndefined();
31+
Object.defineProperty(document, 'fullscreenElement', {
32+
configurable: true,
33+
value: fullscreen,
34+
});
35+
expect(getPopupScrollContainer(anchor)).toBe(fullscreen);
36+
} finally {
37+
outer.remove();
38+
for (const key of ['scrollingElement', 'fullscreenElement']) {
39+
if (original[key]) {
40+
Object.defineProperty(document, key, original[key]);
41+
} else {
42+
Reflect.deleteProperty(document, key);
43+
}
44+
}
45+
}
46+
});
47+
48+
test.each([false, true])(
49+
'ContentWithPopup renders in the selected portal (override: %s)',
50+
async (override) => {
51+
const host = document.createElement('div');
52+
makeScrollable(host);
53+
document.body.append(host);
54+
const explicitContainer = document.createElement('div');
55+
document.body.append(explicitContainer);
56+
const view = render(
57+
<ThemeProvider theme="light">
58+
<ContentWithPopup
59+
content="Popup details"
60+
{...(override ? {container: explicitContainer} : {})}
61+
>
62+
Open popup
63+
</ContentWithPopup>
64+
</ThemeProvider>,
65+
{container: host},
66+
);
67+
try {
68+
fireEvent.mouseEnter(screen.getByText('Open popup'));
69+
const popup = await screen.findByText('Popup details');
70+
expect((override ? explicitContainer : host).contains(popup)).toBe(true);
71+
} finally {
72+
view.unmount();
73+
host.remove();
74+
explicitContainer.remove();
75+
}
76+
},
77+
);
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
export function getPopupScrollContainer(anchor: HTMLElement | null) {
2+
if (!anchor) {
3+
return undefined;
4+
}
5+
const doc = anchor.ownerDocument;
6+
let parent = anchor.parentElement;
7+
while (parent) {
8+
if (
9+
parent === doc.fullscreenElement ||
10+
parent.classList.contains('ydb-fullscreen_fullscreen') ||
11+
(/auto|scroll/.test(doc.defaultView?.getComputedStyle(parent).overflowY ?? '') &&
12+
parent.scrollHeight > parent.clientHeight)
13+
) {
14+
// Document scrolling already works through the default portal.
15+
return parent === doc.scrollingElement ? undefined : parent;
16+
}
17+
parent = parent.parentElement;
18+
}
19+
return undefined;
20+
}

‎tests/suites/storage/vdiskColoring.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,42 @@ test('keeps paired disk popups inside the viewport without expanding the page',
515515
expect(maxPageWidthWhileClosing).toBe(pageWidth);
516516
});
517517

518+
test('wheel over disk popups scrolls the page', async ({page}) => {
519+
const response = createMockStorageGroupsResponse();
520+
const group = response.StorageGroups?.[0];
521+
if (!group) {
522+
throw new Error('Missing storage group fixture');
523+
}
524+
response.StorageGroups = Array.from({length: 40}, (_, index) => ({
525+
...group,
526+
GroupId: String(9000000000 + index),
527+
}));
528+
response.TotalGroups = response.FoundGroups = 40;
529+
await page.setViewportSize({width: 1500, height: 800});
530+
await enableExpertMode(page, VDisksGroupBy.State, false);
531+
await setupVDiskColoringMocks(page, response);
532+
await gotoStoragePage(page, VDisksGroupBy.State, false);
533+
await expectStorageGroupRowsReady(page, false);
534+
const scroll = page.locator('.ydb-cluster');
535+
for (const name of ['PDisk', 'VDisk']) {
536+
await scroll.evaluate((element) => element.scrollTo({top: 0, left: 0}));
537+
const row = getStorageGroupRow(page, 0);
538+
await (name === 'PDisk' ? getPDiskItems(row) : getVDiskItems(row)).first().hover();
539+
const action = page.getByRole('link', {name: `Go to ${name}`, exact: true});
540+
await expect(action).toHaveAttribute('href', /nodeId=7000/);
541+
const popup = page.locator('.ydb-popover').filter({has: action});
542+
await expect(popup).toHaveCSS('max-height', 'none');
543+
await expect(popup).toHaveCSS('overflow-y', 'visible');
544+
await action.hover();
545+
const before = await scroll.evaluate((element) => element.scrollTop);
546+
await page.mouse.wheel(0, 200);
547+
await expect
548+
.poll(() => scroll.evaluate((element) => element.scrollTop))
549+
.toBeGreaterThan(before);
550+
await page.keyboard.press('Escape');
551+
}
552+
});
553+
518554
test.describe('VDisk Coloring - Expert Mode visual snapshots', () => {
519555
test.describe.configure({timeout: 60_000});
520556

0 commit comments

Comments
 (0)