Skip to content

Scrolling optimization #66

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Mar 18, 2016
Merged

Scrolling optimization #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
727cd0c
adjustBuffer counters
mfeingold Mar 14, 2016
7c839c2
A fix for the performance problem?
mfeingold Mar 15, 2016
1500958
fixed linter complains
mfeingold Mar 16, 2016
3ff93b1
fixed 4 broken tests
mfeingold Mar 16, 2016
b4632b8
Merge commit '6df8a3dfa9a64f923a94b3b4a774127c28c63b29' into scrollin…
mfeingold Mar 16, 2016
310d023
tests fix + artifacts build
dhilt Mar 17, 2016
3c3107e
Update README.md
mfeingold Mar 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ The data source object implements methods and properties to be used by the direc
**Important:** Make sure to respect the `index` and `count` parameters of the request. The array passed to the success method should have
exactly `count` elements unless it hit eof/bof

* Properties `minIndex` and `maxIndex`

#### Description
As the scroller recieves the items requested by the `get` method, the value of minimum and maximum values of the item index are placed in the `minIndex` and `maxIndex` properties respectively. The values of the properties are cumulative - the value of the `minIndex` will never increase, and the value of the `maIndex` will never decrease - except the values are reset in response to a call to the adapter `reload` method. The values of the properties are used to maintain the appearance of the scroller scrollBar.

Values of the properties can be assigned programmatically. If the range of the index values is known in advance, assigneing them programmatically would improve the usability of the scrollBar.

###Adapter
The adapter object is an internal object created for every instance of the scroller. Properties and methods of the adapter can be used to manipulate and assess the scroller the adapter was created for. Adapter based API replaces old (undocumented) event based API introduced earlier for this purpose. The event based API is now deprecated and no longer supported.

Expand Down
4 changes: 2 additions & 2 deletions demo/examples/scopeDatasource.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
</head>
<body ng-controller="mainController">

<h2 style="position: fixed; left: 150px; top: 0;">is loading: {{loading}}; &nbsp; &nbsp; &nbsp; <a href="../index.html"> browse sample code</a></h2>
<h2 style="position: fixed; left: 150px; top: 0;">is loading: {{loading}}; &nbsp; &nbsp; &nbsp; <a href="../index.html"> browse sample code</a><br/>counters: ab {{adapter.abCount}} abf {{adapter.abfCount}} s {{adapter.sCount}}</h2>

<div ui-scroll-viewport style="width: 340px; height: 300px; display: block; background-color: white;">
<ul>
<li ui-scroll="item in datasource" is-loading="loading">*{{item}}*</li>
<li ui-scroll="item in datasource" adapter="adapter" is-loading="loading">*{{item}}*</li>
</ul>
</div>

Expand Down
2 changes: 1 addition & 1 deletion dist/ui-scroll-jqlite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*!
* angular-ui-scroll
* https://github.com/angular-ui/ui-scroll.git
* Version: 1.3.3 -- 2016-03-16T09:12:39.242Z
* Version: 1.3.3 -- 2016-03-17T12:18:01.421Z
* License: MIT
*/

Expand Down
2 changes: 1 addition & 1 deletion dist/ui-scroll-jqlite.min.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 28 additions & 6 deletions dist/ui-scroll.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*!
* angular-ui-scroll
* https://github.com/angular-ui/ui-scroll.git
* Version: 1.3.3 -- 2016-03-16T09:12:39.242Z
* Version: 1.3.3 -- 2016-03-17T12:18:01.421Z
* License: MIT
*/

Expand Down Expand Up @@ -586,15 +586,21 @@ angular.module('ui.scroll', []).directive('uiScrollViewport', function () {
adapter.reload = reload;

// events and bindings
viewport.bind('resize', resizeAndScrollHandler);
viewport.bind('scroll', resizeAndScrollHandler);
function bindEvents() {
viewport.bind('resize', resizeAndScrollHandler);
viewport.bind('scroll', resizeAndScrollHandler);
}
viewport.bind('mousewheel', wheelHandler);

function unbindEvents() {
viewport.unbind('resize', resizeAndScrollHandler);
viewport.unbind('scroll', resizeAndScrollHandler);
}

$scope.$on('$destroy', function () {
// clear the buffer. It is necessary to remove the elements and $destroy the scopes
buffer.clear();
viewport.unbind('resize', resizeAndScrollHandler);
viewport.unbind('scroll', resizeAndScrollHandler);
unbindEvents();
viewport.unbind('mousewheel', wheelHandler);
});

Expand Down Expand Up @@ -634,6 +640,10 @@ angular.module('ui.scroll', []).directive('uiScrollViewport', function () {
viewport.resetTopPaddingHeight();
viewport.resetBottomPaddingHeight();

adapter.abCount = 0;
adapter.abfCount = 0;
adapter.sCount = 0;

if (arguments.length) {
buffer.clear(arguments[0]);
} else {
Expand Down Expand Up @@ -755,6 +765,7 @@ angular.module('ui.scroll', []).directive('uiScrollViewport', function () {
function adjustBuffer(rid) {
// We need the item bindings to be processed before we can do adjustment
return $timeout(function () {
adapter.abCount++;
processBufferedItems(rid);

if (viewport.shouldLoadBottom()) {
Expand All @@ -772,6 +783,7 @@ angular.module('ui.scroll', []).directive('uiScrollViewport', function () {
function adjustBufferAfterFetch(rid) {
// We need the item bindings to be processed before we can do adjustment
return $timeout(function () {
adapter.abfCount++;
var keepFetching = processBufferedItems(rid);

if (viewport.shouldLoadBottom() && keepFetching) {
Expand All @@ -787,6 +799,7 @@ angular.module('ui.scroll', []).directive('uiScrollViewport', function () {

if (!pending.length) {
adapter.loading(false);
bindEvents();
return adapter.calculateProperties();
}

Expand Down Expand Up @@ -852,7 +865,16 @@ angular.module('ui.scroll', []).directive('uiScrollViewport', function () {

function resizeAndScrollHandler() {
if (!$rootScope.$$phase && !adapter.isLoading) {
adjustBuffer();
adapter.sCount++;
if (viewport.shouldLoadBottom()) {
enqueueFetch(ridActual, true);
} else if (viewport.shouldLoadTop()) {
enqueueFetch(ridActual, false);
}

if (pending.length) {
unbindEvents();
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions dist/ui-scroll.min.js
View file
Open in desktop

Large diffs are not rendered by default.

32 changes: 27 additions & 5 deletions src/ui-scroll.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,15 +568,21 @@ angular.module('ui.scroll', [])
adapter.reload = reload;

// events and bindings
viewport.bind('resize', resizeAndScrollHandler);
viewport.bind('scroll', resizeAndScrollHandler);
function bindEvents() {
viewport.bind('resize', resizeAndScrollHandler);
viewport.bind('scroll', resizeAndScrollHandler);
}
viewport.bind('mousewheel', wheelHandler);

function unbindEvents() {
viewport.unbind('resize', resizeAndScrollHandler);
viewport.unbind('scroll', resizeAndScrollHandler);
}

$scope.$on('$destroy', () => {
// clear the buffer. It is necessary to remove the elements and $destroy the scopes
buffer.clear();
viewport.unbind('resize', resizeAndScrollHandler);
viewport.unbind('scroll', resizeAndScrollHandler);
unbindEvents();
viewport.unbind('mousewheel', wheelHandler);
});

Expand Down Expand Up @@ -610,6 +616,10 @@ angular.module('ui.scroll', [])
viewport.resetTopPaddingHeight();
viewport.resetBottomPaddingHeight();

adapter.abCount = 0;
adapter.abfCount = 0;
adapter.sCount = 0;

if (arguments.length) {
buffer.clear(arguments[0]);
} else {
Expand Down Expand Up @@ -725,6 +735,7 @@ angular.module('ui.scroll', [])
function adjustBuffer(rid) {
// We need the item bindings to be processed before we can do adjustment
return $timeout(() => {
adapter.abCount++;
processBufferedItems(rid);

if (viewport.shouldLoadBottom()) {
Expand All @@ -742,6 +753,7 @@ angular.module('ui.scroll', [])
function adjustBufferAfterFetch(rid) {
// We need the item bindings to be processed before we can do adjustment
return $timeout(() => {
adapter.abfCount++;
let keepFetching = processBufferedItems(rid);

if (viewport.shouldLoadBottom() && keepFetching) {
Expand All @@ -757,6 +769,7 @@ angular.module('ui.scroll', [])

if (!pending.length) {
adapter.loading(false);
bindEvents();
return adapter.calculateProperties();
}

Expand Down Expand Up @@ -821,7 +834,16 @@ angular.module('ui.scroll', [])

function resizeAndScrollHandler() {
if (!$rootScope.$$phase && !adapter.isLoading) {
adjustBuffer();
adapter.sCount++;
if (viewport.shouldLoadBottom()) {
enqueueFetch(ridActual, true);
} else if (viewport.shouldLoadTop()) {
enqueueFetch(ridActual, false);
}

if (pending.length) {
unbindEvents();
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions test/BasicSetupSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ describe('uiScroll', function () {
runTest(scrollSettings,
function (viewport) {
expect($.fn.bind.calls.all().length).toBe(3);
expect($.fn.bind.calls.all()[0].args[0]).toBe('resize');
expect($.fn.bind.calls.all()[0].args[0]).toBe('mousewheel');
expect($.fn.bind.calls.all()[0].object[0]).toBe(viewport[0]);
expect($.fn.bind.calls.all()[1].args[0]).toBe('scroll');
expect($.fn.bind.calls.all()[1].args[0]).toBe('resize');
expect($.fn.bind.calls.all()[1].object[0]).toBe(viewport[0]);
expect($.fn.bind.calls.all()[2].args[0]).toBe('mousewheel');
expect($.fn.bind.calls.all()[2].args[0]).toBe('scroll');
expect($.fn.bind.calls.all()[2].object[0]).toBe(viewport[0]);
}, {
cleanupTest: function (viewport, scope, $timeout) {
Expand Down
33 changes: 12 additions & 21 deletions test/BasicTestsSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ describe('uiScroll', function () {

viewport.scrollTop(400);
viewport.trigger('scroll');
flush();

viewport.scrollTop(0);
viewport.trigger('scroll');
Expand Down Expand Up @@ -302,7 +301,6 @@ describe('uiScroll', function () {

viewport.scrollTop(400);
viewport.trigger('scroll');
flush();

viewport.scrollTop(0);
viewport.trigger('scroll');
Expand Down Expand Up @@ -397,11 +395,10 @@ describe('uiScroll', function () {
var flush = $timeout.flush;
viewport.scrollTop(viewportHeight + itemHeight);
viewport.trigger('scroll');
flush();

viewport.scrollTop(viewportHeight + itemHeight * 2);
viewport.trigger('scroll');
flush();
expect(flush).toThrow();

expect(spy.calls.all().length).toBe(4);

Expand Down Expand Up @@ -434,19 +431,15 @@ describe('uiScroll', function () {

viewport.scrollTop(0); //first full, scroll to -2
viewport.trigger('scroll');
flush();

viewport.scrollTop(0); //last full, scroll to -5, bof is reached
viewport.trigger('scroll');
flush();

expect(flush).toThrow();
viewport.scrollTop(0); //empty, no scroll occurred (-8)
viewport.trigger('scroll');
flush();

expect(flush).toThrow();

expect(spy.calls.all().length).toBe(5);
expect(spy.calls.all()[0].args[0]).toBe(1);
expect(spy.calls.all()[1].args[0]).toBe(4);
Expand Down Expand Up @@ -500,15 +493,15 @@ describe('uiScroll', function () {
wheelEventElement.dispatchEvent(getNewWheelEvent()); //now we are at the top but preventDefault is occurred because of bof will be reached only after next scroll trigger
expect(documentScrollBubblingCount).toBe(2); //here! the only one prevented wheel-event

flush();
//flush();

wheelEventElement.dispatchEvent(getNewWheelEvent()); //preventDefault will not occurred but document will not scroll because of viewport will be scrolled
expect(documentScrollBubblingCount).toBe(3);

viewport.scrollTop(0);
viewport.trigger('scroll'); //bof will be reached right after that

flush();
//flush();

wheelEventElement.dispatchEvent(getNewWheelEvent()); //preventDefault will not occurred because of we are at the top and bof is reached
expect(documentScrollBubblingCount).toBe(4);
Expand Down Expand Up @@ -560,19 +553,17 @@ describe('uiScroll', function () {

expect(!!scope.container1 && !!scope.container1.adapter && !!scope.container2).toBe(true);

scope.$watch('container2.isLoading', function(newValue, oldValue) {
switch(++isLoadingChangeCount) {
case 1: expect(newValue).toBe(true); expect(oldValue).toBe(true); break;
case 2: expect(newValue).toBe(false); expect(oldValue).toBe(true); break;
}
expect(scope.container1.adapter.isLoading).toBe(newValue);
// need to review: isLoading=true can't be catched since subscribe/unsibscribe optimization
scope.$watch('container1.adapter.isLoading', function(newValue) {
isLoadingChangeCount++;
expect(scope.container2.isLoading).toBe(newValue);
});

viewport.scrollTop(100);
viewport.trigger('scroll');
$timeout.flush();

expect(isLoadingChangeCount).toBe(2);
expect(isLoadingChangeCount).toBe(1);
}
);
});
Expand Down Expand Up @@ -636,16 +627,16 @@ describe('uiScroll', function () {
for(i = 0; i < limit; i++) {
viewport.scrollTop(viewport.scrollTop() + scrollDelta);
viewport.trigger('scroll');
$timeout.flush();
}

// scroll up, return to the top
for(i = 0; i < limit; i++) {
viewport.scrollTop(viewport.scrollTop() - scrollDelta);
viewport.trigger('scroll');
$timeout.flush();
}

$timeout.flush();

// scroll down + expectation
for(i = 0; i < limit; i++) {
viewport.scrollTop(viewport.scrollTop() + scrollDelta);
Expand Down Expand Up @@ -683,16 +674,16 @@ describe('uiScroll', function () {
for(i = 0; i < limit; i++) {
viewport.scrollTop(viewport.scrollTop() - scrollDelta);
viewport.trigger('scroll');
$timeout.flush();
}

// scroll down, return to the bottom
for(i = 0; i < limit; i++) {
viewport.scrollTop(viewport.scrollTop() + scrollDelta);
viewport.trigger('scroll');
$timeout.flush();
}

$timeout.flush();

// unstable delta passing
viewport.scrollTop(viewport.scrollTop());

Expand Down