Skeleton provides error in <p> tags

[ximex]

• 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 😯

Got an error in the logs

Expected Behavior 🤔

No error logs

Steps to Reproduce 🕹

Put a Skeleton in a ListItemText secondary prop. Than i got error logs in develop mode.
It's because a <div> in a <p> (Secondary Typography) is invalid

Your Environment 🌎

Tech	Version
Material-UI	v4.8.3
React	v16.11.0
Material-UI/lab	4.0.0-alpha.39

Solution

I think a skeletion variant=text should be <span> instead of a <div>.

[oliviertassinari]

@ximex What do you think of this diff?

diff --git a/packages/material-ui-lab/src/Skeleton/Skeleton.js b/packages/material-ui-lab/src/Skeleton/Skeleton.js
index c4696edc7..56b0ecf51 100644
--- a/packages/material-ui-lab/src/Skeleton/Skeleton.js
+++ b/packages/material-ui-lab/src/Skeleton/Skeleton.js
@@ -74,7 +74,7 @@ const Skeleton = React.forwardRef(function Skeleton(props, ref) {
     animation = 'pulse',
     classes,
     className,
-    component: Component = 'div',
+    component: Component = 'span',
     height,
     variant = 'text',
     width,

Using a span seems to be a frequent approach:

• https://ionicframework.com/docs/api/skeleton-text
• https://sancho-ui.com/components/skeleton/

Do you want to submit a pull request? :)

[oliviertassinari]

Note, the issue resonates with this comment: #7223 (comment) (button > div issue, CircularProgress). We could even address it at the same time.

[ximex]

or this way?

diff --git a/packages/material-ui-lab/src/Skeleton/Skeleton.js b/packages/material-ui-lab/src/Skeleton/Skeleton.js
index c4696edc7..56b0ecf51 100644
--- a/packages/material-ui-lab/src/Skeleton/Skeleton.js
+++ b/packages/material-ui-lab/src/Skeleton/Skeleton.js
@@ -74,7 +74,7 @@ const Skeleton = React.forwardRef(function Skeleton(props, ref) {
     animation = 'pulse',
     classes,
     className,
-    component: Component = 'div',
+    component: Component = props.variant === 'text' ? 'span' : 'div',
     height,
     variant = 'text',
     width,

a div makes sense for rect and circle

[oliviertassinari]

I'm wondering. Would being consistent be better? We render it as a display block no matter the element.

[ximex]

yes the rendering is display: block. but the semantic of the html would be more correct if we use a div for block things and span for text

[oliviertassinari]

OK, fair point. @ximex feel free to go ahead.