[Skeleton] Add `rounded` variant #33044

soullivaneuh · 2022-06-06T12:34:22Z

I have followed (at least) the PR section of the contributing guide.

mui-bot · 2022-06-06T12:38:15Z

Generated by 🚫 dangerJS against ef29835

docs/data/material/components/skeleton/Variants.js

oliviertassinari

It's not obvious to me is why we need this change. It seems that we could either change the Material UI default look & feel or ask developers to apply a theme customization when their designer wants to diverge from the default look.

oliviertassinari · 2022-06-06T22:25:45Z

docs/data/material/components/skeleton/skeleton.md

@@ -37,7 +37,7 @@ For instance:

 ## Variants

-The component supports 3 shape variants.
+The component supports 4 shape variants.


This sounds off. Aren't we mixing aesthetic considerations with the kind of component that the skeleton is meant to replace? I would expect 2 different props.

Aren't we mixing aesthetic considerations with the kind of component that the skeleton is meant to replace?

I'm not sure this is related to the change because we already have similar variants like the circle that does the same thing.

I would expect 2 different props.

I didn't follow you, which two props do you expect? 🤔

soullivaneuh · 2022-06-07T08:27:17Z

when their designer wants to diverge from the default look.

I do not really agree with that, this changes is not diverging from the default look to me.

This rounded variant is introduced here to match the already existing similar, like for the avatar: https://mui.com/material-ui/react-avatar/#variants

This is also why I use theme.shape.borderRadius as reference: https://github.com/mui/material-ui/pull/33044/files#diff-772a1a6c6b82fcb6b5bee1c620c5e856d7eb3bc09ded68930028cee8ad55be31R102

mnajdova

@soullivaneuh thanks! Please update to latest master, so we can merge it.

siriwatknp · 2022-07-18T07:25:58Z

docs/data/material/components/skeleton/skeleton.md

@@ -37,7 +37,7 @@ For instance:

 ## Variants

-The component supports 3 shape variants.
+The component supports 4 shape variants.


Suggested change

The component supports 4 shape variants.

The component supports 4 shape variants:

- `text`: represents a single line of text (you can change the height via font size).

- `circular`, `rectangular`, and `rounded`: provide only a border radius to let you take control of the size of the skeleton.

siriwatknp

👍

siriwatknp · 2022-07-18T07:26:36Z

docs/data/material/components/skeleton/Variants.tsx

@@ -7,7 +7,8 @@ export default function Variants() {
    <Stack spacing={1}>
      <Skeleton variant="text" />


Suggested change

<Skeleton variant="text" />

<Skeleton variant="text" sx={{ fontSize: '1rem' }} />

siriwatknp · 2022-07-18T07:26:45Z

docs/data/material/components/skeleton/Variants.tsx.preview

@@ -1,3 +1,4 @@
 <Skeleton variant="text" />


Suggested change

<Skeleton variant="text" />

<Skeleton variant="text" sx={{ fontSize: '1rem' }} />

siriwatknp · 2022-07-18T07:27:01Z

packages/mui-material/src/Skeleton/Skeleton.js

@@ -98,6 +98,9 @@ const SkeletonRoot = styled('span', {
      ...(ownerState.variant === 'circular' && {
        borderRadius: '50%',
      }),
+      ...(ownerState.variant === 'rounded' && {
+        borderRadius: theme.shape.borderRadius,


Suggested change

borderRadius: theme.shape.borderRadius,

borderRadius: (theme.vars || theme).shape.borderRadius,

siriwatknp · 2022-07-18T07:29:13Z

@soullivaneuh Not sure what's wrong but I could not commit the suggestions or push to your branch. cc @mnajdova

mnajdova · 2022-07-18T07:37:30Z

@soullivaneuh Not sure what's wrong but I could not commit the suggestions or push to your branch.

Hmm interesting, I've just tried and hit the same problem. Let's wait for @soullivaneuh's response. Maybe doing git clean and updating to latest master could fix the issue.

mnajdova · 2022-07-27T14:06:35Z

Looks like @soullivaneuh is not active anymore, so I guess we will need to re-create a new branch with the changes so that we can commit the suggestions.

siriwatknp · 2022-07-28T04:54:56Z

Sure, I can take care of it.

siriwatknp · 2022-07-29T03:03:20Z

closed in favor of #33687

soullivaneuh · 2022-08-23T08:42:41Z

Sorry for the miss, glad to see this finally merge! 👍

soullivaneuh force-pushed the skeleton/rounded-variant branch from 65d94aa to 6ca11b9 Compare June 6, 2022 12:45

soullivaneuh commented Jun 6, 2022

View reviewed changes

docs/data/material/components/skeleton/Variants.js Outdated Show resolved Hide resolved

soullivaneuh force-pushed the skeleton/rounded-variant branch 2 times, most recently from 5ed805a to 2786bec Compare June 6, 2022 12:50

feat(skeleton): rounded variant

ef29835

soullivaneuh force-pushed the skeleton/rounded-variant branch from 2786bec to ef29835 Compare June 6, 2022 12:55

oliviertassinari reviewed Jun 6, 2022

View reviewed changes

mnajdova changed the title ~~feat(skeleton): rounded variant~~ [Skeleton]: Add rounded variant Jul 12, 2022

mnajdova changed the title ~~[Skeleton]: Add rounded variant~~ [Skeleton] Add rounded variant Jul 12, 2022

mnajdova approved these changes Jul 12, 2022

View reviewed changes

siriwatknp reviewed Jul 18, 2022

View reviewed changes

siriwatknp approved these changes Jul 18, 2022

View reviewed changes

mnajdova added new feature New feature or request component: skeleton This is the name of the generic UI component, not the React module! labels Jul 18, 2022

siriwatknp mentioned this pull request Jul 29, 2022

[Skeleton] Add rounded variant #33687

Merged

1 task

siriwatknp closed this Jul 29, 2022

soullivaneuh deleted the skeleton/rounded-variant branch August 23, 2022 08:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Skeleton] Add `rounded` variant #33044

[Skeleton] Add `rounded` variant #33044

soullivaneuh commented Jun 6, 2022 •

edited

mui-bot commented Jun 6, 2022 •

edited

oliviertassinari left a comment •

edited

oliviertassinari Jun 6, 2022 •

edited

soullivaneuh Jun 14, 2022

soullivaneuh commented Jun 7, 2022

mnajdova left a comment •

edited

siriwatknp Jul 18, 2022

siriwatknp left a comment

siriwatknp Jul 18, 2022

siriwatknp Jul 18, 2022

siriwatknp Jul 18, 2022

siriwatknp commented Jul 18, 2022

mnajdova commented Jul 18, 2022

mnajdova commented Jul 27, 2022

siriwatknp commented Jul 28, 2022

siriwatknp commented Jul 29, 2022

soullivaneuh commented Aug 23, 2022

-The component supports 4 shape variants.
+The component supports 4 shape variants:
+- `text`: represents a single line of text (you can change the height via font size).
+- `circular`, `rectangular`, and `rounded`: provide only a border radius to let you take control of the size of the skeleton.

		@@ -7,7 +7,8 @@ export default function Variants() {
		<Stack spacing={1}>
		<Skeleton variant="text" />

	<Skeleton variant="text" />
	<Skeleton variant="text" sx={{ fontSize: '1rem' }} />

	borderRadius: theme.shape.borderRadius,
	borderRadius: (theme.vars \|\| theme).shape.borderRadius,

[Skeleton] Add rounded variant #33044

[Skeleton] Add rounded variant #33044

Conversation

soullivaneuh commented Jun 6, 2022 • edited

mui-bot commented Jun 6, 2022 • edited

oliviertassinari left a comment • edited

Choose a reason for hiding this comment

oliviertassinari Jun 6, 2022 • edited

Choose a reason for hiding this comment

soullivaneuh Jun 14, 2022

Choose a reason for hiding this comment

soullivaneuh commented Jun 7, 2022

mnajdova left a comment • edited

Choose a reason for hiding this comment

siriwatknp Jul 18, 2022

Choose a reason for hiding this comment

siriwatknp left a comment

Choose a reason for hiding this comment

siriwatknp Jul 18, 2022

Choose a reason for hiding this comment

siriwatknp Jul 18, 2022

Choose a reason for hiding this comment

siriwatknp Jul 18, 2022

Choose a reason for hiding this comment

siriwatknp commented Jul 18, 2022

mnajdova commented Jul 18, 2022

mnajdova commented Jul 27, 2022

siriwatknp commented Jul 28, 2022

siriwatknp commented Jul 29, 2022

soullivaneuh commented Aug 23, 2022

[Skeleton] Add `rounded` variant #33044

[Skeleton] Add `rounded` variant #33044

soullivaneuh commented Jun 6, 2022 •

edited

mui-bot commented Jun 6, 2022 •

edited

oliviertassinari left a comment •

edited

oliviertassinari Jun 6, 2022 •

edited

mnajdova left a comment •

edited