Fixed missing 'set' call for updating the variable scrollTop

[redvalley]

No description provided.

[taras]

It looks like in the mixin, scrollTop is set with this.scrollTop and this.set('scrollTop', scrollTop) - potentially depending on wether it should fire observers or not.

Curious, what prompted you to make this change?

[redvalley]

For me the ListView component throws an error when I am not using this.set here:
Uncaught Error: Assertion Failed: You must use Ember.set() to set the scrollTop property (of <(subclass of Ember.ListView):ember1662>) to 50.ember.js:3940 Ember.assertember.js:15818 SETTER_FUNCTIONlist-view.js:667 (anonymous function)ember.js:13173 instrumentlist-view.js:661 exports.default.Ember.Mixin.create._scrollContentTolist-view.js:1449 exports.default.Ember.ContainerView.extend.scrollTolist-view.js:1444 exports.default.Ember.ContainerView.extend.scroll

[stefanpenner]

actually, this.scrollTop was left as non-observable for performance reasons. Especially when tuning for some of the extremely crappy older android devices.

@ruperteder i suspect your code is accidentally observing scrollTop?

[redvalley]

Ok, I see. I am using ember list view for building an autocomplete component with endless scrolling functionality. Therefor I had to to observe the scrollTop property. I was not aware of that the scrollTop property should not be used as observable property.

[stefanpenner]

yes, it is currently not public API because of this. Although something like this should likely be public, but likely an action.

@ruperteder can you share an example of your usage ?

[redvalley]

Actually the scrollTop property was used in the past by my organization to fix a problem:
fixNegativeScrollTop: ember.observer(function () {
if (this.get('scrollTop') < 0) {
this.set('scrollTop', 0);
}
}, 'scrollTop'),

Today and yesterday I retested this and found out that the problem does no more occure.
So forget about my small change, it seems that it is not required anymore -. also by me.
Good thing is that now I know that scrollTop should not be observed, so thank you very much for this hint :-)

[stefanpenner]

:). I do suspect some addition api, likely an action should exists to fill this void. But observing scrollTop is nearly always some work around for something else.

Do you mind if I close this PR?

[mixonic]

Maybe an _scrollTop is in order

[stefanpenner]

list-view works hard to minimize work, it doesn't even enter a run-loop until something actually needs to change. So exposing _scrollTop as something observable forces us to do lots of extra work at often > 60FPS.

Ultimately my concern is only the slower android devices, hopefully they age out.

[tim-evans]

@ruperteder is it ok for us to close out this ticket?

[rwjblue]

I'm sorry we didn't get back to this previously, but at this point this repo is essentially unmaintained. Please use @html-next/vertical-collection or ember-collection for similar functionality.

Closing...