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

NEXT: Cookbook Image Layouts #2786

Merged
merged 2 commits into from
Aug 15, 2024

Conversation

kmalloy24
Copy link
Contributor

Description

Adds image layouts from v2 Blocks to v3 Cookbook. Includes two new layouts,

  • Quad
  • Featured

The page seems laggy on my local dev, it might be all the image URLs loading at full size. I can implement some image optimization features from Astro, but was trying to minimize any maintainable image assets in the codebase for now. It would also complicate the interaction between example components and raw code if we are maintaining a different component for the docs than what the example raw code is showing.

Copy link

changeset-bot bot commented Aug 1, 2024

⚠️ No Changeset found

Latest commit: ecf06cb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Aug 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
skeleton-docs ❌ Failed (Inspect) Aug 15, 2024 6:31pm

Copy link

vercel bot commented Aug 1, 2024

@kmalloy24 is attempting to deploy a commit to the Skeleton Labs Team on Vercel.

A member of the Team first needs to authorize it.

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 1, 2024

@kmalloy24 this is looking great! Just FYI you might can use Unsplash's CDN features to resize the images:

https://images.unsplash.com/photo-1617296538902-887900d9b592?ixid=M3w0Njc5ODF8MHwxfGFsbHx8fHx8fHx8fDE2ODc5NzExMDB8&ixlib=rb-4.0.3&w=128&h=128&auto=format&fit=crop

You'll note several URL params added at the end here:

&w=128
&h=128
&auto=format
&fit=crop
  • w = width
  • h = height

You might tailor those per example to ensure they are no larger than the need to be.

We'll want to try to avoid Astro-only optimization if possible, to keep this as agnostic as possible. Additionally I'm open to other image sources if you have any recommendations. Even something like: https://place-hold.it/

@kmalloy24
Copy link
Contributor Author

I had removed the Unsplash params to make the image sizing dependent on Tailwind classes instead. This way folks can just swap in their image sources without the need for URL params to maintain the layouts. I can go back to the previous approach - it was just a little confusing to me when I copied out the code block and lost the formatting when I put in different image urls.

I'll check out place-hold.it and see if it helps make the page more lightweight.

endigo9740

This comment was marked as resolved.

@endigo9740 endigo9740 marked this pull request as draft August 5, 2024 18:35
@endigo9740
Copy link
Contributor

endigo9740 commented Aug 5, 2024

Note I've bumped this back to draft state until the above work is complete. Feel free to tap "ready for review" when you reach that state again please!

@endigo9740
Copy link
Contributor

@kmalloy24 just checking on the status of this. No rush, I just want to confirm you saw the requested changes above.

@kmalloy24
Copy link
Contributor Author

@endigo9740 I did, thanks for taking the time to review! Was hoping to get to it this week, but realistically I probably won't get a chance until next week.

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 15, 2024

@kmalloy24 I finally had a moment free so I did a quick pass through this section. I made the following changes:

  1. Made a couple minor edits to the text copy on the main guide page
  2. Updated each example to use Lorem Picsum
  3. Cleaned up the styles within each example to only use what we need
  4. Swapped the fixed rounded class to Skeleton's version which will adhere to your theme

The guide should now be complete, so I'll merge right away. Thanks for your contribution as always!

@endigo9740 endigo9740 marked this pull request as ready for review August 15, 2024 18:29
@endigo9740 endigo9740 merged commit 0e104c4 into skeletonlabs:next Aug 15, 2024
4 of 7 checks passed
@kmalloy24
Copy link
Contributor Author

@endigo9740 Sweet! TIL about Lorem Picsum...will definitely be using that in the future.

This was a good experience getting my feet wet with v3 so hopefully will be more efficient on cookbook contributions in the future.

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 15, 2024

@kmalloy24 oh yeah it's cool.

Btw if you jump on another ticket (no rush) just a heads up that we're about to push a change that will make PNPM v9.50 the minimum version (9.7.x is the latest and that's fine too). I'm trying to give all the regular contributors a heads up about this one:

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.

2 participants