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

Svelte Simple Datatables Guide #2443

Merged
merged 8 commits into from
Feb 9, 2024

Conversation

kmalloy24
Copy link
Contributor

@kmalloy24 kmalloy24 commented Jan 23, 2024

Description

Adds 3rd party integration guide to docs for Svelte Simple Datatables (SSD).

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/
  • Ensure Svelte and Typescript linting is current - run pnpm ci:check
  • Ensure Prettier linting is current - run pnpm format
  • All test cases are passing - run pnpm test
  • Includes a changeset (if relevant; see above)

Copy link

vercel bot commented Jan 23, 2024

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

Name Status Preview Updated (UTC)
skeleton-docs ✅ Ready (Inspect) Visit Preview Feb 9, 2024 8:37pm

Copy link

changeset-bot bot commented Jan 23, 2024

⚠️ No Changeset found

Latest commit: 0ccedba

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

@kmalloy24
Copy link
Contributor Author

I added SSD as a dev dependency in order to display working example components... not sure if that's ok or not.

I had some trouble with pnpm ci:check & pnpm format but all tests with pnpm test passed.

@endigo9740 endigo9740 changed the title SSD guide Svelte Simple Datatables Guide Jan 24, 2024
@endigo9740
Copy link
Contributor

endigo9740 commented Jan 29, 2024

@kmalloy24 so I've had a chance to finally sit down and go through the guide and honestly it's brilliant. Super detailed and very well explained, with lots of examples. My only concern that this has been integrated directly into the Skeleton documentation site itself.

While great for the quick preview, this poses a few issues:

  1. It diverges from the Tauri integration guide, which has it's own standalone GitHub project
  2. It seems like quite a lot of extra code to maintain on the v2 doc site, and to port to our new v3 docs
  3. We have no use case for the datatable in the v2 docs before v3 comes out, which again means extra overhead

So here's what I'm thinking:

  1. Like the Tauri guide, we divide the integration guide in two...
  2. The first half being a new GitHub repo to house the integration in a fresh new barebones Skeleton project, generated using the Skeleton CLI. Something folks can easily clone and reference. Or even fork and use as a starting point.
  3. The second half being the written guide on the Skeleton doc site. This becomes purely informational, perhaps substituting the rendered components with a reference links OR a screenshot.
  4. Unlike the Tauri guide, we setup a Cloudflare instance to host a live version of this standalone app. Which means there WILL still be an interactive version of this to preview. We may even be able to embed a version of it using Stackblitz. But it lives outside the Skeleton doc sites for all the reasons mentioned above.

Then, as we move towards Skeleton v3, I might get your help to tailor the guide and table element itself to better support the layout and feature set of SSD. Split the pagination features, add a dedicated styled and presentation for search, support the header column search inputs, etc. Make them feel integrated rather than bolted on. That is of course if you're down to help with this.

The timetable for the v3 stuff will be a bit further out, but I will create the new datatables repo immediately following this message. You should receive an invite shortly. Please feel free to follow up here or on Discord if you have any questions or feedback.

UPDATE

The new repo is available here:
https:/skeletonlabs/skeleton-datatables-integration

You've also been invited to join with a maintain role, which should give you extensive access.

@endigo9740
Copy link
Contributor

endigo9740 commented Jan 29, 2024

@niktek pinging you as an FYI as this may require some effort on Cloudflare. Though I don't think we need a fancy URL for this. Whatever the default is CF provides should suffice.

Also I don't want to bundle this in the monorepo for v2. But we should consideration for how we handle integration projects like this in v3 in the future.

@riziles
Copy link

riziles commented Jan 29, 2024

@endigo9740 , when I open the Vercel preview, there is a broken image link under the "Add Accessory Components" section. @kmalloy24 and I were trying to figure out how the existing Skeleton docs handle image hosting (see my guess here: vincjo/datatables#72 (comment) ). Could you please shed some light on this?

@endigo9740
Copy link
Contributor

endigo9740 commented Jan 29, 2024

@riziles I'm not quite sure which image you're referring to. Can you provide a link to this section?

Per image hosting, honestly we've just kind of throwing whatever works at it. I think given the limited number of images needed, most could be stored in a /static/img directory.

For v3 we're moving from Vercel -> Cloudfrare. Cloudflare apparently provides a pretty robust CDN service, so we'll be investigating that soon.

@riziles
Copy link

riziles commented Jan 29, 2024

@endigo9740 , This is what I see on the preview:
image

That missing image with the alt-text "flow" should be the SSD.webp image saved here:
https:/kmalloy24/skeleton/tree/docs/ssd-integration/sites/skeleton.dev/src/routes/(inner)/docs/ssd

@endigo9740
Copy link
Contributor

endigo9740 commented Jan 29, 2024

@riziles I see, I must have missed that. @kmalloy24 I'm not sure what browser support is for webp, but I'd suggest moving this to a jpg or png for now. Keep it simple.

@kmalloy24
Copy link
Contributor Author

@endigo9740 Sounds good! Sorry I should have confirmed my approach before dumping everything into the docs site. I'll focus this week on the new repo so you guys can get it configured with Cloudflare on your end. Then update this docs branch accordingly (likely next week). FYI, my wife and I are expecting our first baby end of Feb/early March, but could happen before then 🤷‍♂️ lol. Just want to let you know in case I go M.I.A for a bit.

I'd be happy to help with v3 support. The timing might work out well with Svelte 5, and I think @vincjo is planning a major version upgrade for that when it happens.

@riziles Thanks for bridging the communication gap 🙌

@endigo9740
Copy link
Contributor

endigo9740 commented Jan 30, 2024

@kmalloy24 oh my gosh, congrats to you and your wife! That's exciting to hear. And please, family first. Nothing here is a important as welcoming your child into the world. Please do go and enjoy that experience.

The good news per the guide side of things is that I think a lot will port over 1:1 in the new repo, and should happen easily because it's all just a SvelteKit/Skeleton project. If me or another maintainer get time we might even jump in and help make this happen. Maybe @riziles can even help? Let me know and I'll add you to the repo.

We're just very snowed in with the v3 tasks obviously. I'm about to push out the new maintenance release today and then share our big progress update for v3 shortly. So if you're curious about any of that look for the post in Discord and our Discussion announcement soon.

@kmalloy24
Copy link
Contributor Author

Thank you! I just pushed the first commit to the new repo. It's basically a copy of this which was created with the Skeleton CLI. So you're right it should be a pretty quick transition. I just have to part out the stuff from the docs branch.

@riziles
Copy link

riziles commented Jan 31, 2024

@endigo9740 , I'd be happy to help out. My Javascript skills suck, and I need to be careful about using my work laptop, but I'll do whatever I can.

@endigo9740
Copy link
Contributor

Thanks @riziles. We'll let Kyle do what he can for now. Seems like he's got a good head start already!

@kmalloy24
Copy link
Contributor Author

@endigo9740 The starter project & docs page are ready for your eyes again.

  • I setup the docs page with external link buttons like in the Tauri guide. Right now they point to the code in the starter project repo, but you can swap them out with a live link/Stackblitz as you see fit.
  • The two images are hosted in `/static' of the starter project repo and used there in the readme then called into the docs with their raw github link.

@endigo9740
Copy link
Contributor

@kmalloy24 thank you sir. It's end of day for me so I'll plan to circle back either this week or sometime next week. Definitely before the next release though!

@riziles
Copy link

riziles commented Feb 3, 2024

I see images! @kmalloy24 , the docs look great, but I didn't see a link anywhere to your demo site at https://simple-datatables-skeleton.vercel.app/ . I feel like that should be front and center. It's so good. Maybe @endigo9740 wants to eventually move it under the Skeleton umbrella, but I'd leave the working link in for now.

@endigo9740
Copy link
Contributor

endigo9740 commented Feb 3, 2024

We definitely want Kyle credited for the work here. I'm open to whatever that looks like. I'll provide some more formal thoughts after my review though.

@kmalloy24
Copy link
Contributor Author

I think it makes sense for everything to stay under the Skeleton umbrella. A live version of the new starter template can replace the old demo link since it's a little more refined and I'll keep the old one up for a while, but mark it depreciated, and reference the new one just so that there is one source of truth and we're not maintaining both. SSD has a link up on their docs now so we can send them the new link too.

@endigo9740
Copy link
Contributor

endigo9740 commented Feb 9, 2024

@falentio I've had a chance to do a pass through the dedicated repo. I think everything is great, but if we're going to provide this as an "official" recommended template, then I would prefer we match the Skeleton brand theme and aesthetic. As such, I've done a pass through and adjusted the instruction, links, and of course the visual UI a bit.

This is currently pending on a new dev branch. But if you give your blessing I'll be happy to merge it into main.

Screenshot 2024-02-09 at 12 23 35 PM

Screenshot 2024-02-09 at 12 23 45 PM

Screenshot 2024-02-09 at 12 23 56 PM

My review and updates for the doc site guide will follow shortly.

@endigo9740
Copy link
Contributor

endigo9740 commented Feb 9, 2024

@kmalloy24 @riziles in addition to the updates to the template repo above, I've now completed my pass through the written doc guide. You can preview here:

https://skeleton-docs-git-fork-kmalloy24-docs-ssd-cdf888-skeleton-labs.vercel.app/docs/ssd

The overall structure is prefect, but I've taken this opportunity to match the tone and style of our writing conventions, as well as flushed out a few details that weren't immediately obviously to me as a blind user. Full file paths, etc. Additionally I've opted to write "Svelte Simple Datatables" out in full - while it's a bit wordy, it should be more clear to end users.

Also, lease note I've provided attribute both on the template an doc guide. Please feel free to adjust as desired. Though I'd love to keep at least your username to help folks know how to reach you on Discord! That's up to you though, as privacy is important.

Again, once I receive your blessing, all of this will be merge and (hopefully) made available for next week's release. Which should drop on Tuesday (US CST time).

@endigo9740 endigo9740 marked this pull request as ready for review February 9, 2024 20:20
@kmalloy24
Copy link
Contributor Author

@endigo9740 everything looks good to me! I appreciate the attribution and like it as-is.

@endigo9740 endigo9740 merged commit 866cda1 into skeletonlabs:dev Feb 9, 2024
3 of 5 checks passed
@endigo9740
Copy link
Contributor

@kmalloy24 great! In that case I've gone ahead and merged everything. This will likely be the highlight of next week's release. Folks have been wanting something like this for a long while, so thank you so much for your help and support on this one!

@riziles
Copy link

riziles commented Feb 10, 2024

@endigo9740 , what are your thoughts on including a link to a live example (like @kmalloy24 's pre-existing site) or maybe a deploy to stackblitz button for the new repo (or maybe both)?

@endigo9740
Copy link
Contributor

@riziles thanks for reminding me. It's on my list, but we're juggling a couple other deployments for v3 stuff right now. I'll definitely try circle back.

Per Stackblitz @niktek handled that for our projects, so I'm not 100% sure the process.

I'd welcome help setting that up from whomever though!

@riziles
Copy link

riziles commented Feb 10, 2024

@endigo9740 , I created a PR to add an "Open in Stackblitz" button to the readme file. I just followed the instructions here: https://developer.stackblitz.com/guides/integration/open-from-github#the-open-in-stackblitz-button and it seems to work fine. I'll try to take a shot at adding to the docs as well when I'm procrastinating from the infinite amount of other work I should be doing instead...

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.

4 participants