From 0e22420dbef9a108a986fe296c45d8f2eb551a8d Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Wed, 24 Aug 2022 10:23:49 -0400 Subject: [PATCH 1/2] [core] fix: partially revert #5493, create Portal2 --- packages/core/karma.conf.js | 6 + packages/core/src/components/portal/portal.md | 12 +- .../core/src/components/portal/portal.tsx | 112 ++++++------- .../core/src/components/portal/portal2.tsx | 152 ++++++++++++++++++ packages/core/test/portal/portalTests.tsx | 3 +- 5 files changed, 219 insertions(+), 66 deletions(-) create mode 100644 packages/core/src/components/portal/portal2.tsx diff --git a/packages/core/karma.conf.js b/packages/core/karma.conf.js index b7f412b12e..7a18c530f9 100644 --- a/packages/core/karma.conf.js +++ b/packages/core/karma.conf.js @@ -11,12 +11,18 @@ module.exports = function (config) { "src/common/abstractComponent*", "src/common/abstractPureComponent*", "src/compatibility/*", + // HACKHACK: for karma upgrade only "src/common/refs.ts", + // HACKHACK: need to add hotkeys v2 tests "src/components/hotkeys/hotkeysDialog2.tsx", "src/components/hotkeys/hotkeysTarget2.tsx", "src/context/hotkeys/hotkeysProvider.tsx", + + // HACKHACK: see https://github.com/palantir/blueprint/issues/5511 + "src/components/portal/portal2.tsx", + "src/context/portal/portalProvider.tsx", ]; const baseConfig = createKarmaConfig({ diff --git a/packages/core/src/components/portal/portal.md b/packages/core/src/components/portal/portal.md index 31ed761f9e..cf09abb3ff 100644 --- a/packages/core/src/components/portal/portal.md +++ b/packages/core/src/components/portal/portal.md @@ -9,16 +9,16 @@ For the most part, Portal is a thin wrapper around [`ReactDOM.createPortal`](htt @## React context (legacy) -
+

-Deprecated: use [PortalProvider](#core/context/portal-provider) +React legacy API

-This API is **deprecated since @blueprintjs/core v4.8.0** in favor of -[PortalProvider](#core/context/portal-provider), which uses the -[newer React context API](https://reactjs.org/docs/context.html). +This feature uses React's legacy context API. Support for the +[newer React context API](https://reactjs.org/docs/context.html) will be coming soon +in Blueprint v5.x.
@@ -27,12 +27,14 @@ To use them, supply a child context to a subtree that contains the Portals you w @interface PortalLegacyContext + @## Props diff --git a/packages/core/src/components/portal/portal.tsx b/packages/core/src/components/portal/portal.tsx index a1263f4182..6831ad3758 100644 --- a/packages/core/src/components/portal/portal.tsx +++ b/packages/core/src/components/portal/portal.tsx @@ -21,8 +21,6 @@ import * as Classes from "../../common/classes"; import { ValidationMap } from "../../common/context"; import * as Errors from "../../common/errors"; import { DISPLAYNAME_PREFIX, Props } from "../../common/props"; -import { PortalContext } from "../../context/portal/portalProvider"; -import { usePrevious } from "../../hooks/usePrevious"; // eslint-disable-next-line deprecation/deprecation export type PortalProps = IPortalProps; @@ -44,7 +42,11 @@ export interface IPortalProps extends Props { container?: HTMLElement; } -/** @deprecated use PortalProvider */ +export interface IPortalState { + hasMounted: boolean; +} + +/** @deprecated use PortalLegacyContext */ export type IPortalContext = PortalLegacyContext; export interface PortalLegacyContext { /** Additional CSS classes to add to all `Portal` elements in this React context. */ @@ -65,74 +67,64 @@ const REACT_CONTEXT_TYPES: ValidationMap = { * Use it when you need to circumvent DOM z-stacking (for dialogs, popovers, etc.). * Any class names passed to this element will be propagated to the new container element on document.body. */ -export function Portal(props: PortalProps, legacyContext: PortalLegacyContext = {}) { - const context = React.useContext(PortalContext); +export class Portal extends React.Component { + public static displayName = `${DISPLAYNAME_PREFIX}.Portal`; - const [hasMounted, setHasMounted] = React.useState(false); - const [portalElement, setPortalElement] = React.useState(); + public static contextTypes = REACT_CONTEXT_TYPES; - const createContainerElement = React.useCallback(() => { - const container = document.createElement("div"); - container.classList.add(Classes.PORTAL); - maybeAddClass(container.classList, props.className); // directly added to this portal element - maybeAddClass(container.classList, context.portalClassName); // added via PortalProvider context + public static defaultProps: Partial = { + container: typeof document !== "undefined" ? document.body : undefined, + }; - const { blueprintPortalClassName } = legacyContext; - if (blueprintPortalClassName != null && blueprintPortalClassName !== "") { - console.error(Errors.PORTAL_LEGACY_CONTEXT_API); - maybeAddClass(container.classList, blueprintPortalClassName); // added via legacy context - } + public context: PortalLegacyContext = {}; - return container; - }, [props.className, context.portalClassName]); + public state: IPortalState = { hasMounted: false }; + + private portalElement: HTMLElement | null = null; + + public render() { + // Only render `children` once this component has mounted in a browser environment, so they are + // immediately attached to the DOM tree and can do DOM things like measuring or `autoFocus`. + // See long comment on componentDidMount in https://reactjs.org/docs/portals.html#event-bubbling-through-portals + if (typeof document === "undefined" || !this.state.hasMounted || this.portalElement === null) { + return null; + } else { + return ReactDOM.createPortal(this.props.children, this.portalElement); + } + } - // create the container element & attach it to the DOM - React.useEffect(() => { - if (props.container == null) { + public componentDidMount() { + if (this.props.container == null) { return; } - const newPortalElement = createContainerElement(); - props.container.appendChild(newPortalElement); - setPortalElement(newPortalElement); - setHasMounted(true); - - return () => { - newPortalElement.remove(); - setHasMounted(false); - setPortalElement(undefined); - }; - }, [props.container, createContainerElement]); - - // wait until next successful render to invoke onChildrenMount callback - React.useEffect(() => { - if (hasMounted) { - props.onChildrenMount?.(); + this.portalElement = this.createContainerElement(); + this.props.container.appendChild(this.portalElement); + /* eslint-disable-next-line react/no-did-mount-set-state */ + this.setState({ hasMounted: true }, this.props.onChildrenMount); + } + + public componentDidUpdate(prevProps: PortalProps) { + // update className prop on portal DOM element + if (this.portalElement != null && prevProps.className !== this.props.className) { + maybeRemoveClass(this.portalElement.classList, prevProps.className); + maybeAddClass(this.portalElement.classList, this.props.className); } - }, [hasMounted, props.onChildrenMount]); - - // update className prop on portal DOM element when props change - const prevClassName = usePrevious(props.className); - React.useEffect(() => { - if (portalElement != null) { - maybeRemoveClass(portalElement.classList, prevClassName); - maybeAddClass(portalElement.classList, props.className); + } + + public componentWillUnmount() { + this.portalElement?.remove(); + } + + private createContainerElement() { + const container = document.createElement("div"); + container.classList.add(Classes.PORTAL); + maybeAddClass(container.classList, this.props.className); + if (this.context != null) { + maybeAddClass(container.classList, this.context.blueprintPortalClassName); } - }, [props.className]); - - // Only render `children` once this component has mounted in a browser environment, so they are - // immediately attached to the DOM tree and can do DOM things like measuring or `autoFocus`. - // See long comment on componentDidMount in https://reactjs.org/docs/portals.html#event-bubbling-through-portals - if (typeof document === "undefined" || !hasMounted || portalElement == null) { - return null; - } else { - return ReactDOM.createPortal(props.children, portalElement); + return container; } } -Portal.defaultProps = { - container: typeof document !== "undefined" ? document.body : undefined, -}; -Portal.displayName = `${DISPLAYNAME_PREFIX}.Portal`; -Portal.contextTypes = REACT_CONTEXT_TYPES; function maybeRemoveClass(classList: DOMTokenList, className?: string) { if (className != null && className !== "") { diff --git a/packages/core/src/components/portal/portal2.tsx b/packages/core/src/components/portal/portal2.tsx new file mode 100644 index 0000000000..781aa7198d --- /dev/null +++ b/packages/core/src/components/portal/portal2.tsx @@ -0,0 +1,152 @@ +/* + * Copyright 2022 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @fileoverview This is the next version of , reimplemented as a function component. + * + * It supports both the newer React context API and the legacy context API. Support for the legacy context API + * will be removed in Blueprint v6.0. + * + * Portal2 is not currently used anywhere in Blueprint. We had to revert the change which updated the standard + * to use this implementation because of subtle breaks caused by interactions with the (long-deprecated) + * react-hot-loader library. To be safe, we've left Portal as a class component for now, and will promote this Portal2 + * implementation to be the standard Portal in Blueprint v5.0. + * + * @see https://github.com/palantir/blueprint/issues/5511 + */ + +import * as React from "react"; +import * as ReactDOM from "react-dom"; + +import * as Classes from "../../common/classes"; +import { ValidationMap } from "../../common/context"; +import * as Errors from "../../common/errors"; +import { DISPLAYNAME_PREFIX, Props } from "../../common/props"; +import { PortalContext } from "../../context/portal/portalProvider"; +import { usePrevious } from "../../hooks/usePrevious"; +import type { PortalLegacyContext } from "./portal"; + +export interface Portal2Props extends Props { + /** Contents to send through the portal. */ + children: React.ReactNode; + + /** + * Callback invoked when the children of this `Portal` have been added to the DOM. + */ + onChildrenMount?: () => void; + + /** + * The HTML element that children will be mounted to. + * + * @default document.body + */ + container?: HTMLElement; +} + +const REACT_CONTEXT_TYPES: ValidationMap = { + blueprintPortalClassName: (obj: PortalLegacyContext, key: keyof PortalLegacyContext) => { + if (obj[key] != null && typeof obj[key] !== "string") { + return new Error(Errors.PORTAL_CONTEXT_CLASS_NAME_STRING); + } + return undefined; + }, +}; + +/** + * This component detaches its contents and re-attaches them to document.body. + * Use it when you need to circumvent DOM z-stacking (for dialogs, popovers, etc.). + * Any class names passed to this element will be propagated to the new container element on document.body. + */ +export function Portal2(props: Portal2Props, legacyContext: PortalLegacyContext = {}) { + const context = React.useContext(PortalContext); + + const [hasMounted, setHasMounted] = React.useState(false); + const [portalElement, setPortalElement] = React.useState(); + + const createContainerElement = React.useCallback(() => { + const container = document.createElement("div"); + container.classList.add(Classes.PORTAL); + maybeAddClass(container.classList, props.className); // directly added to this portal element + maybeAddClass(container.classList, context.portalClassName); // added via PortalProvider context + + const { blueprintPortalClassName } = legacyContext; + if (blueprintPortalClassName != null && blueprintPortalClassName !== "") { + console.error(Errors.PORTAL_LEGACY_CONTEXT_API); + maybeAddClass(container.classList, blueprintPortalClassName); // added via legacy context + } + + return container; + }, [props.className, context.portalClassName]); + + // create the container element & attach it to the DOM + React.useEffect(() => { + if (props.container == null) { + return; + } + const newPortalElement = createContainerElement(); + props.container.appendChild(newPortalElement); + setPortalElement(newPortalElement); + setHasMounted(true); + + return () => { + newPortalElement.remove(); + setHasMounted(false); + setPortalElement(undefined); + }; + }, [props.container, createContainerElement]); + + // wait until next successful render to invoke onChildrenMount callback + React.useEffect(() => { + if (hasMounted) { + props.onChildrenMount?.(); + } + }, [hasMounted, props.onChildrenMount]); + + // update className prop on portal DOM element when props change + const prevClassName = usePrevious(props.className); + React.useEffect(() => { + if (portalElement != null) { + maybeRemoveClass(portalElement.classList, prevClassName); + maybeAddClass(portalElement.classList, props.className); + } + }, [props.className]); + + // Only render `children` once this component has mounted in a browser environment, so they are + // immediately attached to the DOM tree and can do DOM things like measuring or `autoFocus`. + // See long comment on componentDidMount in https://reactjs.org/docs/portals.html#event-bubbling-through-portals + if (typeof document === "undefined" || !hasMounted || portalElement == null) { + return null; + } else { + return ReactDOM.createPortal(props.children, portalElement); + } +} +Portal2.defaultProps = { + container: typeof document !== "undefined" ? document.body : undefined, +}; +Portal2.displayName = `${DISPLAYNAME_PREFIX}.Portal2`; +Portal2.contextTypes = REACT_CONTEXT_TYPES; + +function maybeRemoveClass(classList: DOMTokenList, className?: string) { + if (className != null && className !== "") { + classList.remove(...className.split(" ")); + } +} + +function maybeAddClass(classList: DOMTokenList, className?: string) { + if (className != null && className !== "") { + classList.add(...className.split(" ")); + } +} diff --git a/packages/core/test/portal/portalTests.tsx b/packages/core/test/portal/portalTests.tsx index 3acf0ef76d..18a635a210 100644 --- a/packages/core/test/portal/portalTests.tsx +++ b/packages/core/test/portal/portalTests.tsx @@ -96,7 +96,8 @@ describe("", () => { assert.isTrue(portalElement?.classList.contains(Classes.PORTAL)); }); - it("respects portalClassName on new context API", () => { + // HACKHACK, see https://github.com/palantir/blueprint/issues/5511 + it.skip("respects portalClassName on new context API", () => { const CLASS_TO_TEST = "bp-test-klass bp-other-class"; portal = mount( From ddf287bb50ae5df1926950b8f1df5b10cd0b8514 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Wed, 24 Aug 2022 10:50:58 -0400 Subject: [PATCH 2/2] minor docs fix --- packages/core/src/components/portal/portal.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/components/portal/portal.md b/packages/core/src/components/portal/portal.md index cf09abb3ff..2d5d935d13 100644 --- a/packages/core/src/components/portal/portal.md +++ b/packages/core/src/components/portal/portal.md @@ -9,7 +9,7 @@ For the most part, Portal is a thin wrapper around [`ReactDOM.createPortal`](htt @## React context (legacy) -
+

React legacy API @@ -32,10 +32,10 @@ To use them, supply a child context to a subtree that contains the Portals you w Portal supports the following options on its [React context](https://reactjs.org/docs/context.html) via [PortalProvider](#core/context/portal-provider). - -@interface PortalContextOptions --> + + @## Props The Portal component functions like a declarative `appendChild()`, or jQuery's @@ -45,7 +45,7 @@ child of the ``. Portal is used inside [Overlay](#core/components/overlay) to actually overlay the content on the application. -
+

A note about responsive layouts

For a single-page app, if the `` is styled with `width: 100%` and `height: 100%`, a `Portal`