shadcn-ui / ui

Beautifully designed components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://ui.shadcn.com
MIT License
73.53k stars 4.52k forks source link

[feat]: Skeleton component with auto-sizing #4608

Open juliusmarminge opened 2 months ago

juliusmarminge commented 2 months ago

Feature description

Radix Themes got this super nice skeleton component which has auto-sizing based on the children and a controllable state:

https://www.radix-ui.com/themes/docs/components/skeleton

Feels overkill installing Radix Themes just for this one component though

Usage:

<Skeleton>
  {Number(500_000).toLocaleString()}
</Skeleton>

No need to manually specify the size, just put some placeholder data and the skeleton adjusts accordingly

With controllable state:

<Skeleton loading={false}>
  {Number(500_000).toLocaleString()}
</Skeleton>

would display the actual number

Affected component/components

Skeleton

Additional Context

Additional details here...

Before submitting

viruop commented 2 months ago

Hi @juliusmarminge I've made some changes. Could you please take a look and merge them? Thanks!

juliusmarminge commented 2 months ago

It seems to take up too much space:

https://github.com/user-attachments/assets/96b5db4c-b204-4bc6-84cb-9141beb56d41

Tried fixing it but I am really bad at all the implicit padding elements have but guessing there just has to be a class swap somehwere

juliusmarminge commented 2 months ago

Ah my bad, just gotta make sure the Skeleton wraps the text node and not any wrapping span or similar:

https://github.com/user-attachments/assets/da0e7154-45b0-400f-ba56-779dc8cff957

-    <Skeleton loading={!!props.loadingTotal}>
     <span className="text-lg font-bold leading-none sm:text-3xl">
+       <Skeleton loading={!!props.loadingTotal}>
          {chartConfig[props.chart].formatValue(props.total)}
+       </Skeleton>
      </span>
-  </Skeleton>
viruop commented 2 months ago

Ah my bad, just gotta make sure the Skeleton wraps the text node and not any wrapping span or similar:

CleanShot.2024-08-22.at.17.19.16.mp4

-    <Skeleton loading={!!props.loadingTotal}>
     <span className="text-lg font-bold leading-none sm:text-3xl">
+       <Skeleton loading={!!props.loadingTotal}>
          {chartConfig[props.chart].formatValue(props.total)}
+       </Skeleton>
      </span>
-  </Skeleton>

Yes, the Skeleton should only wrap the text node to avoid unnecessary padding from other elements.

viruop commented 2 months ago

Hi @juliusmarminge, are there any further changes required from my end? If not, can you please merge this PR (https://github.com/shadcn-ui/ui/pull/4620)

juliusmarminge commented 2 months ago

I am not a maintainer of this repo.

But I did use the change from the PR and it works great, thank you 🙏

viruop commented 2 months ago

Glad it worked for you! I'm happy to help