Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(skeletons): added more examples #2232

Merged
merged 1 commit into from
Dec 20, 2023
Merged

docs(skeletons): added more examples #2232

merged 1 commit into from
Dec 20, 2023

Conversation

saiponnada
Copy link
Contributor

@saiponnada saiponnada commented Dec 14, 2023

Part of #2007

  • This PR contains CSS changes
  • This PR does not contain CSS changes

Description

Added more examples to the skeletons

Checklist

  • I verify the build is in a non-broken state
  • I verify all changes are within scope of the linked issue
  • I regenerated all CSS files under dist folder
  • I tested the UI in all supported browsers
  • I did a visual regression check of the components impacted by doing a Percy build and approved the build
  • I tested the UI in dark mode and RTL mode
  • I added/updated/removed Storybook coverage as appropriate

Copy link
Contributor

@ArtBlue ArtBlue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a couple of things...

Copy link
Contributor

@agliga agliga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm fine to approve as is.

I think the only comment I would add is I see some of the examples aren't using skeletons at all or just show how the rendering happens without them. I'm not sure if those are entirely useful because A) Someone looking at them might be confused what they are trying to convey and B) We are documenting these skeleton loaders and then using them in some examples. It would be almost like documenting an input element without adding our textbox classes on top of it.
So I think we change the title to say in order rendering without skeletons as well as in the description we explain why we are showing this without skeletons.
If there is no reason why we are showing the non-skeleton rendering, then we probably should remove them.

(The first two examples on this are what I am referring to)
Screenshot 2023-12-18 at 3 58 35 PM

@saiponnada
Copy link
Contributor Author

@ianmcburnie said he will help by adding docs (through another PR) explaining these examples in detail.

@saiponnada saiponnada merged commit 175ab36 into 17.0.0 Dec 20, 2023
2 checks passed
@saiponnada saiponnada deleted the skeleton-examples branch December 21, 2023 21:52
ArtBlue pushed a commit that referenced this pull request Dec 27, 2023
agliga pushed a commit that referenced this pull request Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants