From da7edf489c8209f9a227405d1a5d72d9a4e28ea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Fri, 30 Aug 2024 17:16:46 +0200 Subject: [PATCH 01/11] Remove disablePortal prop It was not used within the app anyway --- web/packages/design/src/Modal/Modal.jsx | 8 ---- web/packages/design/src/Modal/Portal.jsx | 44 ++++--------------- web/packages/design/src/Modal/Portal.test.tsx | 5 --- 3 files changed, 9 insertions(+), 48 deletions(-) diff --git a/web/packages/design/src/Modal/Modal.jsx b/web/packages/design/src/Modal/Modal.jsx index 35c3cf250b37b..dac2b18c2346d 100644 --- a/web/packages/design/src/Modal/Modal.jsx +++ b/web/packages/design/src/Modal/Modal.jsx @@ -174,7 +174,6 @@ export default class Modal extends React.Component { BackdropProps, children, container, - disablePortal, modalCss, hideBackdrop, open, @@ -191,7 +190,6 @@ export default class Modal extends React.Component { @@ -254,11 +252,6 @@ Modal.propTypes = { * If `true`, hitting escape will not fire any callback. */ disableEscapeKeyDown: PropTypes.bool, - /** - * Disable the portal behavior. - * The children stay within it's parent DOM hierarchy. - */ - disablePortal: PropTypes.bool, /** * If `true`, the modal will not restore focus to previously focused element once * modal is hidden. @@ -302,7 +295,6 @@ Modal.defaultProps = { disableBackdropClick: false, disableEnforceFocus: false, disableEscapeKeyDown: false, - disablePortal: false, disableRestoreFocus: false, hideBackdrop: false, }; diff --git a/web/packages/design/src/Modal/Portal.jsx b/web/packages/design/src/Modal/Portal.jsx index 1e11f0a295309..d2b2aee46ca94 100644 --- a/web/packages/design/src/Modal/Portal.jsx +++ b/web/packages/design/src/Modal/Portal.jsx @@ -30,27 +30,18 @@ class Portal extends React.Component { componentDidMount() { this.setMountNode(this.props.container); - // Only rerender if needed - if (!this.props.disablePortal) { - // Portal initializes the container and mounts it to the DOM during - // first render. No children are rendered at this time. - // ForceUpdate is called to render children elements inside - // the container after it gets mounted. - this.forceUpdate(); - } + // Portal initializes the container and mounts it to the DOM during + // first render. No children are rendered at this time. + // ForceUpdate is called to render children elements inside + // the container after it gets mounted. + this.forceUpdate(); } componentDidUpdate(prevProps) { - if ( - prevProps.container !== this.props.container || - prevProps.disablePortal !== this.props.disablePortal - ) { + if (prevProps.container !== this.props.container) { this.setMountNode(this.props.container); - // Only rerender if needed - if (!this.props.disablePortal) { - this.forceUpdate(); - } + this.forceUpdate(); } } @@ -59,11 +50,7 @@ class Portal extends React.Component { } setMountNode(container) { - if (this.props.disablePortal) { - this.mountNode = ReactDOM.findDOMNode(this).parentElement; - } else { - this.mountNode = getContainer(container, getOwnerDocument(this).body); - } + this.mountNode = getContainer(container, getOwnerDocument(this).body); } /** @@ -74,11 +61,7 @@ class Portal extends React.Component { }; render() { - const { children, disablePortal } = this.props; - - if (disablePortal) { - return children; - } + const { children } = this.props; return this.mountNode ? ReactDOM.createPortal(children, this.mountNode) @@ -98,15 +81,6 @@ Portal.propTypes = { * so it's simply `document.body` most of the time. */ container: PropTypes.oneOfType([PropTypes.object, PropTypes.func]), - /** - * Disable the portal behavior. - * The children stay within it's parent DOM hierarchy. - */ - disablePortal: PropTypes.bool, -}; - -Portal.defaultProps = { - disablePortal: false, }; function getContainer(container, defaultContainer) { diff --git a/web/packages/design/src/Modal/Portal.test.tsx b/web/packages/design/src/Modal/Portal.test.tsx index 5c2a0b0129185..ae3397331d4ca 100644 --- a/web/packages/design/src/Modal/Portal.test.tsx +++ b/web/packages/design/src/Modal/Portal.test.tsx @@ -37,11 +37,6 @@ describe('design/Modal/Portal', () => { expect(screen.queryByTestId('content')).not.toBeInTheDocument(); expect(customElement).toHaveTextContent('hello'); }); - - test('disable the portal behavior', () => { - const { container } = renderPortal({ disablePortal: true }); - expect(container).toContainElement(screen.getByTestId('content')); - }); }); function renderPortal(props) { From 731f1be08cc5582e1487be7fe9df81064cc7b563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Fri, 30 Aug 2024 17:33:07 +0200 Subject: [PATCH 02/11] Get rid of ownerDocument and ownerWindow The only situation where they'd be needed if we were handling some nodes between iframes, but we don't use