Skip to content

fix(PopoverContainer): Give popover better scroll awareness #3138

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

Merged
merged 35 commits into from
Jul 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
19a45dc
fix(PopoverContainer): Give popover better scroll awareness
dreamwasp Jul 21, 2025
893c7c6
Merge branch 'main' into cass-gm-1236
dreamwasp Jul 21, 2025
a12f630
nicer example
dreamwasp Jul 21, 2025
bf5a0fd
Merge branch 'main' into cass-gm-1236
dreamwasp Jul 22, 2025
7df3952
merge dev
dreamwasp Jul 22, 2025
e28f95c
Merge branch 'main' into cass-gm-1236
dreamwasp Jul 22, 2025
6b48e64
remove example
dreamwasp Jul 22, 2025
8c5a4ef
update react-focus-on
dreamwasp Jul 22, 2025
5b50a83
tentative fix, need to do more testing
dreamwasp Jul 23, 2025
ce4ca53
need to optimize
dreamwasp Jul 23, 2025
d2b1526
keep this for now
dreamwasp Jul 23, 2025
8c99806
ready to test with LXStudio
dreamwasp Jul 23, 2025
0b95b2d
clean up PopoverContainer
dreamwasp Jul 23, 2025
271f4ba
need
dreamwasp Jul 24, 2025
3ca6377
Merge branch 'main' into cass-gm-1236
dreamwasp Jul 24, 2025
1560f76
swap to better prop name and start testing modals
dreamwasp Jul 24, 2025
525d16d
Merge branch 'cass-gm-1236' of github.com:Codecademy/gamut into cass-…
dreamwasp Jul 24, 2025
e3595d7
test overlay changes
dreamwasp Jul 24, 2025
2b98bce
fix tests (need to audit)
dreamwasp Jul 24, 2025
330ea6b
swap to better data-floating naming
dreamwasp Jul 24, 2025
961baa0
better comments
dreamwasp Jul 24, 2025
acba37f
fix docs
dreamwasp Jul 24, 2025
6a56b37
test scroll
dreamwasp Jul 24, 2025
aa655e5
Revert "test scroll"
dreamwasp Jul 24, 2025
15f2c46
test and look over updates
dreamwasp Jul 24, 2025
656aef9
Revert "test and look over updates"
dreamwasp Jul 24, 2025
2ff7c8e
test scrollingparents
dreamwasp Jul 24, 2025
1e1acb3
lint fix
dreamwasp Jul 24, 2025
15765f1
Popover scrolling parents test
dreamwasp Jul 24, 2025
be75610
move things around and share multi-level scrolling
dreamwasp Jul 24, 2025
fb4a130
remove commented out code
dreamwasp Jul 25, 2025
57a03dd
Merge branch 'main' into cass-gm-1236
dreamwasp Jul 25, 2025
5734e50
nicer example
dreamwasp Jul 25, 2025
90a1a08
fancier example
dreamwasp Jul 25, 2025
d1ca909
tooltip floating
dreamwasp Jul 25, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/gamut/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"polished": "^4.1.2",
"react-aria-components": "1.8.0",
"react-aria-tabpanel": "^4.4.0",
"react-focus-on": "^3.5.1",
"react-focus-on": "^3.10.0",
"react-hook-form": "^7.53.1",
"react-player": "^2.16.0",
"react-select": "^5.2.2",
Expand Down
1 change: 1 addition & 0 deletions packages/gamut/src/Overlay/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export const Overlay: React.FC<OverlayProps> = ({
<OverlayContainer
center
className={className}
data-floating="overlay"
data-testid="overlay-content-container"
inline={inline}
inset={0}
Expand Down
64 changes: 26 additions & 38 deletions packages/gamut/src/Popover/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import { useCallback, useEffect, useState } from 'react';
import { useWindowScroll, useWindowSize } from 'react-use';

import { FocusTrap } from '../FocusTrap';
import {
useResizingParentEffect,
useScrollingParentsEffect,
} from '../PopoverContainer/hooks';
import {
Beak,
BeakBox,
Expand All @@ -12,11 +16,7 @@ import {
} from './elements';
import { getBeakVariant } from './styles/beak';
import { PopoverProps } from './types';
import {
findResizingParent,
findScrollingParent,
getDefaultOffset,
} from './utils';
import { getDefaultOffset } from './utils';

export const Popover: React.FC<PopoverProps> = ({
animation,
Expand Down Expand Up @@ -105,39 +105,21 @@ export const Popover: React.FC<PopoverProps> = ({
setTargetRect(targetRef?.current?.getBoundingClientRect());
}, [targetRef, isOpen, width, height, x, y]);

useEffect(() => {
if (!targetRef.current) {
return;
}
const scrollingParent = findScrollingParent(
targetRef.current as HTMLElement
);
if (!scrollingParent?.addEventListener) {
return;
}
const handler = () => {
setTargetRect(targetRef?.current?.getBoundingClientRect());
};
scrollingParent.addEventListener('scroll', handler);
return () => scrollingParent.removeEventListener('scroll', handler);
}, [targetRef]);
// Update target rectangle when parent size/scroll changes
const updateTargetPosition = useCallback(
(rect?: DOMRect) => {
const target = targetRef?.current;
if (!target) return;

useEffect(() => {
// handles movement of target within a clipped container e.g. Drawer
if (!targetRef.current || typeof ResizeObserver === 'undefined') {
return;
}
const resizingParent = findResizingParent(targetRef.current as HTMLElement);
if (!resizingParent?.addEventListener) {
return;
}
const handler = () => {
setTargetRect(targetRef?.current?.getBoundingClientRect());
};
const ro = new ResizeObserver(handler);
ro.observe(resizingParent);
return () => ro.unobserve(resizingParent);
}, [targetRef]);
const newRect = rect || target.getBoundingClientRect();
setTargetRect(newRect);
},
[targetRef]
);

useScrollingParentsEffect(targetRef, updateTargetPosition);

useResizingParentEffect(targetRef, setTargetRect);

useEffect(() => {
if (targetRect) {
Expand All @@ -155,11 +137,16 @@ export const Popover: React.FC<PopoverProps> = ({

const handleClickOutside = useCallback(
(e: MouseEvent) => {
const target = e.target as Node;
const targetElement = targetRef.current;

if (!targetElement) return;

/**
* Allows targetRef to be or contain a button that toggles the popover open and closed.
* Without this check it would toggle closed then back open immediately.
*/
if (targetRef.current?.contains(e.target as Node)) return;
if (targetElement.contains(target)) return;

onRequestClose?.();
},
Expand All @@ -175,6 +162,7 @@ export const Popover: React.FC<PopoverProps> = ({
<PopoverContainer
align={align}
className={className}
data-floating="popover"
data-testid="popover-content-container"
position={position}
{...(popoverContainerRef ? { ref: popoverContainerRef } : {})}
Expand Down
30 changes: 0 additions & 30 deletions packages/gamut/src/Popover/utils.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,5 @@
import { PopoverProps } from './types';

export const findScrollingParent = ({
parentElement,
}: HTMLElement): HTMLElement | null => {
if (parentElement) {
const { overflow, overflowY, overflowX } = getComputedStyle(parentElement);
if (
[overflow, overflowY, overflowX].some((val) =>
['scroll', 'auto'].includes(val)
)
) {
return parentElement;
}
return findScrollingParent(parentElement); // parent of this parent is used via prop destructure
}
return null;
};

export const findResizingParent = ({
parentElement,
}: HTMLElement): HTMLElement | null => {
if (parentElement) {
const { overflow, overflowY, overflowX } = getComputedStyle(parentElement);
if ([overflow, overflowY, overflowX].some((val) => val === 'clip')) {
return parentElement;
}
return findResizingParent(parentElement); // parent of this parent is used via prop destructure
}
return null;
};

const offsets = {
primary: 20,
secondary: 15,
Expand Down
121 changes: 116 additions & 5 deletions packages/gamut/src/PopoverContainer/PopoverContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {

import { BodyPortal } from '../BodyPortal';
import { FocusTrap } from '../FocusTrap';
import { useResizingParentEffect, useScrollingParentsEffect } from './hooks';
import { ContainerState, PopoverContainerProps } from './types';
import { getContainers, getPosition, isInView } from './utils';

Expand All @@ -35,12 +36,15 @@ export const PopoverContainer: React.FC<PopoverContainerProps> = ({
isOpen,
onRequestClose,
targetRef,
allowPageInteraction,
...rest
}) => {
const popoverRef = useRef<HTMLDivElement>(null);
const hasRequestedCloseRef = useRef(false);
const { width: winW, height: winH } = useWindowSize();
const { x: winX, y: winY } = useWindowScroll();
const [containers, setContainers] = useState<ContainerState>();
const [targetRect, setTargetRect] = useState<DOMRect>();
const parent = containers?.parent;

const popoverPosition = useMemo(() => {
Expand All @@ -61,33 +65,140 @@ export const PopoverContainer: React.FC<PopoverContainerProps> = ({
const target = targetRef?.current;
if (!target) return;
setContainers(getContainers(target, inline, { x: winX, y: winY }));
}, [targetRef, inline, winW, winH, winX, winY]);
}, [targetRef, inline, winW, winH, winX, winY, targetRect]);

// Update target rectangle when window size/scroll changes
useEffect(() => {
setTargetRect(targetRef?.current?.getBoundingClientRect());
}, [targetRef, isOpen, winW, winH, winX, winY]);

// Update target rectangle when parent size/scroll changes
const updateTargetPosition = useCallback(
(rect?: DOMRect) => {
const target = targetRef?.current;
if (!target) return;

const newRect = rect || target.getBoundingClientRect();
setTargetRect(newRect);

const currentScrollX =
window.pageXOffset || document.documentElement.scrollLeft;
const currentScrollY =
window.pageYOffset || document.documentElement.scrollTop;

setContainers(
getContainers(target, inline, { x: currentScrollX, y: currentScrollY })
);
},
[targetRef, inline]
);

useScrollingParentsEffect(targetRef, updateTargetPosition);

useResizingParentEffect(targetRef, setTargetRect);

useIsomorphicLayoutEffect(() => {
if (containers?.viewport && !isInView(containers?.viewport)) {
if (
containers?.viewport &&
!isInView(containers?.viewport) &&
!hasRequestedCloseRef.current
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe something to include here is to check against the PopoverContainer's getBoundingClientRect and compare against the parents rect's top/bottom/left/right and if the popover goes out of bounds, then also close the popover?

Would probably need to check with design and can be ticketed as a fast follow

) {
hasRequestedCloseRef.current = true;
onRequestClose?.();
} else if (containers?.viewport && isInView(containers?.viewport)) {
hasRequestedCloseRef.current = false;
}
}, [containers?.viewport, onRequestClose]);

/**
* Allows targetRef to be or contain a button that toggles the popover open and closed.
* Without this check it would toggle closed then back open immediately.
*
*/
const handleClickOutside = useCallback(
(e: MouseEvent | TouchEvent) =>
!targetRef.current?.contains(e.target as Node) && onRequestClose?.(),
(e: MouseEvent | TouchEvent) => {
const target = e.target as Node;
const targetElement = targetRef.current;

if (!targetElement) return;
if (targetElement.contains(target)) return;
if (popoverRef.current?.contains(target)) return;

// If we get here, it's a genuine outside click
onRequestClose?.();
},
[onRequestClose, targetRef]
);

/**
* Backup click outside handler for cases where FocusTrap detection might be interfered with
* by our own floating elements
*/
const handleGlobalClickOutside = useCallback(
(e: MouseEvent) => {
const target = e.target as Node;
const targetElement = targetRef.current;

if (!targetElement || !isOpen) return;

if (
targetElement.contains(target) ||
popoverRef.current?.contains(target)
)
return;

// Check if the clicked element is within an Overlay component
const clickedElement = target as Element;
if (clickedElement.closest('[data-floating="overlay"]')) {
return;
}

// Check if the clicked element is within another Popover or PopoverContainer
const isFloatingElement = clickedElement.closest(
'[data-floating="popover"]'
);
if (
isFloatingElement &&
!popoverRef.current?.contains(isFloatingElement)
) {
onRequestClose?.();
return;
}

onRequestClose?.();
},
[onRequestClose, targetRef, isOpen]
);

// Backup global click listener for when a Popover or PopoverContainer is open
useEffect(() => {
if (isOpen) {
// Use a small delay to ensure this doesn't interfere with the FocusTrap's own detection
const timeoutId = setTimeout(() => {
document.addEventListener('mousedown', handleGlobalClickOutside, true);
}, 50);

return () => {
clearTimeout(timeoutId);
document.removeEventListener(
'mousedown',
handleGlobalClickOutside,
true
);
};
}
}, [isOpen, handleGlobalClickOutside]);

if (!isOpen || !targetRef) return null;

const content = (
<FocusTrap
allowPageInteraction={inline}
allowPageInteraction={inline || allowPageInteraction}
onClickOutside={handleClickOutside}
onEscapeKey={onRequestClose}
>
<PopoverContent
data-floating="popover"
data-testid="popover-content-container"
position="absolute"
ref={popoverRef}
Expand Down
71 changes: 71 additions & 0 deletions packages/gamut/src/PopoverContainer/hooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { useEffect } from 'react';

import { findAllAdditionalScrollingParents, findResizingParent } from './utils';

export const useScrollingParentsEffect = (
targetRef: React.RefObject<
Pick<HTMLDivElement, 'getBoundingClientRect' | 'contains'>
>,
setTargetRect: (rect: DOMRect | undefined) => void
) => {
useEffect(() => {
if (!targetRef.current) {
return;
}

const target = targetRef.current as unknown as HTMLElement;
const scrollingParents = findAllAdditionalScrollingParents(target);

const updatePosition = () => {
setTargetRect(targetRef?.current?.getBoundingClientRect());
};

// For immediate updates during scroll
const immediateUpdate = () => {
updatePosition();
};

const cleanup: (() => void)[] = [];

// Add listeners to all scrolling parents (window scroll handled by useWindowScroll)
scrollingParents.forEach((parent) => {
if (parent.addEventListener) {
// Use immediate update for smoother experience
parent.addEventListener('scroll', immediateUpdate, { passive: true });
cleanup.push(() =>
parent.removeEventListener('scroll', immediateUpdate)
);
}
});

return () => {
cleanup.forEach((fn) => fn());
};
}, [targetRef, setTargetRect]);
};

export const useResizingParentEffect = (
targetRef: React.RefObject<
Pick<HTMLDivElement, 'getBoundingClientRect' | 'contains'>
>,
setTargetRect: (rect: DOMRect | undefined) => void
) => {
useEffect(() => {
// handles movement of target within a clipped container e.g. Drawer
if (!targetRef.current || typeof ResizeObserver === 'undefined') {
return;
}
const resizingParent = findResizingParent(
targetRef.current as unknown as HTMLElement
);
if (!resizingParent?.addEventListener) {
return;
}
const handler = () => {
setTargetRect(targetRef?.current?.getBoundingClientRect());
};
const ro = new ResizeObserver(handler);
ro.observe(resizingParent);
return () => ro.unobserve(resizingParent);
}, [targetRef, setTargetRect]);
};
Loading
Loading