Permalink
Browse files

fix(virtual-repeat): Changing the items to emty array from scrollable…

… list
  • Loading branch information...
fragsalat committed Sep 26, 2017
1 parent 4fbea6b commit 0e0b7027bbef48be4490c8c7674db211fe4c9cf5
Showing with 12 additions and 9 deletions.
  1. +4 −1 src/virtual-repeat.js
  2. +8 −8 test/virtual-repeat-integration.spec.js
View
@@ -113,6 +113,9 @@ export class VirtualRepeat extends AbstractRepeater {
bind(bindingContext, overrideContext): void {
this.scope = { bindingContext, overrideContext };
if (this._isAttached) {
this.itemsChanged();
}
}
call(context, changes): void {
@@ -454,7 +457,7 @@ export class VirtualRepeat extends AbstractRepeater {
}
_calcInitialHeights(itemsLength: number): void {
if (this._viewsLength > 0 && this._itemsLength === itemsLength || itemsLength <= 0) {
if (this._viewsLength > 0 && this._itemsLength === itemsLength || this._viewsLength > 0 && itemsLength < 0) {

This comment has been minimized.

Show comment
Hide comment
@Lee-Nover

Lee-Nover Nov 2, 2017

this change lets the code execute when there are no views, so this line throws an exception "var firstViewElement = this.view(0).lastChild".

@Lee-Nover

Lee-Nover Nov 2, 2017

this change lets the code execute when there are no views, so this line throws an exception "var firstViewElement = this.view(0).lastChild".

This comment has been minimized.

Show comment
Hide comment
@fragsalat

fragsalat Nov 3, 2017

Contributor

I've placed it on my tasklist to review this.

@fragsalat

fragsalat Nov 3, 2017

Contributor

I've placed it on my tasklist to review this.

This comment has been minimized.

Show comment
Hide comment
@djedi

djedi Nov 28, 2017

This also broke our stuff so we had to roll back to the previous version

@djedi

djedi Nov 28, 2017

This also broke our stuff so we had to roll back to the previous version

This comment has been minimized.

Show comment
Hide comment
@avrahamcool

avrahamcool Dec 6, 2017

damaged our work as well.. fixing this issue will be greatly appreciated.

@avrahamcool

avrahamcool Dec 6, 2017

damaged our work as well.. fixing this issue will be greatly appreciated.

This comment has been minimized.

Show comment
Hide comment
@fragsalat

fragsalat Dec 18, 2017

Contributor

Can you tell me how to reproduce this?

<div virtual-repeat.for="item of items"></div>

This is not causing any errors or what do you mean with no views?

@fragsalat

fragsalat Dec 18, 2017

Contributor

Can you tell me how to reproduce this?

<div virtual-repeat.for="item of items"></div>

This is not causing any errors or what do you mean with no views?

This comment has been minimized.

Show comment
Hide comment
@Lee-Nover

Lee-Nover Dec 18, 2017

simplest case to reproduce the problem is to create a new app using aurelia cli and install aurelia-ui-virtualization, then modify app.ts/js and app.html
app.ts

class LicenseInfo {
  name: string;
  constructor(name: string) {
    this.name = name;
  }
}

export class App {
  FilteredLicenses: LicenseInfo[] = [];
  
  addItem(name: string) {
    this.FilteredLicenses.push(new LicenseInfo(name));
  }
}

app.html

<template>
  <div virtual-repeat.for="item of FilteredLicenses">${item.name}</div>
</template>

and this is the error stack:

Unhandled rejection TypeError: Cannot read property 'lastChild' of undefined
vendor-bundle.js:1398
    at VirtualRepeat._calcInitialHeights (http://localhost:9000/scripts/vendor-bundle.js:28267:42)
    at VirtualRepeat.itemsChanged (http://localhost:9000/scripts/vendor-bundle.js:27973:12)
    at VirtualRepeat.attached (http://localhost:9000/scripts/vendor-bundle.js:27886:12)
    at Controller.attached (http://localhost:9000/scripts/vendor-bundle.js:22229:24)
    at View.attached (http://localhost:9000/scripts/vendor-bundle.js:20279:24)
    at ViewSlot.attached (http://localhost:9000/scripts/vendor-bundle.js:20637:15)
@Lee-Nover

Lee-Nover Dec 18, 2017

simplest case to reproduce the problem is to create a new app using aurelia cli and install aurelia-ui-virtualization, then modify app.ts/js and app.html
app.ts

class LicenseInfo {
  name: string;
  constructor(name: string) {
    this.name = name;
  }
}

export class App {
  FilteredLicenses: LicenseInfo[] = [];
  
  addItem(name: string) {
    this.FilteredLicenses.push(new LicenseInfo(name));
  }
}

app.html

<template>
  <div virtual-repeat.for="item of FilteredLicenses">${item.name}</div>
</template>

and this is the error stack:

Unhandled rejection TypeError: Cannot read property 'lastChild' of undefined
vendor-bundle.js:1398
    at VirtualRepeat._calcInitialHeights (http://localhost:9000/scripts/vendor-bundle.js:28267:42)
    at VirtualRepeat.itemsChanged (http://localhost:9000/scripts/vendor-bundle.js:27973:12)
    at VirtualRepeat.attached (http://localhost:9000/scripts/vendor-bundle.js:27886:12)
    at Controller.attached (http://localhost:9000/scripts/vendor-bundle.js:22229:24)
    at View.attached (http://localhost:9000/scripts/vendor-bundle.js:20279:24)
    at ViewSlot.attached (http://localhost:9000/scripts/vendor-bundle.js:20637:15)

This comment has been minimized.

Show comment
Hide comment
@fragsalat

fragsalat Dec 18, 2017

Contributor

Thx. I think I fixed it with an additional checkup. Actually the code is not really transparent what the if should cover.
Created a PR which should fix this #122

@fragsalat

fragsalat Dec 18, 2017

Contributor

Thx. I think I fixed it with an additional checkup. Actually the code is not really transparent what the if should cover.
Created a PR which should fix this #122

This comment has been minimized.

Show comment
Hide comment
@Lee-Nover

Lee-Nover Dec 18, 2017

This seems to be working. Thank you!

@Lee-Nover

Lee-Nover Dec 18, 2017

This seems to be working. Thank you!

return;
}
this._hasCalculatedSizes = true;
@@ -177,15 +177,15 @@ describe('VirtualRepeat Integration', () => {
}
function validateArrayChange(virtualRepeat, viewModel, done) {
viewModel.items = ['Foo B: 1', 'Foo B: 2', 'Foo B: 3', 'Foo B: 4', 'Foo B: 5'];
const createItems = (name, amount) => new Array(amount).map((v, index) => name + index);
viewModel.items = createItems('A', 4);
nq(() => validateState(virtualRepeat, viewModel));
nq(() => {
let newArr = [];
for (let i = 0; i < 100; i++) {
newArr.push('Foo C: ' + i);
}
viewModel.items = newArr;
});
nq(() => viewModel.items = createItems('B', 0));
nq(() => validateState(virtualRepeat, viewModel));
nq(() => viewModel.items = createItems('C', 101));
nq(() => validateState(virtualRepeat, viewModel));
nq(() => viewModel.items = createItems('D', 0));
nq(() => validateState(virtualRepeat, viewModel));
nq(() => done());
}

0 comments on commit 0e0b702

Please sign in to comment.