fix(menu-surface): Hoist menu-surface via a portal (#500,#628) (#693)

Author: Ben McKernan

package.json

@@ -18,7 +18,7 @@
     "test:watch": "karma start karma.local.js --auto-watch",
     "test:unit": "npm run clean && cross-env NODE_ENV=test karma start karma.local.js --single-run",
     "test:unit-ci": "karma start karma.ci.js --single-run",
-    "test:image-diff": "MDC_COMMIT_HASH=$(git rev-parse --short HEAD) MDC_BRANCH_NAME=$(git rev-parse --abbrev-ref HEAD) mocha --require ts-node/register --require babel-core/register --ui tdd --timeout 30000 test/screenshot/diff-suite.tsx",
+    "test:image-diff": "MDC_COMMIT_HASH=$(git rev-parse --short HEAD) MDC_BRANCH_NAME=$(git rev-parse --abbrev-ref HEAD) mocha --require ts-node/register --require babel-core/register --ui tdd --timeout 60000 test/screenshot/diff-suite.tsx",
     "test:screenshots": "docker run -it --rm --cap-add=SYS_ADMIN -e MDC_GCLOUD_SERVICE_ACCOUNT_KEY=\"${MDC_GCLOUD_SERVICE_ACCOUNT_KEY}\" mdcreact/screenshots /bin/sh -c 'git checkout .; git checkout master; git pull; npm i; /home/pptruser/material-components-web-react/test/screenshot/start.sh; sleep 40s; npm run test:image-diff'",
     "upload:screenshots": "node ./test/screenshot/upload-screenshots.js"
   },

packages/menu-surface/index.tsx

@@ -21,6 +21,7 @@
 // THE SOFTWARE.
 
 import * as React from 'react';
+import * as ReactDOM from 'react-dom';
 import * as classnames from 'classnames';
 // @ts-ignore no .d.ts file
 import {MDCMenuSurfaceFoundation, MDCMenuSurfaceAdapter, Corner} from '@material/menu-surface/dist/mdc.menuSurface';
@@ -56,6 +57,7 @@ export interface MenuSurfaceState {
   styleTop?: number;
   styleBottom?: number;
   classList: Set<string>;
+  mounted: boolean;
 };
 
 interface Position {
@@ -83,6 +85,7 @@ class MenuSurface extends React.Component<MenuSurfaceProps, MenuSurfaceState> {
     styleTop: undefined,
     styleBottom: undefined,
     classList: new Set(),
+    mounted: false,
   };
 
   static defaultProps: Partial<MenuSurfaceProps> = {
@@ -104,7 +107,6 @@ class MenuSurface extends React.Component<MenuSurfaceProps, MenuSurfaceState> {
       anchorMargin,
       coordinates,
       fixed,
-      open,
       quickOpen,
     } = this.props;
     this.handleWindowClick = (evt) => this.foundation.handleBodyClick(evt);
@@ -114,7 +116,10 @@ class MenuSurface extends React.Component<MenuSurfaceProps, MenuSurfaceState> {
       window.removeEventListener('click', this.handleWindowClick!);
     this.foundation = new MDCMenuSurfaceFoundation(this.adapter);
     this.foundation.init();
-    this.hoistToBody();
+    // this deviates from the mdc web version.
+    // here we force the menu to hoist, and require either
+    // this.props.(x,y) or this.props.anchorElement.
+    this.foundation.setIsHoisted(true);
     this.foundation.setFixedPosition(fixed);
     if (coordinates) {
       this.setCoordinates();
@@ -128,13 +133,14 @@ class MenuSurface extends React.Component<MenuSurfaceProps, MenuSurfaceState> {
     if (quickOpen) {
       this.foundation.setQuickOpen(quickOpen);
     }
-    if (open) {
-      this.open_();
-    }
+    this.setState({mounted: true});
   }
 
-  componentDidUpdate(prevProps: MenuSurfaceProps) {
-    if (this.props.open !== prevProps.open) {
+  componentDidUpdate(prevProps: MenuSurfaceProps, prevState: MenuSurfaceState) {
+    // if this.props.open was true for the initial render
+    // then it will not be changed but the mounted state will be,
+    // so this.open_() should also be called in that case.
+    if (this.props.open !== prevProps.open || (this.props.open && this.state.mounted !== prevState.mounted)) {
       this.open_();
     }
     if (this.props.coordinates !== prevProps.coordinates) {
@@ -160,19 +166,6 @@ class MenuSurface extends React.Component<MenuSurfaceProps, MenuSurfaceState> {
     }
   }
 
-  private hoistToBody(): void {
-    // this deviates from the mdc web version.
-    // here we force the menu to hoist, and require either
-    // this.props.(x,y) or this.props.anchorElement.
-    const menuSurfaceElement = this.menuSurfaceElement.current;
-    if (!menuSurfaceElement) return;
-    if (!menuSurfaceElement.parentElement) return;
-    document.body.appendChild(
-      menuSurfaceElement.parentElement.removeChild(menuSurfaceElement)
-    );
-    this.foundation.setIsHoisted(true);
-  }
-
   private setCoordinates(): void {
     if (!this.props.coordinates) return;
     const {x, y} = this.props.coordinates;
@@ -359,7 +352,8 @@ class MenuSurface extends React.Component<MenuSurfaceProps, MenuSurfaceState> {
       children,
       ...otherProps
     } = this.props;
-    return (
+    if (!this.state.mounted) return null;
+    return ReactDOM.createPortal(
       <div
         className={this.classes}
         onKeyDown={this.handleKeydown}
@@ -368,7 +362,8 @@ class MenuSurface extends React.Component<MenuSurfaceProps, MenuSurfaceState> {
         {...otherProps}
       >
         {children}
-      </div>
+      </div>,
+      document.body
     );
   }
 }

packages/menu-surface/package.json

@@ -19,7 +19,8 @@
   "dependencies": {
     "@material/menu-surface": "^0.41.0",
     "classnames": "^2.2.5",
-    "react": "^16.4.2"
+    "react": "^16.4.2",
+    "react-dom": "^16.4.2"
   },
   "publishConfig": {
     "access": "public"

packages/webpack.config.js

@@ -109,6 +109,7 @@ function getJavaScriptWebpackConfig() {
       externals: Object.assign(
         {
           'react': 'react',
+          'react-dom': 'react-dom',
           'classnames': 'classnames',
           'prop-types': 'prop-types',
           '@material/textfield/constants': `@material/textfield/constants.js`,

test/screenshot/screenshot.tsx

@@ -233,6 +233,7 @@ export default class Screenshot {
     const page = await browser.newPage();
     await page.goto(`http://localhost:8080/#/${this.urlPath_}`, {
       waitUntil: ['networkidle2'],
+      timeout: 600000,
     });
     // await page.waitForSelector('#screenshot-test-app');
     const imageBuffer = await page.screenshot({fullPage: true});