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

Unexpected behaviour when using scroll() #120

Closed
Devstackr8 opened this issue Jun 18, 2019 · 5 comments
Closed

Unexpected behaviour when using scroll() #120

Devstackr8 opened this issue Jun 18, 2019 · 5 comments
Labels

Comments

@Devstackr8
Copy link

Good Day, KingSora!

I am currently building a chat application, and one of the features is a lazy loading chat log - so when the user scrolls to the top more messages will load. I have created this functionality and with the regular scrollbar it works flawlessly. I got it working with OverlayScrollbars with a few minor adjustments using the scroll method (and passing a y value). It works when there is a lot of content in the scroll box (and when the track is quite small, maybe its because of this?) but when there isn't a lot of content (i.e. initially) it doesn't scroll to the right position.

I extracted the relevant code and made a Github repo: https://github.com/Devstackr8/overlay-scrollbars-lazy-load

I tried to setup a StackBlitz: https://stackblitz.com/edit/github-cq5cak
All the code is there, but for some reason the OverlayScrollbars package isn't installing - maybe you can get it working.

In the example, there are 2 pages (which you can access via tabs) - one shows the example without OverlayScrollbars - which works perfectly. And then the other page is the same example but using OverlayScrollbars. Keep on scrolling up to load new messages.

Many thanks in advance.

@KingSora
Copy link
Owner

Good day!

Ugh, that was a though one... Took me quite a while to figure out where the problem lies. Turns out this is happening because I'm using a MutationObserver to detect changes in the DOM. The problem is, MutationObservers are asynchronous, which means the DOM has changed already, but the plugin isn't aware of it. The scroll method won't scroll to the correct position, because it will clip the given coordinates between the min and max possible scroll value. For example you wanna scroll to 9999, but the element has only a scrollHeight of 1000, this would change the 9999 to the maximum possible scroll value. This ensures correct animation duration and prevents from possible direction: rtl bugs. In your example the scrollValue gets also clipped, thats because it can't scroll all the way down like it should.

After a bit of research I've found the takeRecords method which can be called to check synchronously if there are any mutations. I'll implement this logic in the scroll method in the next update, to ensure that whenever you call the scroll method the mutations are already processed and the plugin is aware of any DOM changes.

Until then you can fix this with a timeout (very ugly) like that:

this.messageElements.changes
	.pipe(take(1))
	.subscribe(() => {
		setTimeout(() => { 
			  let scrollVal = firstMsg.offsetTop - curOffset;
			  this.scrollbarInstance.scroll({ y: scrollVal });
		});
	});

Or with native scrolling since the clipping wont happen there:

this.messageElements.changes
	.pipe(take(1))
	.subscribe(() => {
		let scrollVal = firstMsg.offsetTop - curOffset;
		this.scrollbarInstance.getElements().viewport.scrollTop = scrollVal;
	});

@Devstackr8
Copy link
Author

Devstackr8 commented Jun 18, 2019

Amazing! Thank you so much for your quick response - and fix!
Just tried this out now and it works great!
With the first solution, I have found that there are occasions when the content of the scrollbox is visually updated before the scroll (due to the setTimeout) and results in a bit of a jarring effect. But the second solution works perfectly, thanks so much :)

Much appreciated, have a good day!

@KingSora
Copy link
Owner

I've fixed this bug in v1.7.3 please test it and provide some feedback. Thanks!

@Devstackr8
Copy link
Author

@KingSora Just tried it now and the bug is no longer present - thanks so much for the fix!

@KingSora
Copy link
Owner

No problem, glad I could help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants