[Dialog] Flatten DialogTitle DOM structure

[eps1lon]

Summary 💡

DialogTitle should be flatter

Examples 🌈

• https://codesandbox.io/s/old-pond-bpjb9

Motivation 🔦

• Aligning items of the title

  <DialogTitle style={ display: 'flex', alignItems: 'center' }>
    <SomeIcon />
    My Title
  </DialogTitle>

• fewer DOM elements -> fewer brittle element selectors

It is possible but requires targetting nested elements. disableTypography is not helpful since then we wouldn't render a heading element.

We could leverage aria but this would go against rule 1 of aria (don't use aria): <DialogTitle disableTypography role="heading" aria-level="2" className={variantH2Styles} />

[oliviertassinari]

Always in favor of removing DOM nodes when possible.
In this case, would it still allow developers to use a close button in the header? would it still allow a border-bottom on the element? I imagine it would in both cases.

[eps1lon]

In this case, would it still allow developers to use a close button in the header? would it still allow a border-bottom on the element? I imagine it would in both cases.

I'd be interested to see both use cases in the current implementation.

Generally it's always possible to add additional DOM elements if necessary. Removing is not (or at least a lot harder).

[oliviertassinari]

I believe this demo illustrates the constraints I have mentioned: https://material-ui.com/components/dialogs/#customized-dialogs.

[oliviertassinari]

We recently had a tweet on this matter: https://twitter.com/optimistavf/status/1300343255968747521. If we don't move in the direction proposed in the issue's description, I think that we should apply the List's tradeoff:

https://github.com/mui-org/material-ui/blob/02b722a249d1afe001e01827f6197c4b223ea0ce/packages/material-ui/src/ListItemText/ListItemText.js#L49

• auto enable disableTypography: avoid useless nested DOM structure
• have a TypographyProps to spread: allow configuration at the global theme level.

[vicasas]

The way here is to remove disabledTypography?. I have been working on this solution

diff --git a/packages/material-ui/src/DialogTitle/DialogTitle.js b/packages/material-ui/src/DialogTitle/DialogTitle.js
index 910d45f710..cc774fa724 100644
--- a/packages/material-ui/src/DialogTitle/DialogTitle.js
+++ b/packages/material-ui/src/DialogTitle/DialogTitle.js
@@ -40,10 +40,19 @@ const DialogTitle = React.forwardRef(function DialogTitle(inProps, ref) {
     name: 'MuiDialogTitle',
   });
 
-  const { children, className, disableTypography = false, ...other } = props;
-  const styleProps = { ...props, disableTypography };
+  const { children: childrenProp, className, typographyProps, ...other } = props;
+  const styleProps = { ...props };
   const classes = useUtilityClasses(styleProps);
 
+  let children = childrenProp;
+  if (typeof childrenProp.type === 'undefined') {
+    children = (
+      <Typography component="h2" variant="h6" {...typographyProps}>
+        {childrenProp}
+      </Typography>
+    );
+  }
+
   return (
     <DialogTitleRoot
       className={clsx(classes.root, className)}
@@ -51,13 +60,7 @@ const DialogTitle = React.forwardRef(function DialogTitle(inProps, ref) {
       ref={ref}
       {...other}
     >
-      {disableTypography ? (
-        children
-      ) : (
-        <Typography component="h2" variant="h6">
-          {children}
-        </Typography>
-      )}
+      {children}
     </DialogTitleRoot>
   );
 });

[oliviertassinari]

@vicasas maybe we should test for a children type string directly instead of no type? But otherwise, it looks like an option with potential. The biggest challenge of this tradeoff will probably be documentation. Will developers be able to understand what's going on? To some extent, we have already used the pattern in the past when a developer provides a Typography.

We have recently worked on a similar problem in #25883, maybe we should apply the same pattern in all the other cases?

[oliviertassinari]

I have tried to consider all our options. I could find the following:

• Option 1: We keep a two DOM node structure but we break down the API. We end up with:

<Dialog>
  <DialogTitle>
    <Typography variant="h6" component="h2">
      Set backup account
    </Typography>
  </DialogTitle>
  <DialogContent>

• Option 2: We keep a two DOM node structure and allow developers to append a new typography component, without nesting, it's basically [Dialog] Flatten DialogTitle DOM structure #19696 (comment).

• Option 3: We flatten the header. It's the direction explored in [Dialog] Flatten DialogTitle DOM structure, remove disableTypography #26323.

I would personally vote for 3. It would solve #26386 at its root.

[vicasas]

With option 3, if you want to do something custom, should you stop using DialogTitle? If not used, the root element styles would be lost and it would be left to the developer to add minimal styles (example padding)