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

[group] control scroll position #43

Open
gitbreaker222 opened this issue Apr 8, 2020 · 8 comments · May be fixed by #52
Open

[group] control scroll position #43

gitbreaker222 opened this issue Apr 8, 2020 · 8 comments · May be fixed by #52

Comments

@gitbreaker222
Copy link

gitbreaker222 commented Apr 8, 2020

This is a summary of issues around similar use-cases:

I hope the use-case-namings are helpful - all these issues have in common, that we expect certain behaviours from the scroll position. To prevent solving one issue while re-creating another, I suggest to work towards one PR that solves them together 🚄 🤝

REPL: https://svelte.dev/repl/1c36db7c1e7e4ef2bfb04874321412e5?version=3.20.1


comparable vanilla implementations:

@gitbreaker222
Copy link
Author

gitbreaker222 commented Apr 12, 2020

attempting to create a "scrollTo" method, that scrolls an element by index at start, if list borders not hit. Problems so far:

  • programmatically scrolling until item is in view works gets blocked in repl after about 10 calls to viewport.scrollTo as of "infinite loop" detection (even if its a for loop with a limit of 99 iterations)
    • This approach is not elegant and just does, what user would do manually with the mouse wheel - but it still works for rows with variable height
  • calculating exact scroll disatance only reliable, if items have a fixed height by CSS. The property itemHeight does not control the actual row style, so the height has to be manually synced by user with correct calculation of box-height / border-width / padding / margin / box-sizing => to px value
    • as a result I find it more reliable to avoid itemHeight, instead just mind for a fixed height in CSS and let average_height take care of getting the rows computed pixel height
  • refresh is missing a check, if new list length is shorter than current scroll start
    • solution: call scrollTo(list.length - 1)
  • handle_scroll has this part, that should stabilize scrolling up: // prevent jumping if we scrolled up into unknown territory but this block somehow causes the position to be 0
    • removing it works better
  • if rows have dynamic height, list seems not to scroll to the very bottom (jumps up and down). This especially is problematic with the "chat" use case, because messages vary greatly in content size

@gitbreaker222
Copy link
Author

gitbreaker222 commented Apr 12, 2020

solved problems to this point:

  • "lazy loading" ✔️ : Works as expected without further changes
  • "jump to song" use case ✔️ : As soon rows have a fixed height, the "jumpToIndex" method works very well
  • "filter" use case ✔️ ❌ The new check fires, but items only rendered sometimes
    • problem seems to appear if list.length = 1000, start is at about 400, then filter reduces length to about 250. It works though, if scrolled to bottom or on second filter call
    • edit: fixed ✔️

tested in firefox

@gitbreaker222
Copy link
Author

update: after almost giving up fixing the remaining issue (adjust scroll position when filter list) I finally solved it! The demo has been updated and next I'd like to do a PR with clean code

So @Rich-Harris maybe now is a good time to get your attention :) My proposal:

  • add a new method "scrollToIndex"
    • <VirtualList {items} bind:scrollToIndex}></VirtualList>
  • update README documentation
  • close 4 related issues

Any thoughts before I start coding?

@gitbreaker222
Copy link
Author

Here is a webm of the demo 👼
https://windtanz.windcloud.de/index.php/s/d43AbLtXw9mimj9

@rsousacode
Copy link

rsousacode commented Dec 19, 2020

Very nice. Thanks! Apparently your _scrollTo3 solves my problem!! 👍
#50

@Metis77
Copy link

Metis77 commented Jan 21, 2021

great work!
REPL: https://svelte.dev/repl/1c36db7c1e7e4ef2bfb04874321412e5?version=3.20.1
should be merged.

gitbreaker222 pushed a commit to gitbreaker222/svelte-virtual-list that referenced this issue Jan 23, 2021
@gitbreaker222
Copy link
Author

gitbreaker222 commented Jan 23, 2021

I am preparing a pull request. Fork, cleanup, testing is done. One issue remains in logs:

"<VirtualList> was created with unknown prop 'scrollToIndex'"

How to properly expose a function / "method" so it can be called from outside? I played with bind: let: use: but don't get it :/

update:
bam got it - the consumer of binds to the exposed function scrollToIndex. Outside I've initialized a same named variable to bind to ... with a noop function

<script>
	export let scrollToIndex = () => {} // this should be "undefined"
</script>

<div class="List">
	<VirtualList {items}
		bind:start
		bind:end
		bind:scrollToIndex
		let:item
		>
		<ListItem {item}/>
	</VirtualList>
</div>

initializing with explicit undefined is correct https://stackoverflow.com/a/58574086/3313410 - thx @Rich-Harris

updating demo and opening PR shortly 👀

@gitbreaker222 gitbreaker222 linked a pull request Jan 23, 2021 that will close this issue
gitbreaker222 pushed a commit to gitbreaker222/svelte-virtual-list that referenced this issue Jun 2, 2021
@gitbreaker222
Copy link
Author

ℹ️ published fork
https://www.npmjs.com/package/svelte-virtual-list-ce

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 a pull request may close this issue.

3 participants