diff --git a/.changeset/four-kids-nail.md b/.changeset/four-kids-nail.md new file mode 100644 index 00000000000..0653c576d19 --- /dev/null +++ b/.changeset/four-kids-nail.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': minor +--- + +Fixed issues with Popover, Tooltip, Color Picker and Drop Zone not working correctly when used inside Modals for embedded apps. diff --git a/polaris-react/src/components/ColorPicker/ColorPicker.tsx b/polaris-react/src/components/ColorPicker/ColorPicker.tsx index a768ed83625..5323bfbfaac 100644 --- a/polaris-react/src/components/ColorPicker/ColorPicker.tsx +++ b/polaris-react/src/components/ColorPicker/ColorPicker.tsx @@ -5,8 +5,6 @@ import {clamp} from '../../utilities/clamp'; import {classNames} from '../../utilities/css'; import {hsbToRgb} from '../../utilities/color-transformers'; import type {HSBColor, HSBAColor} from '../../utilities/color-types'; -// eslint-disable-next-line import/no-deprecated -import {EventListener} from '../EventListener'; import {AlphaPicker, HuePicker, Slidable} from './components'; import type {SlidableProps} from './components'; @@ -67,12 +65,21 @@ export class ColorPicker extends PureComponent { {leading: true, trailing: true, maxWait: RESIZE_DEBOUNCE_TIME_MS}, ); + private observer?: ResizeObserver; + + componentWillUnmount() { + this.observer?.disconnect(); + } + componentDidMount() { const {colorNode} = this; - if (colorNode == null) { + if (!colorNode) { return; } + this.observer = new ResizeObserver(this.handleResize); + this.observer.observe(colorNode); + this.setState({ pickerSize: { width: colorNode.clientWidth, @@ -135,7 +142,6 @@ export class ColorPicker extends PureComponent { {alphaSliderMarkup} - ); } diff --git a/polaris-react/src/components/ColorPicker/components/AlphaPicker/AlphaPicker.tsx b/polaris-react/src/components/ColorPicker/components/AlphaPicker/AlphaPicker.tsx index 2beadfb8e20..541e84b1725 100644 --- a/polaris-react/src/components/ColorPicker/components/AlphaPicker/AlphaPicker.tsx +++ b/polaris-react/src/components/ColorPicker/components/AlphaPicker/AlphaPicker.tsx @@ -25,6 +25,24 @@ export class AlphaPicker extends PureComponent { draggerHeight: 0, }; + private node: HTMLElement | null = null; + private observer?: ResizeObserver; + + componentWillUnmount() { + this.observer?.disconnect(); + } + + componentDidMount() { + if (!this.node) { + return; + } + + this.observer = new ResizeObserver(this.setSliderHeight); + this.observer.observe(this.node); + + this.setSliderHeight(); + } + render() { const {color, alpha} = this.props; const {sliderHeight, draggerHeight} = this.state; @@ -33,7 +51,7 @@ export class AlphaPicker extends PureComponent { const background = alphaGradientForColor(color); return ( -
+
{ ); } - private setSliderHeight = (node: HTMLElement | null) => { - if (node == null) { + private setNode = (node: HTMLElement | null) => { + if (!node) { + return; + } + + this.node = node; + }; + + private setSliderHeight = () => { + const {node} = this; + if (!node) { return; } diff --git a/polaris-react/src/components/ColorPicker/components/AlphaPicker/tests/AlphaPicker.test.tsx b/polaris-react/src/components/ColorPicker/components/AlphaPicker/tests/AlphaPicker.test.tsx index b1ae4399efb..5d959da9bdd 100644 --- a/polaris-react/src/components/ColorPicker/components/AlphaPicker/tests/AlphaPicker.test.tsx +++ b/polaris-react/src/components/ColorPicker/components/AlphaPicker/tests/AlphaPicker.test.tsx @@ -79,6 +79,40 @@ describe('', () => { }); }); }); + + describe('Iframe React portal bug fix', () => { + it('observes the resize event for the activator wrapper', () => { + const observe = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe, + unobserve: jest.fn(), + disconnect: jest.fn(), + })); + + const alphaPicker = mountWithApp(); + + expect(observe).toHaveBeenCalledWith(alphaPicker.find('div')?.domNode); + }); + + it('disconnects the resize observer when component unmounts', () => { + const disconnect = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: jest.fn(), + unobserve: jest.fn(), + disconnect, + })); + + const alphaPicker = mountWithApp(); + + alphaPicker.unmount(); + + expect(disconnect).toHaveBeenCalled(); + }); + }); }); function noop() {} diff --git a/polaris-react/src/components/ColorPicker/components/HuePicker/HuePicker.tsx b/polaris-react/src/components/ColorPicker/components/HuePicker/HuePicker.tsx index c6c9327d35c..89f29ddc8b1 100644 --- a/polaris-react/src/components/ColorPicker/components/HuePicker/HuePicker.tsx +++ b/polaris-react/src/components/ColorPicker/components/HuePicker/HuePicker.tsx @@ -22,13 +22,31 @@ export class HuePicker extends PureComponent { draggerHeight: 0, }; + private node: HTMLElement | null = null; + private observer?: ResizeObserver; + + componentWillUnmount() { + this.observer?.disconnect(); + } + + componentDidMount() { + if (!this.node) { + return; + } + + this.observer = new ResizeObserver(this.setSliderHeight); + this.observer.observe(this.node); + + this.setSliderHeight(); + } + render() { const {hue} = this.props; const {sliderHeight, draggerHeight} = this.state; const draggerY = calculateDraggerY(hue, sliderHeight, draggerHeight); return ( -
+
{ ); } - private setSliderHeight = (node: HTMLElement | null) => { - if (node == null) { + private setNode = (node: HTMLElement | null) => { + if (!node) { + return; + } + + this.node = node; + }; + + private setSliderHeight = () => { + const {node} = this; + + if (!node) { return; } diff --git a/polaris-react/src/components/ColorPicker/components/HuePicker/tests/HuePicker.test.tsx b/polaris-react/src/components/ColorPicker/components/HuePicker/tests/HuePicker.test.tsx index 029c02de628..22fa6e7b3bf 100644 --- a/polaris-react/src/components/ColorPicker/components/HuePicker/tests/HuePicker.test.tsx +++ b/polaris-react/src/components/ColorPicker/components/HuePicker/tests/HuePicker.test.tsx @@ -67,6 +67,40 @@ describe('', () => { }); }); }); + + describe('Iframe React portal bug fix', () => { + it('observes the resize event for the activator wrapper', () => { + const observe = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe, + unobserve: jest.fn(), + disconnect: jest.fn(), + })); + + const huePicker = mountWithApp(); + + expect(observe).toHaveBeenCalledWith(huePicker.find('div')?.domNode); + }); + + it('disconnects the resize observer when component unmounts', () => { + const disconnect = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: jest.fn(), + unobserve: jest.fn(), + disconnect, + })); + + const huePicker = mountWithApp(); + + huePicker.unmount(); + + expect(disconnect).toHaveBeenCalled(); + }); + }); }); function noop() {} diff --git a/polaris-react/src/components/ColorPicker/components/Slidable/Slidable.tsx b/polaris-react/src/components/ColorPicker/components/Slidable/Slidable.tsx index acd969a86bc..e16038dfdd8 100644 --- a/polaris-react/src/components/ColorPicker/components/Slidable/Slidable.tsx +++ b/polaris-react/src/components/ColorPicker/components/Slidable/Slidable.tsx @@ -12,6 +12,7 @@ interface Position { interface State { dragging: boolean; + window?: Window | null; } export interface SlidableProps { @@ -49,29 +50,40 @@ export class Slidable extends PureComponent { private node: HTMLElement | null = null; private draggerNode: HTMLElement | null = null; + private observer?: ResizeObserver; + + componentWillUnmount() { + this.observer?.disconnect(); + } componentDidMount() { - const {onDraggerHeight} = this.props; - if (onDraggerHeight == null) { + if (!this.node) { return; } - const {draggerNode} = this; - if (draggerNode == null) { - return; - } + this.observer = new ResizeObserver(() => { + /** + * This is a workaround to enable event listeners to be + * re-attached when moving from one document to another + * when using a React portal across iframes. + * Using a resize observer works because when the clientWidth + * will go from 0 to the real width after the node + * gets rendered in its new place. + */ + const {window} = this.state; + if (window !== this.node?.ownerDocument.defaultView) { + this.setState({window: this.node?.ownerDocument.defaultView}); + } + this.handleResize(); + }); - onDraggerHeight(draggerNode.clientWidth); + this.observer.observe(this.node); - if (process.env.NODE_ENV === 'development') { - setTimeout(() => { - onDraggerHeight(draggerNode.clientWidth); - }, 0); - } + this.handleResize(); } render() { - const {dragging} = this.state; + const {dragging, window} = this.state; const {draggerX = 0, draggerY = 0} = this.props; const draggerPositioning = { @@ -83,6 +95,7 @@ export class Slidable extends PureComponent { event="mousemove" handler={this.handleMove} passive={false} + window={window} /> ) : null; @@ -91,19 +104,32 @@ export class Slidable extends PureComponent { event="touchmove" handler={this.handleMove} passive={false} + window={window} /> ) : null; const endDragListener = dragging ? ( - + ) : null; const touchEndListener = dragging ? ( - + ) : null; const touchCancelListener = dragging ? ( - + ) : null; return ( @@ -127,6 +153,27 @@ export class Slidable extends PureComponent { ); } + private handleResize() { + const {onDraggerHeight} = this.props; + if (!onDraggerHeight) { + return; + } + + const {draggerNode} = this; + + if (!draggerNode) { + return; + } + + onDraggerHeight(draggerNode.clientWidth); + + if (process.env.NODE_ENV === 'development') { + setTimeout(() => { + onDraggerHeight(draggerNode.clientWidth); + }, 0); + } + } + private setDraggerNode = (node: HTMLElement | null) => { this.draggerNode = node; }; diff --git a/polaris-react/src/components/ColorPicker/components/Slidable/tests/Slidable.test.tsx b/polaris-react/src/components/ColorPicker/components/Slidable/tests/Slidable.test.tsx index 190772355cb..c53215302de 100644 --- a/polaris-react/src/components/ColorPicker/components/Slidable/tests/Slidable.test.tsx +++ b/polaris-react/src/components/ColorPicker/components/Slidable/tests/Slidable.test.tsx @@ -1,6 +1,9 @@ +/* eslint-disable import/no-deprecated */ import React from 'react'; import {mountWithApp} from 'tests/utilities'; +import {EventListener} from '../../../../EventListener'; +import styles from '../../../ColorPicker.module.css'; import {Slidable} from '../Slidable'; describe('', () => { @@ -10,4 +13,131 @@ describe('', () => { slidable.find('div')!.trigger('onMouseDown', {}); expect(spy).not.toHaveBeenCalled(); }); + + describe('Iframe React portal bug fix', () => { + it('observes the resize event for the activator', () => { + const observe = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe, + unobserve: jest.fn(), + disconnect: jest.fn(), + })); + + const slidable = mountWithApp(); + + expect(observe).toHaveBeenCalledWith(slidable.find('div')?.domNode); + }); + + it('disconnects the resize observer when component unmounts', () => { + const disconnect = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: jest.fn(), + unobserve: jest.fn(), + disconnect, + })); + + const slidable = mountWithApp(); + + slidable.unmount(); + + expect(disconnect).toHaveBeenCalled(); + }); + + it('passes the node.ownerDocument.defaultView as the window for all event listeners after resize observer fires', () => { + let resizeCallback: () => void; + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation((callback) => { + resizeCallback = callback; + return { + observe: jest.fn(), + unobserve: jest.fn(), + disconnect: jest.fn(), + }; + }); + + const slidable = mountWithApp(); + const node = slidable.find('div')?.domNode; + + slidable.act(() => resizeCallback()); + + slidable.forceUpdate(); + + slidable.find('div')!.trigger('onMouseDown', { + type: 'mousedown', + clientX: 1, + clientY: 1, + }); + + // We can't assert using `toContainReactComponent` because the matcher blows up trying to assert on window as an argument + expect( + slidable + .find(EventListener, { + event: 'mousemove', + })! + .prop('window'), + ).toStrictEqual(node!.ownerDocument.defaultView); + + expect( + slidable + .find(EventListener, { + event: 'touchmove', + })! + .prop('window'), + ).toStrictEqual(node!.ownerDocument.defaultView); + + expect( + slidable + .find(EventListener, { + event: 'mouseup', + })! + .prop('window'), + ).toStrictEqual(node!.ownerDocument.defaultView); + + expect( + slidable + .find(EventListener, { + event: 'touchend', + })! + .prop('window'), + ).toStrictEqual(node!.ownerDocument.defaultView); + + expect( + slidable + .find(EventListener, { + event: 'touchcancel', + })! + .prop('window'), + ).toStrictEqual(node!.ownerDocument.defaultView); + }); + + it('calls provided onDraggerHeight after resize observer fires', () => { + let resizeCallback: () => void; + const onDraggerHeight = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation((callback) => { + resizeCallback = callback; + return { + observe: jest.fn(), + unobserve: jest.fn(), + disconnect: jest.fn(), + }; + }); + + const slidable = mountWithApp( + , + ); + + slidable.act(() => resizeCallback()); + + expect(onDraggerHeight).toHaveBeenCalledWith( + slidable.find('div', {className: styles.Dragger})?.domNode?.clientWidth, + ); + }); + }); }); diff --git a/polaris-react/src/components/ColorPicker/tests/ColorPicker.test.tsx b/polaris-react/src/components/ColorPicker/tests/ColorPicker.test.tsx index 5040d2847ad..c9996c50ebb 100644 --- a/polaris-react/src/components/ColorPicker/tests/ColorPicker.test.tsx +++ b/polaris-react/src/components/ColorPicker/tests/ColorPicker.test.tsx @@ -5,6 +5,7 @@ import {mountWithApp} from 'tests/utilities'; import {EventListener} from '../../EventListener'; import {Slidable, AlphaPicker} from '../components'; import {ColorPicker} from '../ColorPicker'; +import styles from '../ColorPicker.module.css'; const red = { hue: 0, @@ -179,6 +180,45 @@ describe('', () => { expect(event.preventDefault).not.toHaveBeenCalled(); }); }); + + describe('Iframe React portal bug fix', () => { + it('observes the resize event for the activator wrapper', () => { + const observe = jest.fn(); + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe, + unobserve: jest.fn(), + disconnect: jest.fn(), + })); + + const colorPicker = mountWithApp( + , + ); + + expect(observe).toHaveBeenCalledWith( + colorPicker.find('div', {className: styles.MainColor})?.domNode, + ); + }); + + it('disconnects the resize observer when component unmounts', () => { + const disconnect = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: jest.fn(), + unobserve: jest.fn(), + disconnect, + })); + + const colorPicker = mountWithApp( + , + ); + + colorPicker.unmount(); + + expect(disconnect).toHaveBeenCalled(); + }); + }); }); function noop() {} diff --git a/polaris-react/src/components/DropZone/DropZone.tsx b/polaris-react/src/components/DropZone/DropZone.tsx index 25ed2d3ac32..f20c46a86e6 100755 --- a/polaris-react/src/components/DropZone/DropZone.tsx +++ b/polaris-react/src/components/DropZone/DropZone.tsx @@ -266,6 +266,10 @@ export const DropZone: React.FunctionComponent & { [disabled, onDragOver], ); + const document = isServer + ? null + : node.current?.ownerDocument || globalThis.document; + const handleDragLeave = useCallback( (event: DropZoneEvent) => { event.preventDefault(); @@ -285,7 +289,7 @@ export const DropZone: React.FunctionComponent & { onDragLeave && onDragLeave(); }, - [dropOnPage, disabled, onDragLeave], + [disabled, onDragLeave, dropOnPage, document], ); const dropNode = dropOnPage && !isServer ? document : node.current; @@ -294,12 +298,24 @@ export const DropZone: React.FunctionComponent & { useEventListener('dragover', handleDragOver, dropNode); useEventListener('dragenter', handleDragEnter, dropNode); useEventListener('dragleave', handleDragLeave, dropNode); - useEventListener('resize', adjustSize, isServer ? null : window); useComponentDidMount(() => { adjustSize(); }); + useEffect(() => { + if (!node.current) { + return; + } + + const observer = new ResizeObserver(adjustSize); + observer.observe(node.current); + + return () => { + observer.disconnect(); + }; + }, [adjustSize]); + const uniqId = useId(); const id = idProp ?? uniqId; diff --git a/polaris-react/src/components/DropZone/tests/DropZone.test.tsx b/polaris-react/src/components/DropZone/tests/DropZone.test.tsx index 6a25e998107..c631fe12302 100755 --- a/polaris-react/src/components/DropZone/tests/DropZone.test.tsx +++ b/polaris-react/src/components/DropZone/tests/DropZone.test.tsx @@ -10,6 +10,7 @@ import {Labelled} from '../../Labelled'; import {DropZone} from '../DropZone'; import type {DropZoneFileType} from '../DropZone'; import {DropZoneContext} from '../context'; +import styles from '../DropZone.module.css'; const files = [ { @@ -532,6 +533,46 @@ describe('', () => { }).not.toThrow(); }); }); + + describe('Iframe React portal bug fix', () => { + it('observes the resize event for the activator wrapper', () => { + const observe = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe, + unobserve: jest.fn(), + disconnect: jest.fn(), + })); + + const dropZone = mountWithApp(); + + expect(observe).toHaveBeenCalledWith( + dropZone + .findAll('div') + .filter((node) => + node.domNode?.getAttribute('class')?.includes(styles.DropZone), + )[0]?.domNode, + ); + }); + + it('disconnects the resize observer when component unmounts', () => { + const disconnect = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: jest.fn(), + unobserve: jest.fn(), + disconnect, + })); + + const dropZone = mountWithApp(); + + dropZone.unmount(); + + expect(disconnect).toHaveBeenCalled(); + }); + }); }); function createEvent(name: string, files: any) { diff --git a/polaris-react/src/components/EventListener/EventListener.tsx b/polaris-react/src/components/EventListener/EventListener.tsx index 00a2a2f91be..ecc3b372fcb 100644 --- a/polaris-react/src/components/EventListener/EventListener.tsx +++ b/polaris-react/src/components/EventListener/EventListener.tsx @@ -4,6 +4,7 @@ interface BaseEventProps { event: string; capture?: boolean; handler(event: Event): void; + window?: Window | null; } export interface EventListenerProps extends BaseEventProps { @@ -30,12 +31,19 @@ export class EventListener extends PureComponent { } private attachListener() { - const {event, handler, capture, passive} = this.props; + const {event, handler, capture, passive, window: customWindow} = this.props; + const window = customWindow || globalThis.window; window.addEventListener(event, handler, {capture, passive}); } private detachListener(prevProps?: BaseEventProps) { - const {event, handler, capture} = prevProps || this.props; + const { + event, + handler, + capture, + window: customWindow, + } = prevProps || this.props; + const window = customWindow || globalThis.window; window.removeEventListener(event, handler, capture); } } diff --git a/polaris-react/src/components/EventListener/tests/EventListener.test.tsx b/polaris-react/src/components/EventListener/tests/EventListener.test.tsx index 88d4248f2d4..760e31844aa 100644 --- a/polaris-react/src/components/EventListener/tests/EventListener.test.tsx +++ b/polaris-react/src/components/EventListener/tests/EventListener.test.tsx @@ -35,6 +35,27 @@ describe('', () => { window.dispatchEvent(new Event('resize')); expect(spy).not.toHaveBeenCalled(); }); + + it('uses the custom window if provided', () => { + const myWindow = { + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + }; + + const wrapper = mountWithApp( + , + ); + + expect(myWindow.addEventListener).toHaveBeenCalled(); + + wrapper.unmount(); + + expect(myWindow.removeEventListener).toHaveBeenCalled(); + }); }); function noop() {} diff --git a/polaris-react/src/components/KeypressListener/KeypressListener.tsx b/polaris-react/src/components/KeypressListener/KeypressListener.tsx index d9a24e965a3..9e99156383d 100644 --- a/polaris-react/src/components/KeypressListener/KeypressListener.tsx +++ b/polaris-react/src/components/KeypressListener/KeypressListener.tsx @@ -7,6 +7,7 @@ export interface NonMutuallyExclusiveProps { keyCode: Key; handler(event: KeyboardEvent): void; keyEvent?: KeyEvent; + document?: Document; } export type KeypressListenerProps = NonMutuallyExclusiveProps & @@ -23,6 +24,7 @@ export function KeypressListener({ keyEvent = 'keyup', options, useCapture, + document: ownerDocument = globalThis.document, }: KeypressListenerProps) { const tracked = useRef({handler, keyCode}); @@ -38,15 +40,19 @@ export function KeypressListener({ }, []); useEffect(() => { - document.addEventListener(keyEvent, handleKeyEvent, useCapture || options); + ownerDocument.addEventListener( + keyEvent, + handleKeyEvent, + useCapture || options, + ); return () => { - document.removeEventListener( + ownerDocument.removeEventListener( keyEvent, handleKeyEvent, useCapture || options, ); }; - }, [keyEvent, handleKeyEvent, useCapture, options]); + }, [keyEvent, handleKeyEvent, useCapture, options, ownerDocument]); return null; } diff --git a/polaris-react/src/components/KeypressListener/tests/KeypressListener.test.tsx b/polaris-react/src/components/KeypressListener/tests/KeypressListener.test.tsx index d759e60f0e4..5574fd1269f 100644 --- a/polaris-react/src/components/KeypressListener/tests/KeypressListener.test.tsx +++ b/polaris-react/src/components/KeypressListener/tests/KeypressListener.test.tsx @@ -83,6 +83,27 @@ describe('', () => { expect(listenerOptionsMap.keyup).toStrictEqual(eventOptions); }); + + it('uses the custom document if provided', () => { + const myDocument = { + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + }; + + const wrapper = mountWithApp( + , + ); + + expect(myDocument.addEventListener).toHaveBeenCalled(); + + wrapper.unmount(); + + expect(myDocument.removeEventListener).toHaveBeenCalled(); + }); }); function noop() {} diff --git a/polaris-react/src/components/Popover/Popover.tsx b/polaris-react/src/components/Popover/Popover.tsx index 58aeb3c68d2..9c8d8761cf8 100644 --- a/polaris-react/src/components/Popover/Popover.tsx +++ b/polaris-react/src/components/Popover/Popover.tsx @@ -110,8 +110,8 @@ const PopoverComponent = forwardRef( }, ref, ) { + const [isDisplayed, setIsDisplay] = useState(false); const [activatorNode, setActivatorNode] = useState(); - const overlayRef = useRef(null); const activatorContainer = useRef(null); @@ -127,7 +127,6 @@ const PopoverComponent = forwardRef( if (activatorContainer.current == null || preventFocusOnClose) { return; } - if (source === PopoverCloseSource.FocusOut && activatorNode) { const focusableActivator = findFirstFocusableNodeIncludingDisabled(activatorNode) || @@ -191,6 +190,42 @@ const PopoverComponent = forwardRef( }); }, [id, active, ariaHaspopup]); + useEffect(() => { + function setDisplayState() { + /** + * This is a workaround to prevent rendering the Popover when the content is moved into + * a React portal that hasn't been rendered. We don't want to render the Popover in this + * case because the auto-focus logic will break. We wait until the activatorContainer is + * displayed, which is when it has an offsetParent, or if the activatorContainer is the + * body, if it has a clientWidth bigger than 0. + * See: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetParent + */ + + setIsDisplay( + Boolean( + activatorContainer.current && + (activatorContainer.current.offsetParent !== null || + (activatorContainer.current === + activatorContainer.current.ownerDocument.body && + activatorContainer.current.clientWidth > 0)), + ), + ); + } + + if (!activatorContainer.current) { + return; + } + + const observer = new ResizeObserver(setDisplayState); + observer.observe(activatorContainer.current); + + setDisplayState(); + + return () => { + observer.disconnect(); + }; + }, []); + useEffect(() => { if (!activatorNode && activatorContainer.current) { setActivatorNode( @@ -217,23 +252,24 @@ const PopoverComponent = forwardRef( setAccessibilityAttributes(); }, [activatorNode, setAccessibilityAttributes]); - const portal = activatorNode ? ( - - - {children} - - - ) : null; + const portal = + activatorNode && isDisplayed ? ( + + + {children} + + + ) : null; return ( diff --git a/polaris-react/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx b/polaris-react/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx index 88971157e7f..6007fd7562b 100644 --- a/polaris-react/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx +++ b/polaris-react/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx @@ -59,6 +59,7 @@ export interface PopoverOverlayProps { interface State { transitionStatus: TransitionStatus; + window?: Window | null; } export class PopoverOverlay extends PureComponent { @@ -74,6 +75,7 @@ export class PopoverOverlay extends PureComponent { private contentNode = createRef(); private enteringTimer?: number; private overlayRef: React.RefObject; + private observer?: ResizeObserver; constructor(props: PopoverOverlayProps) { super(props); @@ -97,6 +99,22 @@ export class PopoverOverlay extends PureComponent { this.changeTransitionStatus(TransitionStatus.Entered); } + + this.observer = new ResizeObserver(() => { + this.setState({ + /** + * This is a workaround to enable event listeners to be + * re-attached when moving from one document to another + * when using a React portal across iframes. + * Using a resize observer works because when the clientWidth + * will go from 0 to the real width after the activator + * gets rendered in its new place. + */ + window: this.props.activator.ownerDocument.defaultView, + }); + }); + + this.observer.observe(this.props.activator); } componentDidUpdate(oldProps: PopoverOverlayProps) { @@ -116,10 +134,16 @@ export class PopoverOverlay extends PureComponent { this.clearTransitionTimeout(); this.setState({transitionStatus: TransitionStatus.Exited}); } + + if (this.props.activator !== oldProps.activator) { + this.observer?.unobserve(oldProps.activator); + this.observer?.observe(this.props.activator); + } } componentWillUnmount() { this.clearTransitionTimeout(); + this.observer?.disconnect(); } render() { @@ -232,11 +256,25 @@ export class PopoverOverlay extends PureComponent { fluidContent && styles['Content-fluidContent'], ); + const {window} = this.state; + return (
- - - + + +
', () => { let rafSpy: jest.SpyInstance; @@ -596,6 +598,187 @@ describe('', () => { expect(overlayRef).toHaveProperty('current.forceUpdatePosition'); }); }); + + describe('Iframe React portal bug fix', () => { + it('observes the resize event for the activator', () => { + const observe = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe, + unobserve: jest.fn(), + disconnect: jest.fn(), + })); + + mountWithApp( + + {children} + , + ); + + expect(observe).toHaveBeenCalledWith(activator); + }); + + it('disconnects the resize observer when component unmounts', () => { + const disconnect = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: jest.fn(), + unobserve: jest.fn(), + disconnect, + })); + + const popoverOverlay = mountWithApp( + + {children} + , + ); + + popoverOverlay.unmount(); + + expect(disconnect).toHaveBeenCalled(); + }); + + it('updates the observer when activator updates', () => { + const observe = jest.fn(); + const unobserve = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe, + unobserve, + disconnect: jest.fn(), + })); + + const popoverOverlay = mountWithApp( + + {children} + , + ); + + const newActivator = document.createElement('button'); + + popoverOverlay.act(() => { + popoverOverlay.setProps({activator: newActivator}); + }); + + expect(unobserve).toHaveBeenCalledWith(activator); + expect(observe).toHaveBeenCalledWith(newActivator); + }); + + it('passes the activator.ownerDocument.defaultView as the window for the click and touchstart event listeners after resize observer fires', () => { + let resizeCallback: () => void; + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation((callback) => { + resizeCallback = callback; + return { + observe: jest.fn(), + unobserve: jest.fn(), + disconnect: jest.fn(), + }; + }); + + const popoverOverlay = mountWithApp( + + {children} + , + ); + + popoverOverlay.act(() => resizeCallback()); + + popoverOverlay.forceUpdate(); + + // We can't assert using `toContainReactComponent` because the matcher blows up trying to assert on window as an argument + expect( + popoverOverlay + .find(EventListener, { + event: 'click', + })! + .prop('window'), + ).toStrictEqual(activator.ownerDocument.defaultView); + + expect( + popoverOverlay + .find(EventListener, { + event: 'touchstart', + })! + .prop('window'), + ).toStrictEqual(activator.ownerDocument.defaultView); + }); + + it('passes the activator.ownerDocument as the document for the escape key press listener after ResizeObserver fires', () => { + let resizeCallback: () => void; + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation((callback) => { + resizeCallback = callback; + return { + observe: jest.fn(), + unobserve: jest.fn(), + disconnect: jest.fn(), + }; + }); + + const popoverOverlay = mountWithApp( + + {children} + , + ); + + popoverOverlay.act(() => resizeCallback()); + + popoverOverlay.forceUpdate(); + + // We can't assert using `toContainReactComponent` because the matcher blows up trying to assert on window as an argument + expect( + popoverOverlay + .find(KeypressListener, { + keyCode: Key.Escape, + })! + .prop('document'), + ).toStrictEqual(activator.ownerDocument); + + expect( + popoverOverlay + .find(EventListener, { + event: 'touchstart', + })! + .prop('window'), + ).toStrictEqual(activator.ownerDocument.defaultView); + }); + }); }); function noop() {} diff --git a/polaris-react/src/components/Popover/tests/Popover.test.tsx b/polaris-react/src/components/Popover/tests/Popover.test.tsx index 8c0ed16402c..d57db7b674e 100644 --- a/polaris-react/src/components/Popover/tests/Popover.test.tsx +++ b/polaris-react/src/components/Popover/tests/Popover.test.tsx @@ -523,6 +523,63 @@ describe('', () => { }); }); }); + + describe('Iframe React portal bug fix', () => { + it('does not render a positionedOverlay when activator node does not have an offsetParent even when active', () => { + // Replace patch for offsetParent for jsdom + Object.defineProperty(HTMLElement.prototype, 'offsetParent', { + get() { + return null; + }, + }); + + const popover = mountWithApp( + Activator
} onClose={spy} />, + ); + expect(popover).not.toContainReactComponent(PositionedOverlay); + }); + + it('observes the resize event for the activator wrapper', () => { + const observe = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe, + unobserve: jest.fn(), + disconnect: jest.fn(), + })); + + const popover = mountWithApp( + Activator
} + activatorWrapper="span" + onClose={spy} + />, + ); + + expect(observe).toHaveBeenCalledWith(popover.find('span')?.domNode); + }); + + it('disconnects the resize observer when component unmounts', () => { + const disconnect = jest.fn(); + + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: jest.fn(), + unobserve: jest.fn(), + disconnect, + })); + + const popover = mountWithApp( + Activator
} onClose={spy} />, + ); + + popover.unmount(); + + expect(disconnect).toHaveBeenCalled(); + }); + }); }); function noop() {} diff --git a/polaris-react/src/components/PositionedOverlay/PositionedOverlay.tsx b/polaris-react/src/components/PositionedOverlay/PositionedOverlay.tsx index 0c8c0569fdf..3c212784ca6 100644 --- a/polaris-react/src/components/PositionedOverlay/PositionedOverlay.tsx +++ b/polaris-react/src/components/PositionedOverlay/PositionedOverlay.tsx @@ -154,7 +154,11 @@ export class PositionedOverlay extends PureComponent< return (
- + {render(this.overlayDetails())}
); @@ -258,6 +262,8 @@ export class PositionedOverlay extends PureComponent< preferInputActivator = true, } = this.props; + const document = activator.ownerDocument; + const preferredActivator = preferInputActivator ? activator.querySelector('input') || activator : activator; @@ -288,13 +294,16 @@ export class PositionedOverlay extends PureComponent< topBarOffset = topBarElement.clientHeight; } - const overlayMargins = - this.overlay.firstElementChild && - this.overlay.firstChild instanceof HTMLElement - ? getMarginsForNode(this.overlay.firstElementChild as HTMLElement) - : {activator: 0, container: 0, horizontal: 0}; + let overlayMargins = {activator: 0, container: 0, horizontal: 0}; + + if (this.overlay.firstElementChild) { + const nodeMargins = getMarginsForNode( + this.overlay.firstElementChild as HTMLElement, + ); + overlayMargins = nodeMargins; + } - const containerRect = windowRect(); + const containerRect = windowRect(activator); const zIndexForLayer = getZIndexForLayerFromNode(activator); const zIndex = zIndexForLayer == null ? zIndexForLayer : zIndexForLayer + 1; @@ -341,7 +350,10 @@ export class PositionedOverlay extends PureComponent< onScrollOut != null && rectIsOutsideOfRect( activatorRect, - intersectionWithViewport(scrollableContainerRect), + intersectionWithViewport( + scrollableContainerRect, + containerRect, + ), ), zIndex, chevronOffset, @@ -358,6 +370,8 @@ export class PositionedOverlay extends PureComponent< } function getMarginsForNode(node: HTMLElement) { + // Accounts for when the node is moved between documents + const window = node.ownerDocument.defaultView || globalThis.window; const nodeStyles = window.getComputedStyle(node); return { activator: parseFloat(nodeStyles.marginTop || '0'), @@ -367,14 +381,14 @@ function getMarginsForNode(node: HTMLElement) { } function getZIndexForLayerFromNode(node: HTMLElement) { - const layerNode = node.closest(layer.selector) || document.body; + const layerNode = node.closest(layer.selector) || node.ownerDocument.body; const zIndex = - layerNode === document.body + layerNode === node.ownerDocument.body ? 'auto' : parseInt(window.getComputedStyle(layerNode).zIndex || '0', 10); return zIndex === 'auto' || isNaN(zIndex) ? null : zIndex; } function isDocument(node: HTMLElement | Document): node is Document { - return node === document; + return node.ownerDocument === null; } diff --git a/polaris-react/src/components/PositionedOverlay/utilities/math.ts b/polaris-react/src/components/PositionedOverlay/utilities/math.ts index cdce1f82830..e23f9c3812b 100644 --- a/polaris-react/src/components/PositionedOverlay/utilities/math.ts +++ b/polaris-react/src/components/PositionedOverlay/utilities/math.ts @@ -172,7 +172,10 @@ export function intersectionWithViewport( }); } -export function windowRect() { +export function windowRect(node?: HTMLElement) { + const document = node?.ownerDocument || globalThis.document; + const window = document.defaultView || globalThis.window; + return new Rect({ top: window.scrollY, left: window.scrollX, diff --git a/polaris-react/src/components/Tooltip/Tooltip.tsx b/polaris-react/src/components/Tooltip/Tooltip.tsx index ab64dae2618..d46a29bea40 100644 --- a/polaris-react/src/components/Tooltip/Tooltip.tsx +++ b/polaris-react/src/components/Tooltip/Tooltip.tsx @@ -236,8 +236,9 @@ export function Tooltip({ return; } - node.firstElementChild instanceof HTMLElement && - setActivatorNode(node.firstElementChild); + if (node.firstElementChild) { + setActivatorNode(node.firstElementChild as HTMLElement); + } activatorContainerRef.current = node; } diff --git a/polaris-react/src/utilities/geometry.ts b/polaris-react/src/utilities/geometry.ts index 9dc77ce17d9..9653d268909 100644 --- a/polaris-react/src/utilities/geometry.ts +++ b/polaris-react/src/utilities/geometry.ts @@ -38,19 +38,25 @@ export class Rect { export function getRectForNode( node: Element | React.ReactNode | Window | Document, ): Rect { - if (!(node instanceof Element)) { + /** + * NOTE: We cannot do node instanceof Element because it will fail when inside of an iframe. + * Technically we can do `node instanceof node.ownerDocument.defaultView.Element`but this will + * fail when node isn't an Element. We might as well try to run `getBoundingClientRect` and then + * have a fallback for when that breaks. + */ + try { + const rect = (node as Element).getBoundingClientRect(); + + return new Rect({ + top: rect.top, + left: rect.left, + width: rect.width, + height: rect.height, + }); + } catch (_) { return new Rect({ width: window.innerWidth, height: window.innerHeight, }); } - - const rect = node.getBoundingClientRect(); - - return new Rect({ - top: rect.top, - left: rect.left, - width: rect.width, - height: rect.height, - }); } diff --git a/polaris-react/src/utilities/is-element-in-viewport.ts b/polaris-react/src/utilities/is-element-in-viewport.ts index b54ae112086..5b6527cc7ff 100644 --- a/polaris-react/src/utilities/is-element-in-viewport.ts +++ b/polaris-react/src/utilities/is-element-in-viewport.ts @@ -1,5 +1,6 @@ export function isElementInViewport(element: Element) { const {top, left, bottom, right} = element.getBoundingClientRect(); + const window = element.ownerDocument.defaultView || globalThis.window; return ( top >= 0 && diff --git a/polaris-react/tests/setup/environment.ts b/polaris-react/tests/setup/environment.ts index 4a3c0030ae0..62c9cc8c671 100644 --- a/polaris-react/tests/setup/environment.ts +++ b/polaris-react/tests/setup/environment.ts @@ -47,3 +47,12 @@ console.warn = (...args: any[]) => { originalConsoleWarn(...args); }; + +if (typeof HTMLElement !== 'undefined') { + // Patch for offsetParent for jsdom, needed for Popover + Object.defineProperty(HTMLElement.prototype, 'offsetParent', { + get() { + return this.parentNode; + }, + }); +} diff --git a/polaris-react/tests/setup/tests.ts b/polaris-react/tests/setup/tests.ts index 96328e9fd29..eba7784a977 100644 --- a/polaris-react/tests/setup/tests.ts +++ b/polaris-react/tests/setup/tests.ts @@ -11,6 +11,13 @@ if (typeof window !== 'undefined' && !matchMedia.isMocked()) { // eslint-disable-next-line jest/require-top-level-describe beforeEach(() => { + // eslint-disable-next-line jest/prefer-spy-on + global.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: jest.fn(), + unobserve: jest.fn(), + disconnect: jest.fn(), + })); + if (typeof window !== 'undefined' && !matchMedia.isMocked()) { matchMedia.mock(); }