[CL-427] Add skeleton loading components to the CL #16728
Conversation
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16728 +/- ##
========================================
Coverage 38.92% 38.93%
========================================
Files 3435 3446 +11
Lines 97482 97756 +274
Branches 14660 14694 +34
========================================
+ Hits 37949 38061 +112
- Misses 57874 58033 +159
- Partials 1659 1662 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| /** | ||
| * The shape of the corners of the skeleton element | ||
| */ | ||
| readonly edgeShape = input<"box" | "circle">("box"); |
There was a problem hiding this comment.
Does it make more sense for this to just be 'shape'?
There was a problem hiding this comment.
Maybe! I felt like shape="circle" would imply it's always a circle versus just the corners, but I could be overthinking it
There was a problem hiding this comment.
Yeah, I guess the consumer could pass circle and then give it differing width and heights. Might be confusing. Unless if we added width and height inputs. Then, if `shape='circle' we only take one of those values... 🤔 Do we think consumers will want full control to set styles with tailwind? Or are the width/height inputs a less complex DX?
There was a problem hiding this comment.
I thought about doing width/height inputs but it felt kind of silly when it's super simple with tailwind already, like I'd just be re-implementing classes. The designs have non-circle shapes with rounded corners so that is why I didn't want to imply the object itself being a circle
There was a problem hiding this comment.
Ah, got ya. In that case, I suppose we can't assume everything will be circular.
re width/height inputs: I'm not sure the assumption that it's easy to do with tailwind is necessarily true for everyone. We find it easy because we use tailwind every day but, some folks may not be as familiar. Explicit inputs might feel easier for them. IDK which is 'better' per se though
There was a problem hiding this comment.
Imo since tailwind is the standard throughout the apps, as long as it's clearly documented in Storybook how to use tailwind to apply width/height, it shouldn't be too confusing. (I think I need to work on the docs more.)
There was a problem hiding this comment.
@BryanCunningham I added more to the docs, lmk if you think it's more clear
BryanCunningham
left a comment
BryanCunningham
left a comment
There was a problem hiding this comment.
Looks good to me
vleague2
changed the title
|
This looks great Vicki! Only question is whether there is a strong reason to support both an oval and rectangle variant? I wonder if we could simplify to just the oval to reduce confusion on when to use which? |
@sukhleenb Looking at the Figma, it looks like some of the shapes have the full border radius like a circle and some have the 4px more boxy border radius. So I think we will need both unless we want to remove the 4px radius option? The component is pretty configurable, so the stories are examples of what you could do to make whatever basic shape you need. (I.e. you specify the border radius and then you can set whatever height and width you want, which gives the flexibility to make all those shapes) |
|
@vleague2 Thanks for reformatting some of the docs, I think that's where most of my confusion was coming from! |
|
🎟️ Tracking
CL-427
📔 Objective
Add skeleton components to our CL to allow teams to compose skeleton loading screens.
📸 Screenshots
See new stories under Skeleton and new skeleton loading story under Popup Layout
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes