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/tailwind blocks image layout page #1617

Conversation

FarhanAliRaza
Copy link
Contributor

@FarhanAliRaza FarhanAliRaza commented Jun 1, 2023

Linked Issue

Closes #1600

Description

Added a page in documentation for images layout

Changsets

Changesets automate our changelog. If you modify files in /package/skeleton, run pnpm changeset, follow the prompts, then commit the markdown file. Changes that add features should be minor while chores and bugfixes should be patch. Please prefix the changeset message with feat:, bugfix: or chore:.

Checklist

Please read and apply all contribution requirements.

  • This PR targets the dev branch (NEVER master)
  • Documentation reflects all relevant changes
  • Branch is prefixed with: docs/, feat/, chore/, bugfix/
  • Includes a changeset (if relevant; see above)
  • Ensure code linting is current - run pnpm format
  • All test cases are passing - run pnpm test

@changeset-bot
Copy link

changeset-bot bot commented Jun 1, 2023

⚠️ No Changeset found

Latest commit: 9a25147

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

@vercel
Copy link

vercel bot commented Jun 1, 2023

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

Name Status Preview Comments Updated (UTC)
skeleton-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 9, 2023 4:58pm

@endigo9740
Copy link
Contributor

endigo9740 commented Jun 1, 2023

Thank you @FarhanAliRaza, excellent choices for the images! Here's a few bits of feedback:

  1. The checklist in the PR message above actually creates interactive checkboxes. No need to insert emoji there. I've corrected this for you.
  2. We typically don't create Changesets for pure documentation updates, but the Changeset is responsible for generating our changelog. I'd really like for this change to be mentioned there, so please follow the Changeset instructions provided in the message above as well. Prefix with feat: and this will be considered a minor update.
  3. Per the contribution itself, the page looks great, but go ahead and remove the "Default Gallery" section with the code snippet. That's redundant with the code blog provided by the featured example at the top of the page.
  4. Go ahead and bump up the dimensions of the featured example at the top to 300x300. This should ensure it fills the full width for larger displays.
  5. The masonry example is only showing 2 columns for me - is there any way we can match the 4 column layout of the Flowbite reference? If you need help picking or sizing additional images let me know. Glad to help!
  6. As a convention, only the featured demo at the top should keep the gradient background - please set the Masonry DocPreviewer component to have a prop of background="neutral" please
  7. The "Featured Image" section is cool, but doesn't quite size correctly on larger screens, and for this to work properly we would need to introduce some JS logic. Let's go ahead and leave that one, but comment it out for now. I'll circle back to that in the near future.
  8. Finally, as mentioned in the original post, we're borrowing this directly from Flowbite. Even though Flowbite is covered under an MIT license and open source, let's provide the courtesy of attribution here. Here's an example for how to handle that. I'll provide text below.

Per Attribution, add the following:

Image layout markup and styles provided courtesy Flowbite. Please view our guide to learn more about integrating Flowbite in your Skeleton projects.

@endigo9740
Copy link
Contributor

endigo9740 commented Jun 1, 2023

Hey @FarhanAliRaza I see you've pushed revisions based on my feedback. Thank, this looks absolutely beautiful. Again, brilliant choices of images.

That said, we're going to be need to hold back this PR being merged for a short while, for two reasons:

  1. It seems some package-lock changes made it into your commit. We're going to reviewing our end the best way to handle this, as this could be detrimental if merged as is. This is not fault on you, it's just a new issue related to pnpm we're going to need to resolve on our end.
  2. I'm actually planning to meet with Zoltan, the creator of Flowbite, late next week. I'd actually like to show this to him directly and get his approval before we merge and go public with it.

Just wanted to give you a heads up - we do plan to move forward with merging this, and I'll keep you posted when this occurs. Unfortunately it won't make it in by next week's release, but I'd expect it to be in place by the following.

Thank you for contribution! I'm super excited to share this!

@endigo9740
Copy link
Contributor

@FarhanAliRaza two quick follow ups that we'll need fixed:

  1. Make sure the page title and URL are "Image Layouts" (plural) to follow our doc conventions
  2. Looks like Masonry was missed on the doc page as Masonary

If we come across anything else we'll let you know. But no rush per the comment above.

@endigo9740
Copy link
Contributor

@FarhanAliRaza We got the thumbs up from Zoltan and I've implemented a couple minor tweaks, so merging this now. Thanks again for your contribution. This will be included as part of the next release of Skeleton.

@endigo9740 endigo9740 merged commit 473f59e into skeletonlabs:dev Jun 9, 2023
3 of 4 checks passed
@FarhanAliRaza
Copy link
Contributor Author

Hover Card - shadcn_ui - Awesome Screenshot.webm
not related but I have a question. when we use hover in the popup the popup menu closes when we move away from the button. But it will be greater if we hover on the popup body it should not disappear. like shown in the video. Should i open a issue? I want to contribute

@endigo9740
Copy link
Contributor

@FarhanAliRaza I'd recommend you look at our roadmap to understand our future plans regarding popups:
#1367

In short, these will be split off, rebuilt to more tightly couple Floating UI, and the feature set improved and expanded. We hear the desire for features like this all the time, we're aware of them, but do have some technical prerequisites in play before we can begin implementing these changes.

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.

Tailwind Blocks - Image Layouts
2 participants