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.

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

fix(virtualScroll) flickering issue fixes. No need for re-rendering the whole DOM #12917

Closed
wants to merge 22 commits into from
Closed

fix(virtualScroll) flickering issue fixes. No need for re-rendering the whole DOM #12917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4cf1f39
Convert queue consts to enum
masimplo Sep 15, 2017
fdd9bd1
Initialize differ with trackByFunction as ngForOf does
masimplo Sep 15, 2017
8e5774a
Throw errors not strings
masimplo Sep 15, 2017
ad9c6d9
Simplify addCell function
masimplo Sep 15, 2017
a716e58
Simplify changes calculation
masimplo Sep 15, 2017
7d3b7c4
Make debug logs consistent
masimplo Sep 15, 2017
21a7ce9
Stop searching nodes if cell is already rendered
masimplo Sep 15, 2017
ebef282
Change TemplateType consts to enum
masimplo Sep 15, 2017
75aed51
Stop looking for available nodes after one is found
masimplo Sep 15, 2017
a13184f
Change comment
masimplo Sep 15, 2017
e06d757
Return records from VirtualScroll getter
masimplo Sep 15, 2017
8590b9b
Stop clearing DOM in slow path
masimplo Sep 18, 2017
acec8ef
Update existing nodes by matching cells
masimplo Sep 18, 2017
c8d5e09
Fix top and bottom cells special cases
masimplo Sep 18, 2017
9b3b0db
Use recordSize consistently
masimplo Sep 18, 2017
07e15d7
Simplify adjustRendered function
masimplo Sep 18, 2017
84d402d
Remove console.warn
masimplo Sep 18, 2017
bfa44b9
Remove needClean param from populateNodeData
masimplo Sep 18, 2017
564ef41
Recalculated nodes once more on scrollEnd
masimplo Sep 19, 2017
6eaf7d1
Use const enums to reduce generated code
masimplo Sep 20, 2017
c6a2a97
Fix virtualScroll e2e infiniteScroll test
masimplo Sep 20, 2017
3497d40
Fix virtualScroll spec
masimplo Sep 20, 2017
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
6 changes: 4 additions & 2 deletions src/components/virtual-scroll/test/infinite-scroll/app.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Component, NgModule } from '@angular/core';
import { IonicApp, IonicModule } from '../../../..';
import { BrowserModule } from '@angular/platform-browser';


@Component({
Expand All @@ -26,7 +27,7 @@ export class E2EPage {

return getAsyncData().then(newData => {
for (var i = 0; i < newData.length; i++) {
this.items.push( this.items.length );
this.items.push(this.items.length);
}

console.log('Finished receiving data, async operation complete');
Expand Down Expand Up @@ -71,6 +72,7 @@ export class E2EApp {
E2EPage
],
imports: [
BrowserModule,
IonicModule.forRoot(E2EApp)
],
bootstrap: [IonicApp],
Expand All @@ -79,4 +81,4 @@ export class E2EApp {
E2EPage
]
})
export class AppModule {}
export class AppModule { }
5 changes: 5 additions & 0 deletions src/components/virtual-scroll/test/infinite-scroll/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';

import { AppModule } from './app.module';

platformBrowserDynamic().bootstrapModule(AppModule);
132 changes: 66 additions & 66 deletions src/components/virtual-scroll/test/virtual-scroll.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('VirtualScroll', () => {
ftrWidth: viewportWidth,
ftrHeight: HEIGHT_FOOTER
};
window.getComputedStyle = function() {
window.getComputedStyle = function () {
var styles: any = {
marginTop: '0px',
marginRight: '0px',
Expand All @@ -33,7 +33,7 @@ describe('VirtualScroll', () => {
};
});

describe('estimateHeight', () => {
describe('estimateHeight', () => {

it('should return zero when no records', () => {
const h = estimateHeight(0, undefined, 100, .25);
Expand All @@ -42,14 +42,14 @@ describe('VirtualScroll', () => {

});

describe('processRecords', () => {
describe('processRecords', () => {

it('should load data for 100% width items', () => {
records = [0, 1, 2, 3, 4];
let stopAtHeight = 200;

processRecords(stopAtHeight, records, cells,
headerFn, footerFn, data);
headerFn, footerFn, data);

expect(cells.length).toBe(4);

Expand Down Expand Up @@ -81,16 +81,16 @@ describe('VirtualScroll', () => {
data.hdrWidth = data.viewWidth; // 100%, 1 per row
data.ftrWidth = data.viewWidth; // 100%, 1 per row

headerFn = function(record: any) {
headerFn = function (record: any) {
return (record === 0) ? 'Header' : null;
};

footerFn = function(record: any) {
footerFn = function (record: any) {
return (record === 4) ? 'Footer' : null;
};

processRecords(stopAtHeight, records, cells,
headerFn, footerFn, data);
headerFn, footerFn, data);

expect(cells.length).toBe(7);

Expand Down Expand Up @@ -165,12 +165,12 @@ describe('VirtualScroll', () => {
data.itmWidth = 90; // 2 per row
data.hdrWidth = data.viewWidth; // 100%, 1 per row

headerFn = function(record: any) {
headerFn = function (record: any) {
return (record === 0) ? 'Header' : null;
};

processRecords(stopAtHeight, records, cells,
headerFn, footerFn, data);
headerFn, footerFn, data);

expect(cells.length).toBe(5);

Expand All @@ -188,7 +188,7 @@ describe('VirtualScroll', () => {

stopAtHeight = 150;
processRecords(stopAtHeight, records, cells,
headerFn, footerFn, data);
headerFn, footerFn, data);

expect(cells[2].row).toBe(1);
expect(cells[2].top).toBe(HEIGHT_HEADER);
Expand All @@ -206,7 +206,7 @@ describe('VirtualScroll', () => {

stopAtHeight = 20000;
processRecords(stopAtHeight, records, cells,
headerFn, footerFn, data);
headerFn, footerFn, data);

expect(cells.length).toBe(11);

Expand All @@ -230,36 +230,36 @@ describe('VirtualScroll', () => {
let endCellIndex = 0;

populateNodeData(startCellIndex, endCellIndex, true,
cells, records, nodes, viewContainer,
itmTmp, hdrTmp, ftrTmp, false);
cells, records, nodes, viewContainer,
itmTmp, hdrTmp, ftrTmp);

expect(nodes.length).toBe(0);
});

it('should skip already rendered, and create nodes', () => {
cells = [
{row: 0, tmpl: TEMPLATE_ITEM},
{row: 1, tmpl: TEMPLATE_ITEM},
{row: 2, tmpl: TEMPLATE_HEADER},
{row: 3, tmpl: TEMPLATE_ITEM},
{row: 4, tmpl: TEMPLATE_FOOTER},
{row: 5, tmpl: TEMPLATE_ITEM},
{row: 6, tmpl: TEMPLATE_ITEM, reads: 0}
{ row: 0, tmpl: TEMPLATE_ITEM },
{ row: 1, tmpl: TEMPLATE_ITEM },
{ row: 2, tmpl: TEMPLATE_HEADER },
{ row: 3, tmpl: TEMPLATE_ITEM },
{ row: 4, tmpl: TEMPLATE_FOOTER },
{ row: 5, tmpl: TEMPLATE_ITEM },
{ row: 6, tmpl: TEMPLATE_ITEM, reads: 0 }
];

nodes = [
{cell: 0, tmpl: TEMPLATE_ITEM, view: getView()},
{cell: 1, tmpl: TEMPLATE_ITEM, view: getView()},
{cell: 2, tmpl: TEMPLATE_HEADER, view: getView()},
{cell: 3, tmpl: TEMPLATE_ITEM, view: getView()},
{ cell: 0, tmpl: TEMPLATE_ITEM, view: getView() },
{ cell: 1, tmpl: TEMPLATE_ITEM, view: getView() },
{ cell: 2, tmpl: TEMPLATE_HEADER, view: getView() },
{ cell: 3, tmpl: TEMPLATE_ITEM, view: getView() },
];

let startCellIndex = 2;
let endCellIndex = 5;

populateNodeData(startCellIndex, endCellIndex, true,
cells, records, nodes, viewContainer,
itmTmp, hdrTmp, ftrTmp, false);
cells, records, nodes, viewContainer,
itmTmp, hdrTmp, ftrTmp);

expect(nodes.length).toBe(5);

Expand All @@ -274,21 +274,21 @@ describe('VirtualScroll', () => {

it('should create nodes', () => {
cells = [
{row: 0, tmpl: TEMPLATE_ITEM},
{row: 1, tmpl: TEMPLATE_ITEM},
{row: 2, tmpl: TEMPLATE_HEADER},
{row: 3, tmpl: TEMPLATE_ITEM},
{row: 4, tmpl: TEMPLATE_FOOTER},
{row: 5, tmpl: TEMPLATE_ITEM},
{row: 6, tmpl: TEMPLATE_ITEM}
{ row: 0, tmpl: TEMPLATE_ITEM },
{ row: 1, tmpl: TEMPLATE_ITEM },
{ row: 2, tmpl: TEMPLATE_HEADER },
{ row: 3, tmpl: TEMPLATE_ITEM },
{ row: 4, tmpl: TEMPLATE_FOOTER },
{ row: 5, tmpl: TEMPLATE_ITEM },
{ row: 6, tmpl: TEMPLATE_ITEM }
];

let startCellIndex = 2;
let endCellIndex = 4;

populateNodeData(startCellIndex, endCellIndex, true,
cells, records, nodes, viewContainer,
itmTmp, hdrTmp, ftrTmp, true);
cells, records, nodes, viewContainer,
itmTmp, hdrTmp, ftrTmp);

expect(nodes.length).toBe(3);

Expand All @@ -308,21 +308,21 @@ describe('VirtualScroll', () => {
it('should get all the row heights w/ 30% width rows', () => {
let firstTop = 13;
nodes = [
{cell: 0, tmpl: TEMPLATE_HEADER, view: getView(data.viewWidth, HEIGHT_HEADER, firstTop, 0)},
{cell: 1, tmpl: TEMPLATE_ITEM, view: getView(90, HEIGHT_ITEM, HEIGHT_HEADER + firstTop, 0)},
{cell: 2, tmpl: TEMPLATE_ITEM, view: getView(90, HEIGHT_ITEM, HEIGHT_HEADER + firstTop, 90)},
{cell: 3, tmpl: TEMPLATE_ITEM, view: getView(90, HEIGHT_ITEM, HEIGHT_HEADER + firstTop, 180)},
{cell: 4, tmpl: TEMPLATE_ITEM, view: getView(90, HEIGHT_ITEM, HEIGHT_HEADER + HEIGHT_ITEM + firstTop, 0)},
{cell: 5, tmpl: TEMPLATE_FOOTER, view: getView(data.viewWidth, HEIGHT_FOOTER, HEIGHT_HEADER + HEIGHT_ITEM + HEIGHT_ITEM + firstTop, 0)},
{ cell: 0, tmpl: TEMPLATE_HEADER, view: getView(data.viewWidth, HEIGHT_HEADER, firstTop, 0) },
{ cell: 1, tmpl: TEMPLATE_ITEM, view: getView(90, HEIGHT_ITEM, HEIGHT_HEADER + firstTop, 0) },
{ cell: 2, tmpl: TEMPLATE_ITEM, view: getView(90, HEIGHT_ITEM, HEIGHT_HEADER + firstTop, 90) },
{ cell: 3, tmpl: TEMPLATE_ITEM, view: getView(90, HEIGHT_ITEM, HEIGHT_HEADER + firstTop, 180) },
{ cell: 4, tmpl: TEMPLATE_ITEM, view: getView(90, HEIGHT_ITEM, HEIGHT_HEADER + HEIGHT_ITEM + firstTop, 0) },
{ cell: 5, tmpl: TEMPLATE_FOOTER, view: getView(data.viewWidth, HEIGHT_FOOTER, HEIGHT_HEADER + HEIGHT_ITEM + HEIGHT_ITEM + firstTop, 0) },
];

cells = [
{row: 0, tmpl: TEMPLATE_HEADER, reads: 0},
{row: 0, tmpl: TEMPLATE_ITEM, reads: 0},
{row: 0, tmpl: TEMPLATE_ITEM, reads: 0},
{row: 0, tmpl: TEMPLATE_ITEM, reads: 0},
{row: 0, tmpl: TEMPLATE_ITEM, reads: 0},
{row: 0, tmpl: TEMPLATE_FOOTER, reads: 0},
{ row: 0, tmpl: TEMPLATE_HEADER, reads: 0 },
{ row: 0, tmpl: TEMPLATE_ITEM, reads: 0 },
{ row: 0, tmpl: TEMPLATE_ITEM, reads: 0 },
{ row: 0, tmpl: TEMPLATE_ITEM, reads: 0 },
{ row: 0, tmpl: TEMPLATE_ITEM, reads: 0 },
{ row: 0, tmpl: TEMPLATE_FOOTER, reads: 0 },
];

initReadNodes(mockPlatform(), nodes, cells, data);
Expand Down Expand Up @@ -360,19 +360,19 @@ describe('VirtualScroll', () => {

it('should get all the row heights w/ all 100% width rows', () => {
nodes = [
{cell: 0, tmpl: TEMPLATE_HEADER, view: getView(data.viewWidth, HEIGHT_HEADER, 0, 0)},
{cell: 1, tmpl: TEMPLATE_ITEM, view: getView(data.viewWidth, HEIGHT_ITEM, HEIGHT_HEADER, 0)},
{cell: 2, tmpl: TEMPLATE_ITEM, view: getView(data.viewWidth, HEIGHT_ITEM, HEIGHT_HEADER + HEIGHT_ITEM, 0)},
{cell: 3, tmpl: TEMPLATE_ITEM, view: getView(data.viewWidth, HEIGHT_ITEM, HEIGHT_HEADER + HEIGHT_ITEM + HEIGHT_ITEM, 0)},
{cell: 4, tmpl: TEMPLATE_FOOTER, view: getView(data.viewWidth, HEIGHT_FOOTER, HEIGHT_HEADER + HEIGHT_ITEM + HEIGHT_ITEM + HEIGHT_ITEM, 0)},
{ cell: 0, tmpl: TEMPLATE_HEADER, view: getView(data.viewWidth, HEIGHT_HEADER, 0, 0) },
{ cell: 1, tmpl: TEMPLATE_ITEM, view: getView(data.viewWidth, HEIGHT_ITEM, HEIGHT_HEADER, 0) },
{ cell: 2, tmpl: TEMPLATE_ITEM, view: getView(data.viewWidth, HEIGHT_ITEM, HEIGHT_HEADER + HEIGHT_ITEM, 0) },
{ cell: 3, tmpl: TEMPLATE_ITEM, view: getView(data.viewWidth, HEIGHT_ITEM, HEIGHT_HEADER + HEIGHT_ITEM + HEIGHT_ITEM, 0) },
{ cell: 4, tmpl: TEMPLATE_FOOTER, view: getView(data.viewWidth, HEIGHT_FOOTER, HEIGHT_HEADER + HEIGHT_ITEM + HEIGHT_ITEM + HEIGHT_ITEM, 0) },
];

cells = [
{row: 0, tmpl: TEMPLATE_HEADER, reads: 0},
{row: 1, tmpl: TEMPLATE_ITEM, reads: 0},
{row: 2, tmpl: TEMPLATE_ITEM, reads: 0},
{row: 3, tmpl: TEMPLATE_ITEM, reads: 0},
{row: 4, tmpl: TEMPLATE_FOOTER, reads: 0},
{ row: 0, tmpl: TEMPLATE_HEADER, reads: 0 },
{ row: 1, tmpl: TEMPLATE_ITEM, reads: 0 },
{ row: 2, tmpl: TEMPLATE_ITEM, reads: 0 },
{ row: 3, tmpl: TEMPLATE_ITEM, reads: 0 },
{ row: 4, tmpl: TEMPLATE_FOOTER, reads: 0 },
];

initReadNodes(mockPlatform(), nodes, cells, data);
Expand Down Expand Up @@ -400,7 +400,7 @@ describe('VirtualScroll', () => {

it('should adjust when all the way to the top, scrolling down', () => {
for (var i = 0; i < 100; i++) {
cells.push({top: i, height: 1, row: i});
cells.push({ top: i, height: 1, row: i });
}
data.scrollDiff = 1;
data.viewHeight = 20;
Expand All @@ -418,7 +418,7 @@ describe('VirtualScroll', () => {

it('should adjust when in the middle, scrolling down', () => {
for (var i = 0; i < 100; i++) {
cells.push({top: i, height: 1, row: i});
cells.push({ top: i, height: 1, row: i });
}
data.scrollDiff = 1;
data.viewHeight = 20;
Expand All @@ -436,7 +436,7 @@ describe('VirtualScroll', () => {

it('should adjust when all the way to the bottom, scrolling down', () => {
for (var i = 0; i < 100; i++) {
cells.push({top: i, height: 1, row: i});
cells.push({ top: i, height: 1, row: i });
}
data.scrollDiff = 1;
data.viewHeight = 20;
Expand All @@ -454,7 +454,7 @@ describe('VirtualScroll', () => {

it('should adjust when all the way to the bottom, scrolling up', () => {
for (var i = 0; i < 100; i++) {
cells.push({top: i, height: 1, row: i});
cells.push({ top: i, height: 1, row: i });
}
data.scrollDiff = -1;
data.viewHeight = 20;
Expand Down Expand Up @@ -546,10 +546,10 @@ describe('VirtualScroll', () => {
let hdrTmp: any = {};
let ftrTmp: any = {};
let viewContainer: any = {
createEmbeddedView: function() {
createEmbeddedView: function () {
return getView();
},
indexOf: function() {
indexOf: function () {
return 0;
}
};
Expand All @@ -570,10 +570,10 @@ describe('VirtualScroll', () => {
left: ''
},
classList: {
add: function(){},
remove: function(){}
add: function () { },
remove: function () { }
},
setAttribute: function(){},
setAttribute: function () { },
innerHTML: '',
}]
};
Expand Down
Loading