From 52c7534d91ed23485bcfe64e20d16b66d02ac230 Mon Sep 17 00:00:00 2001 From: Jerad Gallinger Date: Fri, 16 Dec 2022 20:29:02 +0000 Subject: [PATCH] Fix tooltip scrolling bug --- .changeset/fair-queens-sniff.md | 5 ++ .../PositionedOverlay/PositionedOverlay.tsx | 62 ++++++++++++++----- 2 files changed, 52 insertions(+), 15 deletions(-) create mode 100644 .changeset/fair-queens-sniff.md diff --git a/.changeset/fair-queens-sniff.md b/.changeset/fair-queens-sniff.md new file mode 100644 index 00000000000..5eb13c29734 --- /dev/null +++ b/.changeset/fair-queens-sniff.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': patch +--- + +Fixed a bug where Tooltips nested in Scrollable containers sometimes don't update their positions correctly diff --git a/polaris-react/src/components/PositionedOverlay/PositionedOverlay.tsx b/polaris-react/src/components/PositionedOverlay/PositionedOverlay.tsx index b946fd5f90b..2a219615df7 100644 --- a/polaris-react/src/components/PositionedOverlay/PositionedOverlay.tsx +++ b/polaris-react/src/components/PositionedOverlay/PositionedOverlay.tsx @@ -83,7 +83,7 @@ export class PositionedOverlay extends PureComponent< }; private overlay: HTMLElement | null = null; - private scrollableContainer: HTMLElement | Document | null = null; + private scrollableContainers: (HTMLElement | Document)[] = []; private observer: MutationObserver; constructor(props: PositionedOverlayProps) { @@ -93,23 +93,20 @@ export class PositionedOverlay extends PureComponent< } componentDidMount() { - this.scrollableContainer = Scrollable.forNode(this.props.activator); - if (this.scrollableContainer && !this.props.fixed) { - this.scrollableContainer.addEventListener( - 'scroll', - this.handleMeasurement, - ); + this.setScrollableContainers(); + + if (this.scrollableContainers.length && !this.props.fixed) { + this.registerScrollHandlers(); } + this.handleMeasurement(); } componentWillUnmount() { this.observer.disconnect(); - if (this.scrollableContainer && !this.props.fixed) { - this.scrollableContainer.removeEventListener( - 'scroll', - this.handleMeasurement, - ); + + if (this.scrollableContainers.length && !this.props.fixed) { + this.unregisterScrollHandlers(); } } @@ -160,6 +157,10 @@ export class PositionedOverlay extends PureComponent< ); } + get firstScrollableContainer(): HTMLElement | Document | null { + return this.scrollableContainers[0] ?? null; + } + forceUpdatePosition() { // Wait a single animation frame before re-measuring. // Consumer's may also need to setup their own timers for @@ -186,6 +187,37 @@ export class PositionedOverlay extends PureComponent< this.overlay = node; }; + private setScrollableContainers = () => { + const containers: (HTMLElement | Document)[] = []; + let scrollableContainer = Scrollable.forNode(this.props.activator); + + if (scrollableContainer) { + containers.push(scrollableContainer); + + while (scrollableContainer?.parentElement) { + scrollableContainer = Scrollable.forNode( + scrollableContainer.parentElement, + ); + + containers.push(scrollableContainer); + } + } + + this.scrollableContainers = containers; + }; + + private registerScrollHandlers = () => { + this.scrollableContainers.forEach((node) => { + node.addEventListener('scroll', this.handleMeasurement); + }); + }; + + private unregisterScrollHandlers = () => { + this.scrollableContainers.forEach((node) => { + node.removeEventListener('scroll', this.handleMeasurement); + }); + }; + private handleMeasurement = () => { const {lockPosition, top} = this.state; @@ -201,7 +233,7 @@ export class PositionedOverlay extends PureComponent< measuring: true, }), () => { - if (this.overlay == null || this.scrollableContainer == null) { + if (this.overlay == null || this.firstScrollableContainer == null) { return; } @@ -222,9 +254,9 @@ export class PositionedOverlay extends PureComponent< const activatorRect = getRectForNode(preferredActivator); const currentOverlayRect = getRectForNode(this.overlay); - const scrollableElement = isDocument(this.scrollableContainer) + const scrollableElement = isDocument(this.firstScrollableContainer) ? document.body - : this.scrollableContainer; + : this.firstScrollableContainer; const scrollableContainerRect = getRectForNode(scrollableElement); const overlayRect = fullWidth