svelte-tiny-virtual-list icon indicating copy to clipboard operation
svelte-tiny-virtual-list copied to clipboard

[Fork] Svelte 5 Support

Open TutorLatin opened this issue 1 year ago • 11 comments

Svelte-tiny-virtual-list with runes.

This replaces #43

TutorLatin avatar Jun 21 '24 19:06 TutorLatin

Since the original author seems to have not merged this PR for a long time, should we consider forking this repository and releasing a new npm package until @jonasgeiler has time to review and merge this PR?

rxliuli avatar Feb 01 '25 02:02 rxliuli

for now: https://www.npmjs.com/package/svelte5-tiny-virtual-list

Altough I'm not sure it works - I just published it as is and I'm still experiencing issues.

Would be great if there is an official solution soon @jonasgeiler

LiamKrenn avatar Feb 20 '25 15:02 LiamKrenn

Thanks for the npm package @LiamKrenn What issues are you experiencing?

TutorLatin avatar Feb 21 '25 09:02 TutorLatin

@TutorLatin image After a few elements, the bottom elements are just hidden. You can't scroll too. Height should be more than enough

LiamKrenn avatar Feb 21 '25 09:02 LiamKrenn

That's weird. Make sure you have migrated all slots to snippets.

TutorLatin avatar Feb 21 '25 11:02 TutorLatin

I did, and it seems like it's a styling error. When more items are in the list, it's scrollable again, but it's weird and you can't scroll to the bottom. Here's a screen recording: https://youtu.be/yKUIum_TrOM

LiamKrenn avatar Feb 21 '25 12:02 LiamKrenn

@TutorLatin I figured out that it has to do with the height property. With your fork it expects a string, but this breaks things. If you pass a number like height={200} it works again as expected, even though the type check throws an error.

LiamKrenn avatar Feb 22 '25 12:02 LiamKrenn

I did, and it seems like it's a styling error. When more items are in the list, it's scrollable again, but it's weird and you can't scroll to the bottom. Here's a screen recording: https://youtu.be/yKUIum_TrOM

I figured out that it has to do with the height property. With your fork it expects a string, but this breaks things. If you pass a number like height={200} it works again as expected, even though the type check throws an error.

Same problem here. infiniteHandler triggers twice after initial rendering, and after that not triggers anymore. Only first portion of data is rendered, and there are glitches when scrolling up and down.

Setting integers to height attribute is not helping with a problem.

ubzor avatar Mar 07 '25 10:03 ubzor

The string problem mentioned by @LiamKrenn should have been fixed by 4846715. Make sure you are building from the latest version.

TutorLatin avatar Mar 09 '25 10:03 TutorLatin

@jonasgeiler can you check this PR please 🙏

JLAcostaEC avatar May 27 '25 17:05 JLAcostaEC

I published the fork on npm : https://www.npmjs.com/package/@tutorlatin/svelte-tiny-virtual-list I'll maintain it for now, until the original repo becomes active again.

TutorLatin avatar May 30 '25 08:05 TutorLatin

@TutorLatin @rxliuli @LiamKrenn @ubzor @JLAcostaEC First and foremost I'd like to apologize to all of you, and everyone reading this who relies on my library, for not merging or looking at this PR for ages - and kind of abandoning this project altogether. I am taking some time out of my day right now to finally handle this and get back into this project.

I am very sorry if I caused any delays or inconveniences, and I'd also like to thank you for staying civil.


Dear @TutorLatin, I've put reviewing your PRs off for so often, mostly since I felt a bit overwhelmed by the amount of changes of your previous PR, #43. And then I just used that as an excuse to do the review at a later time, at a later time, and so on... Crap. However, I never took a closer look at this reworked one, and I must say I really love what you did here! It's clean and looks like it has minimal to no breaking changes! After writing this comment I'll take a closer look though.

~~So I'd like to get this PR ready for a merge, for that I'd either need a different branch than your main branch where I can do changes without breaking your fork, or for you to temporarily undo your forked package changes, like adding your namespace to the package name in package.json and so on. Currently your PR includes all these changes.~~ Nevermind, I found out you can change which branch a PR merges into. I'll just make a temporary branch where I merge your PR.

Secondly I'd like to propose adding you, @TutorLatin, as a Contributor to this repo, if you want! I feel like I've kinda lost track of how to use Svelte a bit, since I'm still a Svelte v4 guy, and have never really gotten familiar with those new features like runes and stuff. 😅 Part of the reason I put off the review of your PRs for so long... Since you're the author of this big migration and more familiar with Svelte v5, I might need help with issues or bugs in the future that relate to your changes. And, since you've put so much work into your fork, it just makes sense for me to give you a role here. Of course I am not trying to hand over all the maintenance work to you, since I'll try my best to finally learn Svelte v5 and not to abandon this project again - especially with all the traction and attention it has now.

Thank you so much for the PR and all your hard work and patience! I'll go over this PR now, ignoring changes that relate to your forked package.

jonasgeiler avatar Jul 02 '25 21:07 jonasgeiler