Material-ui: [LinearProgress][CircularProgress] Change from div to span

[Mangatt]

So far, both [LinearProgress] and [CircularProgess] are made of <div />s.

That makes it hard to use inside of any text (e.g. Intractive Integration Button, which can naturally be placed inside of <p /> or <span />), because you will get Warning: validateDOMNesting(...) in that case.

I think that best solution would be to change <div />s in those components to <span />s with display: block. It would probably won't event be considered breaking change, since nowhere in Docs is stated that those Components uses any particular HTML structure.

[Mangatt]

If that would be too big change, we can always expose component prop as in other Components.

[oliviertassinari]

@Mangatt Would you mind following our issue template? Do you have an example where this is problematic?

[Mangatt]

@oliviertassinari Sorry, I thought that it would be ok like that for issue as simple as this one. There you go:

• [X] This is a v1.x issue.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

One should be able to use [LinearProgress] and [CircularProgress] no matter where those components are placed in HTML tree.

Current Behavior

When you place [LinearProgress] or [CircularProgress] inside <p /> or any inline element, you'll get Warning: validateDOMNesting(...): <div> cannot appear as a descendant of <p>. in div (created by LinearProgress)

Steps to Reproduce

1. Open https://codesandbox.io/s/7zy0yk3nr0
2. Open console

Context

It is valid use case to show loading for <button />, <img /> or <input />, which can all appear inside paragraphs or inline elements. There are cases that does not allow to change whole parent structure to block elements.

Your Environment

| Tech | Version |
|--------------|---------|
| Material-UI | v1.4.3 |
| React | 16.4.2 |
| browser | Chrome 68 |

[oliviertassinari]

@Mangatt Thank you for following the issue template. As far as I can think of the issue, I fail to see a good reason to support p > LinearProgress or p > CircularProgress. Why don't you change the parent element?

[Mangatt]

Sometimes it's not possible to change parent structure, especially when you don't have immediate control over it - predefined templates, contenteditable or when you let user create content via wysiwyg.

[oliviertassinari]

@Mangatt I have added the waiting for users upvotes tag. I'm closing the issue as we are not sure people are looking for such behavior. So please upvote this issue if you are. We will prioritize our effort based on the number of upvotes.

[sarbogast]

I confirm the interest of this. Sometimes, you just want to display a small progress indicator instead of a text element that is loading, not just for a whole interface div.

[avilabiel]

I also confirm the interest. Right now, I just want to put the CircularProgress inside of a <p> for many reasons. I was 100% sure that we would have a component prop to use and change this behavior like Typograph.

[oliviertassinari]

@avilabiel I doubt we even need a component prop for this problem. Applying the following diff should be enough:

diff --git a/packages/material-ui/src/CircularProgress/CircularProgress.js b/packages/material-ui/src/CircularProgress/CircularProgress.js
index 12c0b188cf..56a8c85c67 100644
--- a/packages/material-ui/src/CircularProgress/CircularProgress.js
+++ b/packages/material-ui/src/CircularProgress/CircularProgress.js
@@ -111,7 +111,7 @@ const CircularProgress = React.forwardRef(function CircularProgress(props, ref)
   }

   return (
-    <div
+    <span
       className={clsx(
         classes.root,
         {
@@ -142,7 +142,7 @@ const CircularProgress = React.forwardRef(function CircularProgress(props, ref)
           strokeWidth={thickness}
         />
       </svg>
-    </div>
+    </span>
   );
 });

diff --git a/packages/material-ui/src/LinearProgress/LinearProgress.js b/packages/material-ui/src/LinearProgress/LinearProgress.js
index 23404c389f..d72ee51c80 100644
--- a/packages/material-ui/src/LinearProgress/LinearProgress.js
+++ b/packages/material-ui/src/LinearProgress/LinearProgress.js
@@ -19,6 +19,7 @@ export const styles = (theme) => {
     /* Styles applied to the root element. */
     root: {
       position: 'relative',
+      display: 'block',
       overflow: 'hidden',
       height: 4,
       zIndex: 0, // Fix Safari's bug during composition of different paint.
@@ -215,7 +216,7 @@ const LinearProgress = React.forwardRef(function LinearProgress(props, ref) {
   }

   return (
-    <div
+    <span
       className={clsx(
         classes.root,
         classes[`color${capitalize(color)}`],
@@ -233,9 +234,9 @@ const LinearProgress = React.forwardRef(function LinearProgress(props, ref) {
       {...other}
     >
       {variant === 'buffer' ? (
-        <div className={clsx(classes.dashed, classes[`dashedColor${capitalize(color)}`])} />
+        <span className={clsx(classes.dashed, classes[`dashedColor${capitalize(color)}`])} />
       ) : null}
-      <div
+      <span
         className={clsx(classes.bar, classes[`barColor${capitalize(color)}`], {
           [classes.bar1Indeterminate]: variant === 'indeterminate' || variant === 'query',
           [classes.bar1Determinate]: variant === 'determinate',
@@ -244,7 +245,7 @@ const LinearProgress = React.forwardRef(function LinearProgress(props, ref) {
         style={inlineStyles.bar1}
       />
       {variant === 'determinate' ? null : (
-        <div
+        <span
           className={clsx(classes.bar, {
             [classes[`barColor${capitalize(color)}`]]: variant !== 'buffer',
             [classes[`color${capitalize(color)}`]]: variant === 'buffer',
@@ -254,7 +255,7 @@ const LinearProgress = React.forwardRef(function LinearProgress(props, ref) {
           style={inlineStyles.bar2}
         />
       )}
-    </div>
+    </span>
   );
 });

What do you think about it?

[avilabiel]

@oliviertassinari thank you for your quick answer! Yeah, I do agree with you, but the @Mangatt description also makes sense:

It is valid use case to show loading for , or , which can all appear inside paragraphs or inline elements. There are cases that do not allow to change the whole parent structure to block elements.

This kind of flexibility seems good to implement because of the description above.

[oliviertassinari]

@avilabiel If you want to work on it, feel free to. I have added the "good first issue" label :).

[avilabiel]

Great, I will. Thank you, @oliviertassinari .

[bruno-azzi]

hey, is this issue still open? I would like to work on this.

[avilabiel]

Hey, @bruno-azzi feel free to create the PR. Unfortunately, I didn't have time to accomplish that task. Tks for that help!