Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BackDrop component does not render children #17084

Closed
2 tasks done
dominictwlee opened this issue Aug 21, 2019 · 2 comments · Fixed by #17115
Closed
2 tasks done

BackDrop component does not render children #17084

dominictwlee opened this issue Aug 21, 2019 · 2 comments · Fixed by #17115
Labels
component: backdrop This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request

Comments

@dominictwlee
Copy link
Contributor

dominictwlee commented Aug 21, 2019

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Backdrop does not render children content.

Expected Behavior 🤔

Backdrop should render children.

Steps to Reproduce 🕹

https://codesandbox.io/s/create-react-app-t695r

The issue is with the implementation of Backdrop
https://github.com/mui-org/material-ui/blob/aa4e2dbf3ad98355d2e43cdd281f0ea2c2204046/packages/material-ui/src/Backdrop/Backdrop.js#L32-L47

children gets spread into Fade, but the nested div will just overwrite that.

I recommend doing this instead:

const Backdrop = React.forwardRef(function Backdrop(props, ref) {
  const { classes, className, invisible = false, open, transitionDuration, children, ...other } = props;

  return (
    <Fade in={open} timeout={transitionDuration} {...other}>
      <div
        data-mui-test="Backdrop"
        className={clsx(
          classes.root,
          {
            [classes.invisible]: invisible,
          },
          className,
        )}
        aria-hidden
        ref={ref}
      >
        {children}
      </ div>
    </Fade>
  );
});

Context 🔦

I'm trying to create a full screen loader with a backdrop and the loading indicator doesn't get rendered because the Backdrop component only renders the div.

Your Environment 🌎

Tech Version
Material-UI v4.3.2
React ^16.8.0
Browser Firefox 68.0.2
TypeScript 3.5.3
@oliviertassinari oliviertassinari added component: backdrop This is the name of the generic UI component, not the React module! new feature New feature or request labels Aug 21, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 21, 2019

I'm trying to create a full screen loader with a backdrop and the loading indicator doesn't get rendered because the Backdrop component only renders the div.

@dominictwlee Interesting, I like the idea. What do you think of this diff?

diff --git a/packages/material-ui/src/Backdrop/Backdrop.js b/packages/material-ui/src/Backdrop/Backdrop.js
index 7caba73eb..3b8aea3d0 100644
--- a/packages/material-ui/src/Backdrop/Backdrop.js
+++ b/packages/material-ui/src/Backdrop/Backdrop.js
@@ -9,6 +9,9 @@ export const styles = {
   root: {
     zIndex: -1,
     position: 'fixed',
+    display: 'flex',
+    alignItems: 'center',
+    justifyContent: 'center',
     right: 0,
     bottom: 0,
     top: 0,
@@ -26,7 +29,15 @@ export const styles = {
 };

 const Backdrop = React.forwardRef(function Backdrop(props, ref) {
-  const { classes, className, invisible = false, open, transitionDuration, ...other } = props;
+  const {
+    children,
+    classes,
+    className,
+    invisible = false,
+    open,
+    transitionDuration,
+    ...other
+  } = props;

   return (
     <Fade in={open} timeout={transitionDuration} {...other}>
@@ -41,7 +52,9 @@ const Backdrop = React.forwardRef(function Backdrop(props, ref) {
         )}
         aria-hidden
         ref={ref}
-      />
+      >
+        {children}
+      </div>
     </Fade>
   );
 });

Then, we should be able to do

import React from 'react';
import Backdrop from '@material-ui/core/Backdrop';
import CircularProgress from '@material-ui/core/CircularProgress';

export default () => {
  return <Backdrop open><CircularProgress style={{ color: 'white' }} /></Backdrop>
}

Capture d’écran 2019-08-21 à 13 46 18

Do you want to submit a pull request? :)

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Aug 21, 2019
@dominictwlee
Copy link
Contributor Author

Yeah sure, I'll get on it, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: backdrop This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants