From d4e923f9de02168e2e049cf83932aaf41a0ac8cc Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Sat, 24 Jul 2021 13:42:57 +0530 Subject: [PATCH 01/29] Remove code from loadList We don't need this method so best to leave it empty. Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index 4426d97c..b6b4b0d1 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -168,6 +168,8 @@ export class LazyListView extends ListView { mount() { const root = super.mount(); + this._subscription = this._list.subscribe(this); + this._childInstances = []; this._parent = el("div", {className: "LazyListParent"}, root); /* Hooking to scroll events can be expensive. @@ -181,17 +183,12 @@ export class LazyListView extends ListView { update(attributes) { this._renderRange = null; super.update(attributes); + this._childInstances = []; this._initialRender(); } loadList() { - if (!this._list) { return; } - this._subscription = this._list.subscribe(this); - this._childInstances = []; - /* - super.loadList() would render the entire list at this point. - We instead lazy render a part of the list in _renderIfNeeded - */ + // We don't render the entire list; so nothing to see here. } _removeChild(child) { From 1a28b4f887eb49841e53e5ee078aa9bc1f6d93f8 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Sun, 25 Jul 2021 19:48:51 +0530 Subject: [PATCH 02/29] WIP --- src/platform/web/ui/general/LazyListView.js | 38 +++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index b6b4b0d1..a8c686eb 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -19,6 +19,11 @@ import {mountView} from "./utils"; import {ListView} from "./ListView"; import {insertAt} from "./utils"; +class ScrollDirection { + static get downwards() { return 1; } + static get upwards() { return -1; } +} + class ItemRange { constructor(topCount, renderCount, bottomCount) { this.topCount = topCount; @@ -56,6 +61,10 @@ class ItemRange { ); } + get lastIndex() { + return this.topCount + this.renderCount; + } + totalSize() { return this.topCount + this.renderCount + this.bottomCount; } @@ -69,6 +78,23 @@ class ItemRange { */ return idx - this.topCount; } + + scrollDirectionTo(range) { + return range.bottomCount < this.bottomCount ? ScrollDirection.downwards : ScrollDirection.upwards; + } + + diff(range) { + const diffResult = {}; + if (this.scrollDirectionTo(range) === ScrollDirection.downwards) { + diffResult.toRemove = { start: this.topCount, end: range.topCount - 1 }; + diffResult.toAdd = { start: this.lastIndex + 1, end: range.lastIndex }; + } + else { + diffResult.toRemove = { start: range.lastIndex + 1, end: this.lastIndex }; + diffResult.toAdd = { start: range.topCount, end: this.topCount - 1 }; + } + return diffResult; + } } export class LazyListView extends ListView { @@ -101,6 +127,9 @@ export class LazyListView extends ListView { // only update render Range if the new range + overflowMargin isn't contained by the old anymore // or if we are force rendering if (forceRender || !this._renderRange.contains(intersectRange)) { + console.log("new", renderRange); + console.log("current", this._renderRange); + console.log("diff", this._renderRange.diff(renderRange)); this._renderRange = renderRange; this._renderElementsInRange(); } @@ -146,13 +175,18 @@ export class LazyListView extends ListView { return null; } - _renderElementsInRange() { + _adjustPadding() { + + } + + _renderElementsInRange(range) { const { topCount, renderCount, bottomCount } = this._renderRange; + const renderedItems = this._itemsFromList(topCount, topCount + renderCount); const paddingTop = topCount * this._itemHeight; const paddingBottom = bottomCount * this._itemHeight; - const renderedItems = this._itemsFromList(topCount, topCount + renderCount); this._root.style.paddingTop = `${paddingTop}px`; this._root.style.paddingBottom = `${paddingBottom}px`; + for (const child of this._childInstances) { this._removeChild(child); } From 61402e798e878da08d5b3d4e6873f047790bd67a Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Tue, 27 Jul 2021 13:24:36 +0530 Subject: [PATCH 03/29] WIP 2 --- src/platform/web/ui/general/LazyListView.js | 88 +++++++++++++++------ 1 file changed, 64 insertions(+), 24 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index a8c686eb..299e511c 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -84,16 +84,17 @@ class ItemRange { } diff(range) { - const diffResult = {}; - if (this.scrollDirectionTo(range) === ScrollDirection.downwards) { - diffResult.toRemove = { start: this.topCount, end: range.topCount - 1 }; - diffResult.toAdd = { start: this.lastIndex + 1, end: range.lastIndex }; + const scrollDirection = this.scrollDirectionTo(range); + let toRemove, toAdd; + if (scrollDirection === ScrollDirection.downwards) { + toRemove = { start: this.topCount, end: range.topCount - 1 }; + toAdd = { start: this.lastIndex, end: range.lastIndex }; } else { - diffResult.toRemove = { start: range.lastIndex + 1, end: this.lastIndex }; - diffResult.toAdd = { start: range.topCount, end: this.topCount - 1 }; + toRemove = { start: range.lastIndex, end: this.lastIndex }; + toAdd = { start: range.topCount, end: this.topCount - 1 }; } - return diffResult; + return { toRemove, toAdd, scrollDirection }; } } @@ -130,8 +131,7 @@ export class LazyListView extends ListView { console.log("new", renderRange); console.log("current", this._renderRange); console.log("diff", this._renderRange.diff(renderRange)); - this._renderRange = renderRange; - this._renderElementsInRange(); + this._renderElementsInRange(renderRange); } } @@ -149,7 +149,18 @@ export class LazyListView extends ListView { const range = this._getVisibleRange(); const renderRange = range.expand(this._overflowItems); this._renderRange = renderRange; - this._renderElementsInRange(); + + const { topCount, renderCount } = this._renderRange; + const renderedItems = this._itemsFromList(topCount, topCount + renderCount); + this._adjustPadding(renderRange); + this._childInstances = []; + const fragment = document.createDocumentFragment(); + for (const item of renderedItems) { + const view = this._childCreator(item); + this._childInstances.push(view); + fragment.appendChild(mountView(view, this._mountArgs)); + } + this._root.appendChild(fragment); } _itemsFromList(start, end) { @@ -175,29 +186,58 @@ export class LazyListView extends ListView { return null; } - _adjustPadding() { - - } - - _renderElementsInRange(range) { - const { topCount, renderCount, bottomCount } = this._renderRange; - const renderedItems = this._itemsFromList(topCount, topCount + renderCount); + _adjustPadding(range) { + const { topCount, bottomCount } = range; const paddingTop = topCount * this._itemHeight; const paddingBottom = bottomCount * this._itemHeight; this._root.style.paddingTop = `${paddingTop}px`; this._root.style.paddingBottom = `${paddingBottom}px`; + } - for (const child of this._childInstances) { - this._removeChild(child); - } - this._childInstances = []; + _renderedFragment(items, childInstanceModifier) { const fragment = document.createDocumentFragment(); - for (const item of renderedItems) { + for (const item of items) { const view = this._childCreator(item); - this._childInstances.push(view); + childInstanceModifier(view); fragment.appendChild(mountView(view, this._mountArgs)); } - this._root.appendChild(fragment); + return fragment; + } + + _renderElementsInRange(range) { + const diff = this._renderRange.diff(range); + const {start, end} = diff.toAdd; + const renderedItems = this._itemsFromList(start, end); + this._adjustPadding(range); + + if (diff.scrollDirection === ScrollDirection.downwards) { + const {start, end} = diff.toRemove; + this._childInstances.splice(0, end - start + 1) + .forEach(child => this._removeChild(child)); + const fragment = this._renderedFragment(renderedItems, view => this._childInstances.push(view)); + this._root.appendChild(fragment); + } + else { + const {start, end} = diff.toRemove; + const normalizedStart = this._renderRange.normalize(start); + this._childInstances.splice(normalizedStart, end - start + 1) + .forEach(child => this._removeChild(child)); + const fragment = this._renderedFragment(renderedItems, view => this._childInstances.unshift(view)); + this._root.insertBefore(fragment, this._root.firstChild); + } + this._renderRange = range; + // for (const child of this._childInstances) { + // this._removeChild(child); + // } + // this._childInstances = []; + + // const fragment = document.createDocumentFragment(); + // for (const item of renderedItems) { + // const view = this._childCreator(item); + // this._childInstances.push(view); + // fragment.appendChild(mountView(view, this._mountArgs)); + // } + // this._root.appendChild(fragment); } mount() { From 168312627df2cc3512f7f986489e44ed8085f1c5 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 9 Aug 2021 14:21:42 +0530 Subject: [PATCH 04/29] Render only diff of ranges Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 52 ++++++++++++--------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index 299e511c..ffd834eb 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -83,18 +83,40 @@ class ItemRange { return range.bottomCount < this.bottomCount ? ScrollDirection.downwards : ScrollDirection.upwards; } + /** + * Check if this range intersects with another range + * @param {ItemRange} range The range to check against + * @param {ScrollDirection} scrollDirection + * @returns {Boolean} + */ + intersects(range) { + return !!Math.max(0, Math.min(this.lastIndex, range.lastIndex) - Math.max(this.topCount, range.topCount)); + } + diff(range) { + /** + * Range-1 + * |----------------------| + * Range-2 + * |---------------------| + * <-------><------------><-------> + * bisect-1 intersection bisect-2 + */ const scrollDirection = this.scrollDirectionTo(range); - let toRemove, toAdd; - if (scrollDirection === ScrollDirection.downwards) { - toRemove = { start: this.topCount, end: range.topCount - 1 }; - toAdd = { start: this.lastIndex, end: range.lastIndex }; + if (!this.intersects(range)) { + // There is no intersection between the ranges; which can happen if you scroll really fast + // In this case, we need to do full render of the new range + const toRemove = { start: this.topCount, end: this.lastIndex }; + const toAdd = { start: range.topCount, end: range.lastIndex }; + return {toRemove, toAdd, scrollDirection}; } - else { - toRemove = { start: range.lastIndex, end: this.lastIndex }; - toAdd = { start: range.topCount, end: this.topCount - 1 }; - } - return { toRemove, toAdd, scrollDirection }; + const bisection1 = {start: Math.min(this.topCount, range.topCount), end: Math.max(this.topCount, range.topCount) - 1}; + const bisection2 = {start: Math.min(this.lastIndex, range.lastIndex), end: Math.max(this.lastIndex, range.lastIndex)}; + // When scrolling down, bisection1 needs to be removed and bisection2 needs to be added + // When scrolling up, vice versa + const toRemove = scrollDirection === ScrollDirection.downwards ? bisection1 : bisection2; + const toAdd = scrollDirection === ScrollDirection.downwards ? bisection2 : bisection1; + return {toAdd, toRemove, scrollDirection}; } } @@ -226,18 +248,6 @@ export class LazyListView extends ListView { this._root.insertBefore(fragment, this._root.firstChild); } this._renderRange = range; - // for (const child of this._childInstances) { - // this._removeChild(child); - // } - // this._childInstances = []; - - // const fragment = document.createDocumentFragment(); - // for (const item of renderedItems) { - // const view = this._childCreator(item); - // this._childInstances.push(view); - // fragment.appendChild(mountView(view, this._mountArgs)); - // } - // this._root.appendChild(fragment); } mount() { From a02b6b68d3a81de9c5f06382a161cb3042bb34cf Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 9 Aug 2021 14:35:10 +0530 Subject: [PATCH 05/29] Move common code from if-else Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index ffd834eb..f0de677c 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -173,7 +173,7 @@ export class LazyListView extends ListView { this._renderRange = renderRange; const { topCount, renderCount } = this._renderRange; - const renderedItems = this._itemsFromList(topCount, topCount + renderCount); + const renderedItems = this._itemsFromList({ start: topCount, end: topCount + renderCount}); this._adjustPadding(renderRange); this._childInstances = []; const fragment = document.createDocumentFragment(); @@ -185,7 +185,7 @@ export class LazyListView extends ListView { this._root.appendChild(fragment); } - _itemsFromList(start, end) { + _itemsFromList({start, end}) { const array = []; let i = 0; for (const item of this._list) { @@ -228,22 +228,18 @@ export class LazyListView extends ListView { _renderElementsInRange(range) { const diff = this._renderRange.diff(range); - const {start, end} = diff.toAdd; - const renderedItems = this._itemsFromList(start, end); + const renderedItems = this._itemsFromList(diff.toAdd); this._adjustPadding(range); + const {start, end} = diff.toRemove; + const normalizedStart = this._renderRange.normalize(start); + this._childInstances.splice(normalizedStart, end - start + 1).forEach(child => this._removeChild(child)); + if (diff.scrollDirection === ScrollDirection.downwards) { - const {start, end} = diff.toRemove; - this._childInstances.splice(0, end - start + 1) - .forEach(child => this._removeChild(child)); const fragment = this._renderedFragment(renderedItems, view => this._childInstances.push(view)); this._root.appendChild(fragment); } else { - const {start, end} = diff.toRemove; - const normalizedStart = this._renderRange.normalize(start); - this._childInstances.splice(normalizedStart, end - start + 1) - .forEach(child => this._removeChild(child)); const fragment = this._renderedFragment(renderedItems, view => this._childInstances.unshift(view)); this._root.insertBefore(fragment, this._root.firstChild); } From 587dd3848e6517fb04c9eadcb091aaea3e275f53 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 9 Aug 2021 15:04:17 +0530 Subject: [PATCH 06/29] Use existing render function for initial render Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index f0de677c..aa38a4fb 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -168,21 +168,10 @@ export class LazyListView extends ListView { this._height = this._parent.clientHeight; if (this._height === 0) { console.error("LazyListView could not calculate parent height."); } - const range = this._getVisibleRange(); - const renderRange = range.expand(this._overflowItems); - this._renderRange = renderRange; - - const { topCount, renderCount } = this._renderRange; - const renderedItems = this._itemsFromList({ start: topCount, end: topCount + renderCount}); - this._adjustPadding(renderRange); - this._childInstances = []; - const fragment = document.createDocumentFragment(); - for (const item of renderedItems) { - const view = this._childCreator(item); - this._childInstances.push(view); - fragment.appendChild(mountView(view, this._mountArgs)); - } - this._root.appendChild(fragment); + const initialRange = this._getVisibleRange(); + const initialRenderRange = initialRange.expand(this._overflowItems); + this._renderRange = new ItemRange(0, 0, 0); + this._renderElementsInRange(initialRenderRange); } _itemsFromList({start, end}) { From 83ff2dd810066c263d9b07faa66efd7d97cc20f9 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Tue, 10 Aug 2021 14:37:33 +0530 Subject: [PATCH 07/29] Fix onAdd Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 29 +++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index aa38a4fb..c4a1c6b9 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -43,6 +43,8 @@ class ItemRange { } containsIndex(idx) { + // TODO: Replace by lastIndex + // TODO: Should idx be <= since lastIndex is not rendered? return idx >= this.topCount && idx <= (this.topCount + this.renderCount); } @@ -170,7 +172,7 @@ export class LazyListView extends ListView { if (this._height === 0) { console.error("LazyListView could not calculate parent height."); } const initialRange = this._getVisibleRange(); const initialRenderRange = initialRange.expand(this._overflowItems); - this._renderRange = new ItemRange(0, 0, 0); + this._renderRange = new ItemRange(0, 0, initialRange.bottomCount + 1); this._renderElementsInRange(initialRenderRange); } @@ -219,7 +221,6 @@ export class LazyListView extends ListView { const diff = this._renderRange.diff(range); const renderedItems = this._itemsFromList(diff.toAdd); this._adjustPadding(range); - const {start, end} = diff.toRemove; const normalizedStart = this._renderRange.normalize(start); this._childInstances.splice(normalizedStart, end - start + 1).forEach(child => this._removeChild(child)); @@ -265,9 +266,27 @@ export class LazyListView extends ListView { child.unmount(); } - // If size of the list changes, re-render - onAdd() { - this._renderIfNeeded(true); + onAdd(idx, value) { + const {topCount, renderCount, bottomCount} = this._renderRange; + if (this._renderRange.containsIndex(idx)) { + const normalizedIdx = this._renderRange.normalize(idx); + if (bottomCount === 0) { + // We're completely scrolled; so the extra element needs to be removed from top + this._removeChild(this._childInstances.shift()); + this._renderRange = new ItemRange(topCount + 1, renderCount, bottomCount); + } + else { + // Remove the last element, render the new element + this._removeChild(this._childInstances.pop()); + this._renderRange = new ItemRange(topCount, renderCount, bottomCount + 1); + } + super.onAdd(normalizedIdx, value); + } + else { + this._renderRange = idx < topCount ? new ItemRange(topCount + 1, renderCount, bottomCount): + new ItemRange(topCount, renderCount, bottomCount + 1); + } + this._adjustPadding(this._renderRange); } onRemove() { From 1165683f69badc40e4354eeb6729ac796a77d551 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Tue, 10 Aug 2021 18:44:00 +0530 Subject: [PATCH 08/29] Fix onRemove Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 25 +++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index c4a1c6b9..d94487ba 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -289,8 +289,29 @@ export class LazyListView extends ListView { this._adjustPadding(this._renderRange); } - onRemove() { - this._renderIfNeeded(true); + onRemove(idx, value) { + const {topCount, renderCount, bottomCount} = this._renderRange; + if (this._renderRange.containsIndex(idx)) { + const normalizedIdx = this._renderRange.normalize(idx); + super.onRemove(normalizedIdx, value); + if (bottomCount === 0) { + const child = this._childCreator(this._itemAtIndex(topCount - 1)); + this._childInstances.unshift(child); + this._root.insertBefore(mountView(child, this._mountArgs), this._root.firstChild); + this._renderRange = new ItemRange(topCount - 1, renderCount, bottomCount); + } + else { + const child = this._childCreator(this._itemAtIndex(this._renderRange.lastIndex - 1)); + this._childInstances.push(child); + this._root.appendChild(mountView(child, this._mountArgs)); + this._renderRange = new ItemRange(topCount, renderCount, bottomCount - 1); + } + } + else { + this._renderRange = idx < topCount ? new ItemRange(topCount - 1, renderCount, bottomCount): + new ItemRange(topCount, renderCount, bottomCount - 1); + } + this._adjustPadding(this._renderRange); } onUpdate(idx, value, params) { From 3ae52ea1ca0c37767d9703ca8bbf49ec9930a21f Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Wed, 11 Aug 2021 12:23:26 +0530 Subject: [PATCH 09/29] Fix bug in onAdd and onRemove Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index d94487ba..b0d28c51 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -271,9 +271,15 @@ export class LazyListView extends ListView { if (this._renderRange.containsIndex(idx)) { const normalizedIdx = this._renderRange.normalize(idx); if (bottomCount === 0) { - // We're completely scrolled; so the extra element needs to be removed from top - this._removeChild(this._childInstances.shift()); - this._renderRange = new ItemRange(topCount + 1, renderCount, bottomCount); + /* + If we're at the bottom of the list, we need to render the additional item + without removing another item from the list. + We can't increment topCount because the index topCount is not affected by the + add operation (and any modification will thus break ItemRange.normalize()). + We can't increment bottomCount because there's not enough items left to trigger + a further render. + */ + this._renderRange = new ItemRange(topCount, renderCount + 1, bottomCount); } else { // Remove the last element, render the new element @@ -295,10 +301,8 @@ export class LazyListView extends ListView { const normalizedIdx = this._renderRange.normalize(idx); super.onRemove(normalizedIdx, value); if (bottomCount === 0) { - const child = this._childCreator(this._itemAtIndex(topCount - 1)); - this._childInstances.unshift(child); - this._root.insertBefore(mountView(child, this._mountArgs), this._root.firstChild); - this._renderRange = new ItemRange(topCount - 1, renderCount, bottomCount); + // See onAdd for explanation + this._renderRange = new ItemRange(topCount, renderCount - 1, bottomCount); } else { const child = this._childCreator(this._itemAtIndex(this._renderRange.lastIndex - 1)); From e10b494f0ca859aaa27b53e999b4205490925346 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Wed, 11 Aug 2021 12:30:38 +0530 Subject: [PATCH 10/29] Improve containsIndex Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index b0d28c51..2c070d95 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -43,9 +43,7 @@ class ItemRange { } containsIndex(idx) { - // TODO: Replace by lastIndex - // TODO: Should idx be <= since lastIndex is not rendered? - return idx >= this.topCount && idx <= (this.topCount + this.renderCount); + return idx >= this.topCount && idx < this.lastIndex; } expand(amount) { From da715c70b0ec3b5f53b0db6ff437c3cda358f8b2 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Wed, 11 Aug 2021 12:37:07 +0530 Subject: [PATCH 11/29] Remove forceRender Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index 2c070d95..00e9b9d8 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -139,17 +139,12 @@ export class LazyListView extends ListView { return new ItemRange(topCount, renderCount, bottomCount); } - _renderIfNeeded(forceRender = false) { - /* - forceRender only because we don't optimize onAdd/onRemove yet. - Ideally, onAdd/onRemove should only render whatever has changed + update padding + update renderRange - */ + _renderIfNeeded() { const range = this._getVisibleRange(); const intersectRange = range.expand(this._overflowMargin); const renderRange = range.expand(this._overflowItems); // only update render Range if the new range + overflowMargin isn't contained by the old anymore - // or if we are force rendering - if (forceRender || !this._renderRange.contains(intersectRange)) { + if (!this._renderRange.contains(intersectRange)) { console.log("new", renderRange); console.log("current", this._renderRange); console.log("diff", this._renderRange.diff(renderRange)); From aee135a6cdaaad5a3a9a52fdf5da9e8f48f66da6 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Wed, 11 Aug 2021 12:45:45 +0530 Subject: [PATCH 12/29] Jsdoc fix Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index 00e9b9d8..f1fc533e 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -87,7 +87,7 @@ class ItemRange { * Check if this range intersects with another range * @param {ItemRange} range The range to check against * @param {ScrollDirection} scrollDirection - * @returns {Boolean} + * @returns {boolean} */ intersects(range) { return !!Math.max(0, Math.min(this.lastIndex, range.lastIndex) - Math.max(this.topCount, range.topCount)); From 5d54285640934b5a3cb8b728de684d2140e622c7 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Wed, 11 Aug 2021 12:52:38 +0530 Subject: [PATCH 13/29] Move ItemRange to separate file Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/ItemRange.js | 116 ++++++++++++++++++++ src/platform/web/ui/general/LazyListView.js | 102 +---------------- 2 files changed, 117 insertions(+), 101 deletions(-) create mode 100644 src/platform/web/ui/general/ItemRange.js diff --git a/src/platform/web/ui/general/ItemRange.js b/src/platform/web/ui/general/ItemRange.js new file mode 100644 index 00000000..ab6b23b0 --- /dev/null +++ b/src/platform/web/ui/general/ItemRange.js @@ -0,0 +1,116 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +export class ScrollDirection { + static get downwards() { return 1; } + static get upwards() { return -1; } +} + +export class ItemRange { + constructor(topCount, renderCount, bottomCount) { + this.topCount = topCount; + this.renderCount = renderCount; + this.bottomCount = bottomCount; + } + + contains(range) { + // don't contain empty ranges + // as it will prevent clearing the list + // once it is scrolled far enough out of view + if (!range.renderCount && this.renderCount) { + return false; + } + return range.topCount >= this.topCount && + (range.topCount + range.renderCount) <= (this.topCount + this.renderCount); + } + + containsIndex(idx) { + return idx >= this.topCount && idx < this.lastIndex; + } + + expand(amount) { + // don't expand ranges that won't render anything + if (this.renderCount === 0) { + return this; + } + + const topGrow = Math.min(amount, this.topCount); + const bottomGrow = Math.min(amount, this.bottomCount); + return new ItemRange( + this.topCount - topGrow, + this.renderCount + topGrow + bottomGrow, + this.bottomCount - bottomGrow, + ); + } + + get lastIndex() { + return this.topCount + this.renderCount; + } + + totalSize() { + return this.topCount + this.renderCount + this.bottomCount; + } + + normalize(idx) { + /* + map index from list to index in rendered range + eg: if the index range of this._list is [0, 200] and we have rendered + elements in range [50, 60] then index 50 in list must map to index 0 + in DOM tree/childInstance array. + */ + return idx - this.topCount; + } + + scrollDirectionTo(range) { + return range.bottomCount < this.bottomCount ? ScrollDirection.downwards : ScrollDirection.upwards; + } + + /** + * Check if this range intersects with another range + * @param {ItemRange} range The range to check against + * @param {ScrollDirection} scrollDirection + * @returns {boolean} + */ + intersects(range) { + return !!Math.max(0, Math.min(this.lastIndex, range.lastIndex) - Math.max(this.topCount, range.topCount)); + } + + diff(range) { + /** + * Range-1 + * |----------------------| + * Range-2 + * |---------------------| + * <-------><------------><-------> + * bisect-1 intersection bisect-2 + */ + const scrollDirection = this.scrollDirectionTo(range); + if (!this.intersects(range)) { + // There is no intersection between the ranges; which can happen if you scroll really fast + // In this case, we need to do full render of the new range + const toRemove = { start: this.topCount, end: this.lastIndex }; + const toAdd = { start: range.topCount, end: range.lastIndex }; + return {toRemove, toAdd, scrollDirection}; + } + const bisection1 = {start: Math.min(this.topCount, range.topCount), end: Math.max(this.topCount, range.topCount) - 1}; + const bisection2 = {start: Math.min(this.lastIndex, range.lastIndex), end: Math.max(this.lastIndex, range.lastIndex)}; + // When scrolling down, bisection1 needs to be removed and bisection2 needs to be added + // When scrolling up, vice versa + const toRemove = scrollDirection === ScrollDirection.downwards ? bisection1 : bisection2; + const toAdd = scrollDirection === ScrollDirection.downwards ? bisection2 : bisection1; + return {toAdd, toRemove, scrollDirection}; + } +} diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index f1fc533e..6864172c 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -18,107 +18,7 @@ import {el} from "./html"; import {mountView} from "./utils"; import {ListView} from "./ListView"; import {insertAt} from "./utils"; - -class ScrollDirection { - static get downwards() { return 1; } - static get upwards() { return -1; } -} - -class ItemRange { - constructor(topCount, renderCount, bottomCount) { - this.topCount = topCount; - this.renderCount = renderCount; - this.bottomCount = bottomCount; - } - - contains(range) { - // don't contain empty ranges - // as it will prevent clearing the list - // once it is scrolled far enough out of view - if (!range.renderCount && this.renderCount) { - return false; - } - return range.topCount >= this.topCount && - (range.topCount + range.renderCount) <= (this.topCount + this.renderCount); - } - - containsIndex(idx) { - return idx >= this.topCount && idx < this.lastIndex; - } - - expand(amount) { - // don't expand ranges that won't render anything - if (this.renderCount === 0) { - return this; - } - - const topGrow = Math.min(amount, this.topCount); - const bottomGrow = Math.min(amount, this.bottomCount); - return new ItemRange( - this.topCount - topGrow, - this.renderCount + topGrow + bottomGrow, - this.bottomCount - bottomGrow, - ); - } - - get lastIndex() { - return this.topCount + this.renderCount; - } - - totalSize() { - return this.topCount + this.renderCount + this.bottomCount; - } - - normalize(idx) { - /* - map index from list to index in rendered range - eg: if the index range of this._list is [0, 200] and we have rendered - elements in range [50, 60] then index 50 in list must map to index 0 - in DOM tree/childInstance array. - */ - return idx - this.topCount; - } - - scrollDirectionTo(range) { - return range.bottomCount < this.bottomCount ? ScrollDirection.downwards : ScrollDirection.upwards; - } - - /** - * Check if this range intersects with another range - * @param {ItemRange} range The range to check against - * @param {ScrollDirection} scrollDirection - * @returns {boolean} - */ - intersects(range) { - return !!Math.max(0, Math.min(this.lastIndex, range.lastIndex) - Math.max(this.topCount, range.topCount)); - } - - diff(range) { - /** - * Range-1 - * |----------------------| - * Range-2 - * |---------------------| - * <-------><------------><-------> - * bisect-1 intersection bisect-2 - */ - const scrollDirection = this.scrollDirectionTo(range); - if (!this.intersects(range)) { - // There is no intersection between the ranges; which can happen if you scroll really fast - // In this case, we need to do full render of the new range - const toRemove = { start: this.topCount, end: this.lastIndex }; - const toAdd = { start: range.topCount, end: range.lastIndex }; - return {toRemove, toAdd, scrollDirection}; - } - const bisection1 = {start: Math.min(this.topCount, range.topCount), end: Math.max(this.topCount, range.topCount) - 1}; - const bisection2 = {start: Math.min(this.lastIndex, range.lastIndex), end: Math.max(this.lastIndex, range.lastIndex)}; - // When scrolling down, bisection1 needs to be removed and bisection2 needs to be added - // When scrolling up, vice versa - const toRemove = scrollDirection === ScrollDirection.downwards ? bisection1 : bisection2; - const toAdd = scrollDirection === ScrollDirection.downwards ? bisection2 : bisection1; - return {toAdd, toRemove, scrollDirection}; - } -} +import {ItemRange, ScrollDirection} from "./ItemRange.js"; export class LazyListView extends ListView { constructor({itemHeight, overflowMargin = 5, overflowItems = 20,...options}, childCreator) { From 33ac34b04e05eced0ffec18cf9f985e7e74d5e20 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Sun, 15 Aug 2021 20:48:51 +0530 Subject: [PATCH 14/29] Do not break onListChanged Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/LazyListView.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index 6864172c..c064a2b5 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -162,6 +162,7 @@ export class LazyListView extends ListView { onAdd(idx, value) { const {topCount, renderCount, bottomCount} = this._renderRange; if (this._renderRange.containsIndex(idx)) { + this.onBeforeListChanged(); const normalizedIdx = this._renderRange.normalize(idx); if (bottomCount === 0) { /* @@ -179,7 +180,8 @@ export class LazyListView extends ListView { this._removeChild(this._childInstances.pop()); this._renderRange = new ItemRange(topCount, renderCount, bottomCount + 1); } - super.onAdd(normalizedIdx, value); + super.onAdd(normalizedIdx, value, true); + this.onListChanged(); } else { this._renderRange = idx < topCount ? new ItemRange(topCount + 1, renderCount, bottomCount): @@ -191,8 +193,9 @@ export class LazyListView extends ListView { onRemove(idx, value) { const {topCount, renderCount, bottomCount} = this._renderRange; if (this._renderRange.containsIndex(idx)) { + this.onBeforeListChanged(); const normalizedIdx = this._renderRange.normalize(idx); - super.onRemove(normalizedIdx, value); + super.onRemove(normalizedIdx, value, true); if (bottomCount === 0) { // See onAdd for explanation this._renderRange = new ItemRange(topCount, renderCount - 1, bottomCount); @@ -203,6 +206,7 @@ export class LazyListView extends ListView { this._root.appendChild(mountView(child, this._mountArgs)); this._renderRange = new ItemRange(topCount, renderCount, bottomCount - 1); } + this.onListChanged(); } else { this._renderRange = idx < topCount ? new ItemRange(topCount - 1, renderCount, bottomCount): From bbeb909bdc233a2e874e5855ef69d70493db12d4 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Sun, 15 Aug 2021 21:07:40 +0530 Subject: [PATCH 15/29] Use createEnum Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/ItemRange.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/platform/web/ui/general/ItemRange.js b/src/platform/web/ui/general/ItemRange.js index ab6b23b0..53627aa4 100644 --- a/src/platform/web/ui/general/ItemRange.js +++ b/src/platform/web/ui/general/ItemRange.js @@ -14,10 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -export class ScrollDirection { - static get downwards() { return 1; } - static get upwards() { return -1; } -} +import {createEnum} from "../../../../utils/enum.js"; + +export const ScrollDirection = createEnum("upwards", "downwards"); export class ItemRange { constructor(topCount, renderCount, bottomCount) { From d625d57aa4ec89dbf4b37a1ea27f3a3c2989db73 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 16 Aug 2021 15:28:57 +0530 Subject: [PATCH 16/29] Fix lastIndex Signed-off-by: RMidhunSuresh --- src/platform/web/ui/general/ItemRange.js | 6 +++--- src/platform/web/ui/general/LazyListView.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/platform/web/ui/general/ItemRange.js b/src/platform/web/ui/general/ItemRange.js index 53627aa4..494f5dcb 100644 --- a/src/platform/web/ui/general/ItemRange.js +++ b/src/platform/web/ui/general/ItemRange.js @@ -37,7 +37,7 @@ export class ItemRange { } containsIndex(idx) { - return idx >= this.topCount && idx < this.lastIndex; + return idx >= this.topCount && idx <= this.lastIndex; } expand(amount) { @@ -56,7 +56,7 @@ export class ItemRange { } get lastIndex() { - return this.topCount + this.renderCount; + return this.topCount + this.renderCount - 1; } totalSize() { @@ -105,7 +105,7 @@ export class ItemRange { return {toRemove, toAdd, scrollDirection}; } const bisection1 = {start: Math.min(this.topCount, range.topCount), end: Math.max(this.topCount, range.topCount) - 1}; - const bisection2 = {start: Math.min(this.lastIndex, range.lastIndex), end: Math.max(this.lastIndex, range.lastIndex)}; + const bisection2 = {start: Math.min(this.lastIndex, range.lastIndex) + 1, end: Math.max(this.lastIndex, range.lastIndex)}; // When scrolling down, bisection1 needs to be removed and bisection2 needs to be added // When scrolling up, vice versa const toRemove = scrollDirection === ScrollDirection.downwards ? bisection1 : bisection2; diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/LazyListView.js index c064a2b5..3213ffee 100644 --- a/src/platform/web/ui/general/LazyListView.js +++ b/src/platform/web/ui/general/LazyListView.js @@ -73,7 +73,7 @@ export class LazyListView extends ListView { const array = []; let i = 0; for (const item of this._list) { - if (i >= start && i < end) { + if (i >= start && i <= end) { array.push(item); } i = i + 1; @@ -201,7 +201,7 @@ export class LazyListView extends ListView { this._renderRange = new ItemRange(topCount, renderCount - 1, bottomCount); } else { - const child = this._childCreator(this._itemAtIndex(this._renderRange.lastIndex - 1)); + const child = this._childCreator(this._itemAtIndex(this._renderRange.lastIndex)); this._childInstances.push(child); this._root.appendChild(mountView(child, this._mountArgs)); this._renderRange = new ItemRange(topCount, renderCount, bottomCount - 1); From 4a64d0ee171b442b3b4f1354b3ef641b2a25cd86 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 19 Nov 2021 22:49:46 +0100 Subject: [PATCH 17/29] WIP --- src/platform/web/ui/general/ItemRange.js | 115 ---------- src/platform/web/ui/general/ItemRange.ts | 210 ++++++++++++++++++ src/platform/web/ui/general/LazyListView.ts | 200 +++++++++++++++++ src/platform/web/ui/general/ListView.ts | 53 +++-- .../{LazyListView.js => foo-LazyListView.js} | 0 src/platform/web/ui/general/utils.ts | 4 + .../ui/session/rightpanel/MemberListView.js | 2 +- 7 files changed, 450 insertions(+), 134 deletions(-) delete mode 100644 src/platform/web/ui/general/ItemRange.js create mode 100644 src/platform/web/ui/general/ItemRange.ts create mode 100644 src/platform/web/ui/general/LazyListView.ts rename src/platform/web/ui/general/{LazyListView.js => foo-LazyListView.js} (100%) diff --git a/src/platform/web/ui/general/ItemRange.js b/src/platform/web/ui/general/ItemRange.js deleted file mode 100644 index 494f5dcb..00000000 --- a/src/platform/web/ui/general/ItemRange.js +++ /dev/null @@ -1,115 +0,0 @@ -/* -Copyright 2021 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import {createEnum} from "../../../../utils/enum.js"; - -export const ScrollDirection = createEnum("upwards", "downwards"); - -export class ItemRange { - constructor(topCount, renderCount, bottomCount) { - this.topCount = topCount; - this.renderCount = renderCount; - this.bottomCount = bottomCount; - } - - contains(range) { - // don't contain empty ranges - // as it will prevent clearing the list - // once it is scrolled far enough out of view - if (!range.renderCount && this.renderCount) { - return false; - } - return range.topCount >= this.topCount && - (range.topCount + range.renderCount) <= (this.topCount + this.renderCount); - } - - containsIndex(idx) { - return idx >= this.topCount && idx <= this.lastIndex; - } - - expand(amount) { - // don't expand ranges that won't render anything - if (this.renderCount === 0) { - return this; - } - - const topGrow = Math.min(amount, this.topCount); - const bottomGrow = Math.min(amount, this.bottomCount); - return new ItemRange( - this.topCount - topGrow, - this.renderCount + topGrow + bottomGrow, - this.bottomCount - bottomGrow, - ); - } - - get lastIndex() { - return this.topCount + this.renderCount - 1; - } - - totalSize() { - return this.topCount + this.renderCount + this.bottomCount; - } - - normalize(idx) { - /* - map index from list to index in rendered range - eg: if the index range of this._list is [0, 200] and we have rendered - elements in range [50, 60] then index 50 in list must map to index 0 - in DOM tree/childInstance array. - */ - return idx - this.topCount; - } - - scrollDirectionTo(range) { - return range.bottomCount < this.bottomCount ? ScrollDirection.downwards : ScrollDirection.upwards; - } - - /** - * Check if this range intersects with another range - * @param {ItemRange} range The range to check against - * @param {ScrollDirection} scrollDirection - * @returns {boolean} - */ - intersects(range) { - return !!Math.max(0, Math.min(this.lastIndex, range.lastIndex) - Math.max(this.topCount, range.topCount)); - } - - diff(range) { - /** - * Range-1 - * |----------------------| - * Range-2 - * |---------------------| - * <-------><------------><-------> - * bisect-1 intersection bisect-2 - */ - const scrollDirection = this.scrollDirectionTo(range); - if (!this.intersects(range)) { - // There is no intersection between the ranges; which can happen if you scroll really fast - // In this case, we need to do full render of the new range - const toRemove = { start: this.topCount, end: this.lastIndex }; - const toAdd = { start: range.topCount, end: range.lastIndex }; - return {toRemove, toAdd, scrollDirection}; - } - const bisection1 = {start: Math.min(this.topCount, range.topCount), end: Math.max(this.topCount, range.topCount) - 1}; - const bisection2 = {start: Math.min(this.lastIndex, range.lastIndex) + 1, end: Math.max(this.lastIndex, range.lastIndex)}; - // When scrolling down, bisection1 needs to be removed and bisection2 needs to be added - // When scrolling up, vice versa - const toRemove = scrollDirection === ScrollDirection.downwards ? bisection1 : bisection2; - const toAdd = scrollDirection === ScrollDirection.downwards ? bisection2 : bisection1; - return {toAdd, toRemove, scrollDirection}; - } -} diff --git a/src/platform/web/ui/general/ItemRange.ts b/src/platform/web/ui/general/ItemRange.ts new file mode 100644 index 00000000..66db6078 --- /dev/null +++ b/src/platform/web/ui/general/ItemRange.ts @@ -0,0 +1,210 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// start is included in the range, +// end is excluded, +// so [2, 2[ means an empty range +class Range { + constructor( + public readonly start: number, + public readonly end: number + ) {} + + get length() { + return this.end - this.start; + } + + contains(range: Range): boolean { + return range.start >= this.start && range.end <= this.end; + } + + containsIndex(idx: number): boolean { + return idx >= this.start && idx < this.end; + } + + intersects(range: Range): boolean { + return range.start < this.end && this.start < range.end; + } + + forEach(callback: ((i: number) => void)) { + for (let i = this.start; i < this.end; i += 1) { + callback(i); + } + } + + forEachInIterator(it: IterableIterator, callback: ((T, i: number) => void)) { + let i = 0; + for (i = 0; i < this.start; i += 1) { + it.next(); + } + for (i = 0; i < this.length; i += 1) { + const result = it.next(); + if (result.done) { + break; + } else { + callback(result.value, this.start + i); + } + } + } + + [Symbol.iterator](): Iterator { + return new RangeIterator(this); + } +} + +class RangeIterator implements Iterator { + private idx: number; + constructor(private readonly range: Range) { + this.idx = range.start - 1; + } + + next(): IteratorResult { + if (this.idx < (this.range.end - 1)) { + this.idx += 1; + return {value: this.idx, done: false}; + } else { + return {value: undefined, done: true}; + } + } +} + +export function tests() { + return { + "length": assert => { + const a = new Range(2, 5); + assert.equal(a.length, 3); + }, + "iterator": assert => { + assert.deepEqual(Array.from(new Range(2, 5)), [2, 3, 4]); + }, + "containsIndex": assert => { + const a = new Range(2, 5); + assert.equal(a.containsIndex(0), false); + assert.equal(a.containsIndex(1), false); + assert.equal(a.containsIndex(2), true); + assert.equal(a.containsIndex(3), true); + assert.equal(a.containsIndex(4), true); + assert.equal(a.containsIndex(5), false); + assert.equal(a.containsIndex(6), false); + }, + "intersects returns false for touching ranges": assert => { + const a = new Range(2, 5); + const b = new Range(5, 10); + assert.equal(a.intersects(b), false); + assert.equal(b.intersects(a), false); + }, + "intersects returns false": assert => { + const a = new Range(2, 5); + const b = new Range(50, 100); + assert.equal(a.intersects(b), false); + assert.equal(b.intersects(a), false); + }, + "intersects returns true for 1 overlapping item": assert => { + const a = new Range(2, 5); + const b = new Range(4, 10); + assert.equal(a.intersects(b), true); + assert.equal(b.intersects(a), true); + }, + "contains beyond left edge": assert => { + const a = new Range(2, 5); + const b = new Range(1, 3); + assert.equal(a.contains(b), false); + }, + "contains at left edge": assert => { + const a = new Range(2, 5); + const b = new Range(2, 3); + assert.equal(a.contains(b), true); + }, + "contains between edges": assert => { + const a = new Range(2, 5); + const b = new Range(3, 4); + assert.equal(a.contains(b), true); + }, + "contains at right edge": assert => { + const a = new Range(2, 5); + const b = new Range(3, 5); + assert.equal(a.contains(b), true); + }, + "contains beyond right edge": assert => { + const a = new Range(2, 5); + const b = new Range(4, 6); + assert.equal(a.contains(b), false); + }, + "contains for non-intersecting ranges": assert => { + const a = new Range(2, 5); + const b = new Range(5, 6); + assert.equal(a.contains(b), false); + }, + "forEachInIterator with more values available": assert => { + const callbackValues: {v: string, i: number}[] = []; + const values = ["a", "b", "c", "d", "e", "f"]; + const it = values[Symbol.iterator](); + new Range(2, 5).forEachInIterator(it, (v, i) => callbackValues.push({v, i})); + assert.deepEqual(callbackValues, [ + {v: "c", i: 2}, + {v: "d", i: 3}, + {v: "e", i: 4}, + ]); + }, + "forEachInIterator with fewer values available": assert => { + const callbackValues: {v: string, i: number}[] = []; + const values = ["a", "b", "c"]; + const it = values[Symbol.iterator](); + new Range(2, 5).forEachInIterator(it, (v, i) => callbackValues.push({v, i})); + assert.deepEqual(callbackValues, [ + {v: "c", i: 2}, + ]); + }, + }; +} + +export class ItemRange extends Range { + constructor( + start: number, + end: number, + public readonly totalLength: number + ) { + super(start, end); + } + + + expand(amount: number): ItemRange { + // don't expand ranges that won't render anything + if (this.length === 0) { + return this; + } + + const topGrow = Math.min(amount, this.start); + const bottomGrow = Math.min(amount, this.totalLength - this.end); + return new ItemRange( + this.start - topGrow, + this.end + topGrow + bottomGrow, + this.totalLength, + ); + } + + static fromViewport(listLength: number, itemHeight: number, listHeight: number, scrollTop: number) { + const topCount = Math.min(Math.max(0, Math.floor(scrollTop / itemHeight)), listLength); + const itemsAfterTop = listLength - topCount; + const visibleItems = listHeight !== 0 ? Math.ceil(listHeight / itemHeight) : 0; + const renderCount = Math.min(visibleItems, itemsAfterTop); + return new ItemRange(topCount, topCount + renderCount, listLength); + } + + missingFrom() { + + } +} diff --git a/src/platform/web/ui/general/LazyListView.ts b/src/platform/web/ui/general/LazyListView.ts new file mode 100644 index 00000000..65c53c2d --- /dev/null +++ b/src/platform/web/ui/general/LazyListView.ts @@ -0,0 +1,200 @@ +/* +Copyright 2020 Bruno Windels + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import {tag} from "./html"; +import {removeChildren, mountView} from "./utils"; +import {ItemRange} from "./ItemRange"; +import {ListView, IOptions as IParentOptions} from "./ListView"; +import {IView} from "./types"; + +export interface IOptions extends IParentOptions { + itemHeight: number; + overflowMargin?: number; + overflowItems?: number; +} + +export class LazyListView extends ListView { + private renderRange?: ItemRange; + private height?: number; + private itemHeight: number; + private overflowItems: number; + private scrollContainer?: Element; + + constructor( + {itemHeight, overflowMargin = 5, overflowItems = 20,...options}: IOptions, + childCreator: (value: T) => V + ) { + super(options, childCreator); + this.itemHeight = itemHeight; + this.overflowItems = overflowItems; + // TODO: this.overflowMargin = overflowMargin; + } + + handleEvent(e: Event) { + if (e.type === "scroll") { + this.handleScroll(); + } else { + super.handleEvent(e); + } + } + + handleScroll() { + const visibleRange = this._getVisibleRange(); + // don't contain empty ranges + // as it will prevent clearing the list + // once it is scrolled far enough out of view + if (visibleRange.length !== 0 && !this.renderRange!.contains(visibleRange)) { + const prevRenderRange = this.renderRange!; + this.renderRange = visibleRange.expand(this.overflowItems); + this.renderUpdate(prevRenderRange, this.renderRange); + } + } + + override async loadList() { + /* + Wait two frames for the return from mount() to be inserted into DOM. + This should be enough, but if this gives us trouble we can always use + MutationObserver. + */ + await new Promise(r => requestAnimationFrame(r)); + await new Promise(r => requestAnimationFrame(r)); + + if (!this._list) { + return; + } + const visibleRange = this._getVisibleRange(); + this.renderRange = visibleRange.expand(this.overflowItems); + this._childInstances = []; + this._subscription = this._list.subscribe(this); + this.reRenderFullRange(this.renderRange); + } + + private _getVisibleRange() { + const {clientHeight, scrollTop} = this.root()!; + if (clientHeight === 0) { + throw new Error("LazyListView height is 0"); + } + return ItemRange.fromViewport(this._list.length, this.itemHeight, clientHeight, scrollTop); + } + + private reRenderFullRange(range: ItemRange) { + removeChildren(this._listElement!); + const fragment = document.createDocumentFragment(); + const it = this._list[Symbol.iterator](); + this._childInstances!.length = 0; + range.forEachInIterator(it, item => { + const child = this._childCreator(item); + this._childInstances!.push(child); + fragment.appendChild(mountView(child, this._mountArgs)); + }); + this._listElement!.appendChild(fragment); + this.adjustPadding(range); + } + + private renderUpdate(prevRange: ItemRange, newRange: ItemRange) { + if (newRange.intersects(prevRange)) { + for (const idxInList of prevRange) { + // TODO: we need to make sure we keep childInstances in order so the indices lign up. + // Perhaps we should join both ranges and see in which range it appears and either add or remove? + if (!newRange.containsIndex(idxInList)) { + const localIdx = idxInList - prevRange.start; + this.removeChild(localIdx); + } + } + const addedRange = newRange.missingFrom(prevRange); + addedRange.forEachInIterator(this._list[Symbol.iterator](), (item, idxInList) => { + const localIdx = idxInList - newRange.start; + this.addChild(localIdx, item); + }); + this.adjustPadding(newRange); + } else { + this.reRenderFullRange(newRange); + } + } + + private adjustPadding(range: ItemRange) { + const paddingTop = range.start * this.itemHeight; + const paddingBottom = (range.totalLength - range.end) * this.itemHeight; + const style = this.scrollContainer!.style; + style.paddingTop = `${paddingTop}px`; + style.paddingBottom = `${paddingBottom}px`; + } + + mount() { + const listElement = super.mount(); + this.scrollContainer = tag.div({className: "LazyListParent"}, listElement); + /* + Hooking to scroll events can be expensive. + Do we need to do more (like event throttling)? + */ + this.scrollContainer.addEventListener("scroll", this); + return this.scrollContainer; + } + + unmount() { + this.root()!.removeEventListener("scroll", this); + this.scrollContainer = undefined; + super.unmount(); + } + + root(): Element | undefined { + return this.scrollContainer; + } + + private get _listElement(): Element | undefined { + return super.root(); + } + + onAdd(idx: number, value: T) { + // TODO: update totalLength in renderRange + const result = this.renderRange!.queryAdd(idx); + if (result.addIdx !== -1) { + this.addChild(result.addIdx, value); + } + if (result.removeIdx !== -1) { + this.removeChild(result.removeIdx); + } + } + + onRemove(idx: number, value: T) { + // TODO: update totalLength in renderRange + const result = this.renderRange!.queryRemove(idx); + if (result.removeIdx !== -1) { + this.removeChild(result.removeIdx); + } + if (result.addIdx !== -1) { + this.addChild(result.addIdx, value); + } + } + + onMove(fromIdx: number, toIdx: number, value: T) { + const result = this.renderRange!.queryMove(fromIdx, toIdx); + if (result.moveFromIdx !== -1 && result.moveToIdx !== -1) { + this.moveChild(result.moveFromIdx, result.moveToIdx); + } else if (result.removeIdx !== -1) { + this.removeChild(result.removeIdx); + } else if (result.addIdx !== -1) { + this.addChild(result.addIdx, value); + } + } + + onUpdate(i: number, value: T, params: any) { + const updateIdx = this.renderRange!.queryUpdate(i); + if (updateIdx !== -1) { + this.updateChild(updateIdx, value, params); + } + } +} diff --git a/src/platform/web/ui/general/ListView.ts b/src/platform/web/ui/general/ListView.ts index f76bd961..9d639098 100644 --- a/src/platform/web/ui/general/ListView.ts +++ b/src/platform/web/ui/general/ListView.ts @@ -17,10 +17,10 @@ limitations under the License. import {el} from "./html"; import {mountView, insertAt} from "./utils"; import {SubscriptionHandle} from "../../../../observable/BaseObservable"; -import {BaseObservableList as ObservableList} from "../../../../observable/list/BaseObservableList"; +import {BaseObservableList as ObservableList, IListObserver} from "../../../../observable/list/BaseObservableList"; import {IView, IMountArgs} from "./types"; -interface IOptions { +export interface IOptions { list: ObservableList, onItemClick?: (childView: V, evt: UIEvent) => void, className?: string, @@ -28,17 +28,17 @@ interface IOptions { parentProvidesUpdates?: boolean } -export class ListView implements IView { +export class ListView implements IView, IListObserver { private _onItemClick?: (childView: V, evt: UIEvent) => void; - private _list: ObservableList; private _className?: string; private _tagName: string; private _root?: Element; - private _subscription?: SubscriptionHandle; - private _childCreator: (value: T) => V; - private _childInstances?: V[]; - private _mountArgs: IMountArgs; + protected _subscription?: SubscriptionHandle; + protected _childCreator: (value: T) => V; + protected _mountArgs: IMountArgs; + protected _list: ObservableList; + protected _childInstances?: V[]; constructor( {list, onItemClick, className, tagName = "ul", parentProvidesUpdates = true}: IOptions, @@ -145,31 +145,48 @@ export class ListView implements IView { } onAdd(idx: number, value: T) { - const child = this._childCreator(value); - this._childInstances!.splice(idx, 0, child); - insertAt(this._root!, idx, mountView(child, this._mountArgs)); + this.addChild(idx, value); } onRemove(idx: number, value: T) { - const [child] = this._childInstances!.splice(idx, 1); + this.removeChild(idx); + } + + onMove(fromIdx: number, toIdx: number, value: T) { + this.moveChild(fromIdx, toIdx); + } + + onUpdate(i: number, value: T, params: any) { + this.updateChild(i, value, params); + } + + protected addChild(childIdx: number, value: T) { + const child = this._childCreator(value); + this._childInstances!.splice(childIdx, 0, child); + insertAt(this._root!, childIdx, mountView(child, this._mountArgs)); + } + + protected removeChild(childIdx: number) { + const [child] = this._childInstances!.splice(childIdx, 1); child.root()!.remove(); child.unmount(); } - onMove(fromIdx: number, toIdx: number, value: T) { - const [child] = this._childInstances!.splice(fromIdx, 1); - this._childInstances!.splice(toIdx, 0, child); + protected moveChild(fromChildIdx: number, toChildIdx: number) { + const [child] = this._childInstances!.splice(fromChildIdx, 1); + this._childInstances!.splice(toChildIdx, 0, child); child.root()!.remove(); - insertAt(this._root!, toIdx, child.root()! as Element); + insertAt(this._root!, toChildIdx, child.root()! as Element); } - onUpdate(i: number, value: T, params: any) { + protected updateChild(childIdx: number, value: T, params: any) { if (this._childInstances) { - const instance = this._childInstances![i]; + const instance = this._childInstances![childIdx]; instance && instance.update(value, params); } } + // TODO: is this the list or view index? protected recreateItem(index: number, value: T) { if (this._childInstances) { const child = this._childCreator(value); diff --git a/src/platform/web/ui/general/LazyListView.js b/src/platform/web/ui/general/foo-LazyListView.js similarity index 100% rename from src/platform/web/ui/general/LazyListView.js rename to src/platform/web/ui/general/foo-LazyListView.js diff --git a/src/platform/web/ui/general/utils.ts b/src/platform/web/ui/general/utils.ts index 7eb1d7f9..f8d407e9 100644 --- a/src/platform/web/ui/general/utils.ts +++ b/src/platform/web/ui/general/utils.ts @@ -50,3 +50,7 @@ export function insertAt(parentNode: Element, idx: number, childNode: Node): voi parentNode.insertBefore(childNode, nextDomNode); } } + +export function removeChildren(parentNode: Element): void { + parentNode.innerHTML = ''; +} diff --git a/src/platform/web/ui/session/rightpanel/MemberListView.js b/src/platform/web/ui/session/rightpanel/MemberListView.js index 6c3d252e..6096f919 100644 --- a/src/platform/web/ui/session/rightpanel/MemberListView.js +++ b/src/platform/web/ui/session/rightpanel/MemberListView.js @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {LazyListView} from "../../general/LazyListView.js"; +import {LazyListView} from "../../general/LazyListView"; import {MemberTileView} from "./MemberTileView.js"; export class MemberListView extends LazyListView{ From cf9f43ab9ebea291af1d6478c44b6f6d30293f74 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 22 Nov 2021 20:35:57 +0100 Subject: [PATCH 18/29] WIP2 --- src/observable/list/ObservableArray.js | 8 + src/platform/web/ui/general/LazyListView.ts | 92 ++-- src/platform/web/ui/general/ListRange.ts | 440 ++++++++++++++++++ .../web/ui/general/{ItemRange.ts => Range.ts} | 105 +++-- 4 files changed, 555 insertions(+), 90 deletions(-) create mode 100644 src/platform/web/ui/general/ListRange.ts rename src/platform/web/ui/general/{ItemRange.ts => Range.ts} (78%) diff --git a/src/observable/list/ObservableArray.js b/src/observable/list/ObservableArray.js index 5d9f5c12..053e7320 100644 --- a/src/observable/list/ObservableArray.js +++ b/src/observable/list/ObservableArray.js @@ -44,6 +44,14 @@ export class ObservableArray extends BaseObservableList { this.emitAdd(idx, item); } + move(fromIdx, toIdx) { + if (fromIdx < this._items.length && toIdx < this._items.length) { + const item = this._items.splice(fromIdx, 1); + this._items.splice(toIdx, 0, item); + this.emitMove(fromIdx, toIdx, item); + } + } + update(idx, item, params = null) { if (idx < this._items.length) { this._items[idx] = item; diff --git a/src/platform/web/ui/general/LazyListView.ts b/src/platform/web/ui/general/LazyListView.ts index 65c53c2d..f8aa80cc 100644 --- a/src/platform/web/ui/general/LazyListView.ts +++ b/src/platform/web/ui/general/LazyListView.ts @@ -16,7 +16,7 @@ limitations under the License. import {tag} from "./html"; import {removeChildren, mountView} from "./utils"; -import {ItemRange} from "./ItemRange"; +import {ListRange, ResultType, AddRemoveResult} from "./ListRange"; import {ListView, IOptions as IParentOptions} from "./ListView"; import {IView} from "./types"; @@ -27,11 +27,11 @@ export interface IOptions extends IParentOptions { } export class LazyListView extends ListView { - private renderRange?: ItemRange; + private renderRange?: ListRange; private height?: number; private itemHeight: number; private overflowItems: number; - private scrollContainer?: Element; + private scrollContainer?: HTMLElement; constructor( {itemHeight, overflowMargin = 5, overflowItems = 20,...options}: IOptions, @@ -87,10 +87,10 @@ export class LazyListView extends ListView { if (clientHeight === 0) { throw new Error("LazyListView height is 0"); } - return ItemRange.fromViewport(this._list.length, this.itemHeight, clientHeight, scrollTop); + return ListRange.fromViewport(this._list.length, this.itemHeight, clientHeight, scrollTop); } - private reRenderFullRange(range: ItemRange) { + private reRenderFullRange(range: ListRange) { removeChildren(this._listElement!); const fragment = document.createDocumentFragment(); const it = this._list[Symbol.iterator](); @@ -104,20 +104,22 @@ export class LazyListView extends ListView { this.adjustPadding(range); } - private renderUpdate(prevRange: ItemRange, newRange: ItemRange) { + private renderUpdate(prevRange: ListRange, newRange: ListRange) { if (newRange.intersects(prevRange)) { - for (const idxInList of prevRange) { - // TODO: we need to make sure we keep childInstances in order so the indices lign up. - // Perhaps we should join both ranges and see in which range it appears and either add or remove? + // remove children in reverse order so child index isn't affected by previous removals + for (const idxInList of prevRange.reverseIterable()) { if (!newRange.containsIndex(idxInList)) { const localIdx = idxInList - prevRange.start; this.removeChild(localIdx); } } - const addedRange = newRange.missingFrom(prevRange); - addedRange.forEachInIterator(this._list[Symbol.iterator](), (item, idxInList) => { - const localIdx = idxInList - newRange.start; - this.addChild(localIdx, item); + // use forEachInIterator instead of for loop as we need to advance + // the list iterator to the start of the range first + newRange.forEachInIterator(this._list[Symbol.iterator](), (item, idxInList) => { + if (!prevRange.containsIndex(idxInList)) { + const localIdx = idxInList - newRange.start; + this.addChild(localIdx, item); + } }); this.adjustPadding(newRange); } else { @@ -125,7 +127,7 @@ export class LazyListView extends ListView { } } - private adjustPadding(range: ItemRange) { + private adjustPadding(range: ListRange) { const paddingTop = range.start * this.itemHeight; const paddingBottom = (range.totalLength - range.end) * this.itemHeight; const style = this.scrollContainer!.style; @@ -135,11 +137,7 @@ export class LazyListView extends ListView { mount() { const listElement = super.mount(); - this.scrollContainer = tag.div({className: "LazyListParent"}, listElement); - /* - Hooking to scroll events can be expensive. - Do we need to do more (like event throttling)? - */ + this.scrollContainer = tag.div({className: "LazyListParent"}, listElement) as HTMLElement; this.scrollContainer.addEventListener("scroll", this); return this.scrollContainer; } @@ -159,42 +157,46 @@ export class LazyListView extends ListView { } onAdd(idx: number, value: T) { - // TODO: update totalLength in renderRange - const result = this.renderRange!.queryAdd(idx); - if (result.addIdx !== -1) { - this.addChild(result.addIdx, value); - } - if (result.removeIdx !== -1) { - this.removeChild(result.removeIdx); - } + const result = this.renderRange!.queryAdd(idx, value, this._list); + this.applyRemoveAddResult(result); } onRemove(idx: number, value: T) { - // TODO: update totalLength in renderRange - const result = this.renderRange!.queryRemove(idx); - if (result.removeIdx !== -1) { - this.removeChild(result.removeIdx); - } - if (result.addIdx !== -1) { - this.addChild(result.addIdx, value); - } + const result = this.renderRange!.queryRemove(idx, this._list); + this.applyRemoveAddResult(result); } onMove(fromIdx: number, toIdx: number, value: T) { - const result = this.renderRange!.queryMove(fromIdx, toIdx); - if (result.moveFromIdx !== -1 && result.moveToIdx !== -1) { - this.moveChild(result.moveFromIdx, result.moveToIdx); - } else if (result.removeIdx !== -1) { - this.removeChild(result.removeIdx); - } else if (result.addIdx !== -1) { - this.addChild(result.addIdx, value); + const result = this.renderRange!.queryMove(fromIdx, toIdx, value, this._list); + if (result) { + if (result.type === ResultType.Move) { + this.moveChild( + this.renderRange!.toLocalIndex(result.fromIdx), + this.renderRange!.toLocalIndex(result.toIdx) + ); + } else { + this.applyRemoveAddResult(result); + } } } onUpdate(i: number, value: T, params: any) { - const updateIdx = this.renderRange!.queryUpdate(i); - if (updateIdx !== -1) { - this.updateChild(updateIdx, value, params); + if (this.renderRange!.containsIndex(i)) { + this.updateChild(this.renderRange!.toLocalIndex(i), value, params); + } + } + + private applyRemoveAddResult(result: AddRemoveResult) { + // order is important here, the new range can have a different start + if (result.type === ResultType.Remove || result.type === ResultType.RemoveAndAdd) { + this.removeChild(this.renderRange!.toLocalIndex(result.removeIdx)); + } + if (result.newRange) { + this.renderRange = result.newRange; + this.adjustPadding(this.renderRange) + } + if (result.type === ResultType.Add || result.type === ResultType.RemoveAndAdd) { + this.addChild(this.renderRange!.toLocalIndex(result.addIdx), result.value); } } } diff --git a/src/platform/web/ui/general/ListRange.ts b/src/platform/web/ui/general/ListRange.ts new file mode 100644 index 00000000..7ccd5726 --- /dev/null +++ b/src/platform/web/ui/general/ListRange.ts @@ -0,0 +1,440 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import {Range, RangeZone} from "./Range"; + +function skipOnIterator(it: Iterator, pos: number): boolean { + let i = 0; + while (i < pos) { + i += 1; + if(it.next().done) { + return false; + } + } + return true; +} + +function getIteratorValueAtIdx(it: Iterator, idx: number): undefined | T { + if (skipOnIterator(it, idx)) { + const result = it.next(); + if (!result.done) { + return result.value; + } + } + return undefined; +} + +export enum ResultType { + Move, + Add, + Remove, + RemoveAndAdd, + UpdateRange +} + +export interface MoveResult { + type: ResultType.Move; + fromIdx: number; + toIdx: number +} + +interface AddResult { + type: ResultType.Add; + newRange?: ListRange; + /** the list index of an item to add */ + addIdx: number; + /** the value to add at addIdx */ + value: T +} + +interface RemoveResult { + type: ResultType.Remove; + newRange?: ListRange; + /** the list index of an item to remove, before the add or remove event has been taken into account */ + removeIdx: number; +} + +interface RemoveAndAddResult { + type: ResultType.RemoveAndAdd; + newRange?: ListRange; + /** the list index of an item to remove, before the add or remove event has been taken into account */ + removeIdx: number; + /** the list index of an item to add */ + addIdx: number; + /** the value to add at addIdx */ + value: T; +} + +interface UpdateRangeResult { + type: ResultType.UpdateRange; + newRange?: ListRange; +} + +export type AddRemoveResult = AddResult | RemoveResult | RemoveAndAddResult | UpdateRangeResult; + +export class ListRange extends Range { + constructor( + start: number, + end: number, + private _totalLength: number, + private _viewportItemCount: number = end - start + ) { + super(start, end); + } + + expand(amount: number): ListRange { + // don't expand ranges that won't render anything + if (this.length === 0) { + return this; + } + const newStart = Math.max(0, this.start - amount); + const newEnd = Math.min(this.totalLength, this.end + amount); + return new ListRange( + newStart, + newEnd, + this.totalLength, + this._viewportItemCount + ); + } + + get totalLength(): number { + return this._totalLength; + } + + get viewportItemCount(): number { + return this._viewportItemCount; + } + + static fromViewport(listLength: number, itemHeight: number, listHeight: number, scrollTop: number) { + const topCount = Math.min(Math.max(0, Math.floor(scrollTop / itemHeight)), listLength); + const itemsAfterTop = listLength - topCount; + const viewportItemCount = listHeight !== 0 ? Math.ceil(listHeight / itemHeight) : 0; + const renderCount = Math.min(viewportItemCount, itemsAfterTop); + return new ListRange(topCount, topCount + renderCount, listLength, viewportItemCount); + } + + queryAdd(idx: number, value: T, list: Iterable): AddRemoveResult { + const maxAddIdx = this.viewportItemCount > this.length ? this.end : this.end - 1; + if (idx <= maxAddIdx) { + // use maxAddIdx to allow to grow the range by one at a time + // if the viewport isn't filled yet + const addIdx = this.clampIndex(idx, this.maxAddIdx); + const addValue = addIdx === idx ? value : getIteratorValueAtIdx(list[Symbol.iterator](), addIdx)!; + return this.createAddResult(addIdx, addValue); + } else { + // if the add happened after the range, we only update the range with the new length + return {type: ResultType.UpdateRange, newRange: this.derive(1, 0)}; + } + } + + queryRemove(idx: number, list: Iterable): AddRemoveResult { + if (idx < this.end) { + const removeIdx = this.clampIndex(idx); + return this.createRemoveResult(removeIdx, list); + } else { + return {type: ResultType.UpdateRange, newRange: this.derive(-1, 0)}; + } + } + + queryMove(fromIdx: number, toIdx: number, value: T, list: Iterable): MoveResult | AddRemoveResult | undefined { + const fromZone = this.getIndexZone(fromIdx); + const toZone = this.getIndexZone(toIdx); + if (fromZone === toZone) { + if (fromZone === RangeZone.Before || fromZone === RangeZone.After) { + return; + } else if (fromZone === RangeZone.Inside) { + return {type: ResultType.Move, fromIdx, toIdx}; + } + } else { + // TODO + const addIdx = this.clampIndex(toIdx); + const removeIdx = this.clampIndex(fromIdx); + const addValue = addIdx === toIdx ? value : getIteratorValueAtIdx(list[Symbol.iterator](), addIdx)!; + return {type: ResultType.RemoveAndAdd, removeIdx, addIdx, value: addValue}; + } + } + + private createAddResult(addIdx: number, value: T): AddRemoveResult { + // if the view port isn't filled yet, we don't remove + if (this.viewportItemCount > this.length) { + return {type: ResultType.Add, addIdx, value, newRange: this.derive(1, 1)}; + } else { + const removeIdx = this.clampIndex(Number.MAX_SAFE_INTEGER); + return {type: ResultType.RemoveAndAdd, removeIdx, addIdx, value, newRange: this.derive(1, 0)}; + } + } + + private createRemoveResult(removeIdx: number, list: Iterable): AddRemoveResult { + if (this.end < this.totalLength) { + // we have items below the range, we can add one from there to fill the viewport + const addIdx = this.clampIndex(Number.MAX_SAFE_INTEGER); + // we assume the value has already been removed from the list, + // so we can just look up the next value which is already at the same idx + const value = getIteratorValueAtIdx(list[Symbol.iterator](), addIdx)!; + return {type: ResultType.RemoveAndAdd, removeIdx, value, addIdx, newRange: this.derive(-1, 0)}; + } else if (this.start !== 0) { + // move the range 1 item up so we still display a viewport full of items + const newRange = this.derive(-1, 0, 1); + const addIdx = newRange.start; + // we assume the value has already been removed from the list, + // so we can just look up the next value which is already at the same idx + const value = getIteratorValueAtIdx(list[Symbol.iterator](), addIdx)!; + return {type: ResultType.RemoveAndAdd, removeIdx, value, addIdx, newRange}; + } else { + // we can't add at the bottom nor top, already constrained + return {type: ResultType.Remove, removeIdx, newRange: this.derive(-1, 0)}; + } + } + + private derive(totalLengthInc: number, viewportItemCountDecr: number, startDecr: number = 0): ListRange { + return new ListRange( + this.start - startDecr, + this.end - startDecr + viewportItemCountDecr, + this.totalLength + totalLengthInc, + this.viewportItemCount + ); + } +} + +import {ObservableArray} from "../../../../observable/list/ObservableArray.js"; + +export function tests() { + return { + "fromViewport": assert => { + const range = ListRange.fromViewport(10, 20, 90, 30); + assert.equal(range.start, 1); + assert.equal(range.end, 6); + assert.equal(range.totalLength, 10); + }, + "fromViewport at end": assert => { + const itemHeight = 20; + const range = ListRange.fromViewport(10, itemHeight, 3 * itemHeight, 7 * itemHeight); + assert.equal(range.start, 7); + assert.equal(range.end, 10); + assert.equal(range.totalLength, 10); + }, + "fromViewport with not enough items to fill viewport": assert => { + const itemHeight = 20; + const range = ListRange.fromViewport(5, itemHeight, 8 * itemHeight, 0); + assert.equal(range.start, 0); + assert.equal(range.end, 5); + assert.equal(range.totalLength, 5); + assert.equal(range.length, 5); + assert.equal(range.viewportItemCount, 8); + }, + "expand at start of list": assert => { + const range = new ListRange(1, 5, 10); + const expanded = range.expand(2); + assert.equal(expanded.start, 0); + assert.equal(expanded.end, 7); + assert.equal(expanded.totalLength, 10); + assert.equal(expanded.length, 7); + }, + "expand at end of list": assert => { + const range = new ListRange(7, 9, 10); + const expanded = range.expand(2); + assert.equal(expanded.start, 5); + assert.equal(expanded.end, 10); + assert.equal(expanded.totalLength, 10); + assert.equal(expanded.length, 5); + }, + "expand in middle of list": assert => { + const range = new ListRange(4, 6, 10); + const expanded = range.expand(2); + assert.equal(expanded.start, 2); + assert.equal(expanded.end, 8); + assert.equal(expanded.totalLength, 10); + assert.equal(expanded.length, 6); + }, + "queryAdd with addition before range": assert => { + const list = new ObservableArray(["b", "c", "d", "e"]); + const range = new ListRange(1, 3, list.length); + let added = false; + list.subscribe({ + onAdd(idx, value) { + added = true; + const result = range.queryAdd(idx, value, list); + assert.deepEqual(result, { + type: ResultType.RemoveAndAdd, + removeIdx: 2, + addIdx: 1, + value: "b", + newRange: new ListRange(1, 3, 5) + }); + } + }); + list.insert(0, "a"); + assert(added); + }, + "queryAdd with addition within range": assert => { + const list = new ObservableArray(["a", "b", "d", "e"]); + const range = new ListRange(1, 3, list.length); + let added = false; + list.subscribe({ + onAdd(idx, value) { + added = true; + const result = range.queryAdd(idx, value, list); + assert.deepEqual(result, { + type: ResultType.RemoveAndAdd, + removeIdx: 2, + addIdx: 2, + value: "c", + newRange: new ListRange(1, 3, 5) + }); + } + }); + list.insert(2, "c"); + assert(added); + }, + "queryAdd with addition after range": assert => { + const list = new ObservableArray(["a", "b", "c", "d"]); + const range = new ListRange(1, 3, list.length); + let added = false; + list.subscribe({ + onAdd(idx, value) { + added = true; + const result = range.queryAdd(idx, value, list); + assert.deepEqual(result, { + type: ResultType.UpdateRange, + newRange: new ListRange(1, 3, 5) + }); + } + }); + list.insert(4, "e"); + assert(added); + }, + "queryAdd with too few items to fill viewport grows the range": assert => { + const list = new ObservableArray(["a", "b", "d"]); + const viewportItemCount = 4; + const range = new ListRange(0, 3, list.length, viewportItemCount); + let added = false; + list.subscribe({ + onAdd(idx, value) { + added = true; + const result = range.queryAdd(idx, value, list); + assert.deepEqual(result, { + type: ResultType.Add, + newRange: new ListRange(0, 4, 4), + addIdx: 2, + value: "c" + }); + } + }); + list.insert(2, "c"); + assert(added); + }, + "queryRemove with removal before range": assert => { + const list = new ObservableArray(["a", "b", "c", "d", "e"]); + const range = new ListRange(1, 3, list.length); + let removed = false; + list.subscribe({ + onRemove(idx) { + removed = true; + const result = range.queryRemove(idx, list); + assert.deepEqual(result, { + type: ResultType.RemoveAndAdd, + removeIdx: 1, + addIdx: 2, + value: "d", + newRange: new ListRange(1, 3, 4) + }); + } + }); + list.remove(0); + assert(removed); + }, + "queryRemove with removal within range": assert => { + const list = new ObservableArray(["a", "b", "c", "d", "e"]); + const range = new ListRange(1, 3, list.length); + let removed = false; + list.subscribe({ + onRemove(idx) { + removed = true; + const result = range.queryRemove(idx, list); + assert.deepEqual(result, { + type: ResultType.RemoveAndAdd, + removeIdx: 2, + addIdx: 2, + value: "d", + newRange: new ListRange(1, 3, 4) + }); + assert.equal(list.length, 4); + } + }); + list.remove(2); + assert(removed); + }, + "queryRemove with removal after range": assert => { + const list = new ObservableArray(["a", "b", "c", "d", "e"]); + const range = new ListRange(1, 3, list.length); + let removed = false; + list.subscribe({ + onRemove(idx) { + removed = true; + const result = range.queryRemove(idx, list); + assert.deepEqual(result, { + type: ResultType.UpdateRange, + newRange: new ListRange(1, 3, 4) + }); + } + }); + list.remove(3); + assert(removed); + }, + "queryRemove at bottom of range moves range one up": assert => { + const list = new ObservableArray(["a", "b", "c"]); + const range = new ListRange(1, 3, list.length); + let removed = false; + list.subscribe({ + onRemove(idx) { + removed = true; + const result = range.queryRemove(idx, list); + assert.deepEqual(result, { + newRange: new ListRange(0, 2, 2), + type: ResultType.RemoveAndAdd, + removeIdx: 2, + addIdx: 0, + value: "a" + }); + } + }); + list.remove(2); + assert(removed); + }, + + "queryMove with move inside range": assert => { + const list = new ObservableArray(["a", "b", "c", "d", "e"]); + const range = new ListRange(1, 4, list.length); + let moved = false; + list.subscribe({ + onMove(fromIdx, toIdx, value) { + moved = true; + const result = range.queryMove(fromIdx, toIdx, value, list); + assert.deepEqual(result, { + type: ResultType.Move, + fromIdx: 2, + toIdx: 3 + }); + } + }); + list.move(2, 3); + assert(moved); + }, + + }; +} + +// TODO: test with view larger than space needed by list diff --git a/src/platform/web/ui/general/ItemRange.ts b/src/platform/web/ui/general/Range.ts similarity index 78% rename from src/platform/web/ui/general/ItemRange.ts rename to src/platform/web/ui/general/Range.ts index 66db6078..16dc0a33 100644 --- a/src/platform/web/ui/general/ItemRange.ts +++ b/src/platform/web/ui/general/Range.ts @@ -17,7 +17,7 @@ limitations under the License. // start is included in the range, // end is excluded, // so [2, 2[ means an empty range -class Range { +export class Range { constructor( public readonly start: number, public readonly end: number @@ -35,14 +35,12 @@ class Range { return idx >= this.start && idx < this.end; } - intersects(range: Range): boolean { - return range.start < this.end && this.start < range.end; + toLocalIndex(idx: number) { + return idx - this.start; } - forEach(callback: ((i: number) => void)) { - for (let i = this.start; i < this.end; i += 1) { - callback(i); - } + intersects(range: Range): boolean { + return range.start < this.end && this.start < range.end; } forEachInIterator(it: IterableIterator, callback: ((T, i: number) => void)) { @@ -63,6 +61,30 @@ class Range { [Symbol.iterator](): Iterator { return new RangeIterator(this); } + + reverseIterable(): Iterable { + return new ReverseRangeIterator(this); + } + + clampIndex(idx: number, end = this.end - 1) { + return Math.min(Math.max(this.start, idx), end); + } + + getIndexZone(idx): RangeZone { + if (idx < this.start) { + return RangeZone.Before; + } else if (idx < this.end) { + return RangeZone.Inside; + } else { + return RangeZone.After; + } + } +} + +export enum RangeZone { + Before = 1, + Inside, + After } class RangeIterator implements Iterator { @@ -81,6 +103,26 @@ class RangeIterator implements Iterator { } } +class ReverseRangeIterator implements Iterable, Iterator { + private idx: number; + constructor(private readonly range: Range) { + this.idx = range.end; + } + + [Symbol.iterator]() { + return this; + } + + next(): IteratorResult { + if (this.idx > this.range.start) { + this.idx -= 1; + return {value: this.idx, done: false}; + } else { + return {value: undefined, done: true}; + } + } +} + export function tests() { return { "length": assert => { @@ -90,6 +132,9 @@ export function tests() { "iterator": assert => { assert.deepEqual(Array.from(new Range(2, 5)), [2, 3, 4]); }, + "reverseIterable": assert => { + assert.deepEqual(Array.from(new Range(2, 5).reverseIterable()), [4, 3, 2]); + }, "containsIndex": assert => { const a = new Range(2, 5); assert.equal(a.containsIndex(0), false); @@ -168,43 +213,13 @@ export function tests() { {v: "c", i: 2}, ]); }, + "clampIndex": assert => { + assert.equal(new Range(2, 5).clampIndex(0), 2); + assert.equal(new Range(2, 5).clampIndex(2), 2); + assert.equal(new Range(2, 5).clampIndex(3), 3); + assert.equal(new Range(2, 5).clampIndex(4), 4); + assert.equal(new Range(2, 5).clampIndex(5), 4); + assert.equal(new Range(2, 5).clampIndex(10), 4); + } }; } - -export class ItemRange extends Range { - constructor( - start: number, - end: number, - public readonly totalLength: number - ) { - super(start, end); - } - - - expand(amount: number): ItemRange { - // don't expand ranges that won't render anything - if (this.length === 0) { - return this; - } - - const topGrow = Math.min(amount, this.start); - const bottomGrow = Math.min(amount, this.totalLength - this.end); - return new ItemRange( - this.start - topGrow, - this.end + topGrow + bottomGrow, - this.totalLength, - ); - } - - static fromViewport(listLength: number, itemHeight: number, listHeight: number, scrollTop: number) { - const topCount = Math.min(Math.max(0, Math.floor(scrollTop / itemHeight)), listLength); - const itemsAfterTop = listLength - topCount; - const visibleItems = listHeight !== 0 ? Math.ceil(listHeight / itemHeight) : 0; - const renderCount = Math.min(visibleItems, itemsAfterTop); - return new ItemRange(topCount, topCount + renderCount, listLength); - } - - missingFrom() { - - } -} From 3aa3b7e1600e342cd3f02b1486c540ff86342e9b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 08:30:52 +0100 Subject: [PATCH 19/29] fix end growing larger than totalLength when range shrinks in case of remove --- src/platform/web/ui/general/ListRange.ts | 47 +++++++++++++++++------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/src/platform/web/ui/general/ListRange.ts b/src/platform/web/ui/general/ListRange.ts index 7ccd5726..a85a7323 100644 --- a/src/platform/web/ui/general/ListRange.ts +++ b/src/platform/web/ui/general/ListRange.ts @@ -131,12 +131,12 @@ export class ListRange extends Range { if (idx <= maxAddIdx) { // use maxAddIdx to allow to grow the range by one at a time // if the viewport isn't filled yet - const addIdx = this.clampIndex(idx, this.maxAddIdx); + const addIdx = this.clampIndex(idx, maxAddIdx); const addValue = addIdx === idx ? value : getIteratorValueAtIdx(list[Symbol.iterator](), addIdx)!; return this.createAddResult(addIdx, addValue); } else { // if the add happened after the range, we only update the range with the new length - return {type: ResultType.UpdateRange, newRange: this.derive(1, 0)}; + return {type: ResultType.UpdateRange, newRange: this.deriveRange(1, 0)}; } } @@ -145,7 +145,7 @@ export class ListRange extends Range { const removeIdx = this.clampIndex(idx); return this.createRemoveResult(removeIdx, list); } else { - return {type: ResultType.UpdateRange, newRange: this.derive(-1, 0)}; + return {type: ResultType.UpdateRange, newRange: this.deriveRange(-1, 0)}; } } @@ -170,10 +170,10 @@ export class ListRange extends Range { private createAddResult(addIdx: number, value: T): AddRemoveResult { // if the view port isn't filled yet, we don't remove if (this.viewportItemCount > this.length) { - return {type: ResultType.Add, addIdx, value, newRange: this.derive(1, 1)}; + return {type: ResultType.Add, addIdx, value, newRange: this.deriveRange(1, 1)}; } else { const removeIdx = this.clampIndex(Number.MAX_SAFE_INTEGER); - return {type: ResultType.RemoveAndAdd, removeIdx, addIdx, value, newRange: this.derive(1, 0)}; + return {type: ResultType.RemoveAndAdd, removeIdx, addIdx, value, newRange: this.deriveRange(1, 0)}; } } @@ -184,10 +184,10 @@ export class ListRange extends Range { // we assume the value has already been removed from the list, // so we can just look up the next value which is already at the same idx const value = getIteratorValueAtIdx(list[Symbol.iterator](), addIdx)!; - return {type: ResultType.RemoveAndAdd, removeIdx, value, addIdx, newRange: this.derive(-1, 0)}; + return {type: ResultType.RemoveAndAdd, removeIdx, value, addIdx, newRange: this.deriveRange(-1, 0)}; } else if (this.start !== 0) { // move the range 1 item up so we still display a viewport full of items - const newRange = this.derive(-1, 0, 1); + const newRange = this.deriveRange(-1, 0, 1); const addIdx = newRange.start; // we assume the value has already been removed from the list, // so we can just look up the next value which is already at the same idx @@ -195,15 +195,19 @@ export class ListRange extends Range { return {type: ResultType.RemoveAndAdd, removeIdx, value, addIdx, newRange}; } else { // we can't add at the bottom nor top, already constrained - return {type: ResultType.Remove, removeIdx, newRange: this.derive(-1, 0)}; + return {type: ResultType.Remove, removeIdx, newRange: this.deriveRange(-1, 0)}; } } - private derive(totalLengthInc: number, viewportItemCountDecr: number, startDecr: number = 0): ListRange { + private deriveRange(totalLengthInc: number, viewportItemCountDecr: number, startDecr: number = 0): ListRange { + const start = this.start - startDecr; + const totalLength = this.totalLength + totalLengthInc; + // prevent end being larger than totalLength + const end = Math.min(Math.max(start, this.end - startDecr + viewportItemCountDecr), totalLength); return new ListRange( - this.start - startDecr, - this.end - startDecr + viewportItemCountDecr, - this.totalLength + totalLengthInc, + start, + end, + totalLength, this.viewportItemCount ); } @@ -414,7 +418,24 @@ export function tests() { list.remove(2); assert(removed); }, - + "queryRemove with range on full length shrinks range": assert => { + const list = new ObservableArray(["a", "b", "c"]); + const range = new ListRange(0, 3, list.length); + let removed = false; + list.subscribe({ + onRemove(idx) { + removed = true; + const result = range.queryRemove(idx, list); + assert.deepEqual(result, { + newRange: new ListRange(0, 2, 2, 3), + type: ResultType.Remove, + removeIdx: 2, + }); + } + }); + list.remove(2); + assert(removed); + }, "queryMove with move inside range": assert => { const list = new ObservableArray(["a", "b", "c", "d", "e"]); const range = new ListRange(1, 4, list.length); From c22718811fa351e0dfa3e447673bffaebab1df6e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 08:56:33 +0100 Subject: [PATCH 20/29] more tests for queryMove --- src/observable/list/ObservableArray.js | 2 +- src/platform/web/ui/general/ListRange.ts | 97 +++++++++++++++++++++++- 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/src/observable/list/ObservableArray.js b/src/observable/list/ObservableArray.js index 053e7320..afdaba37 100644 --- a/src/observable/list/ObservableArray.js +++ b/src/observable/list/ObservableArray.js @@ -46,7 +46,7 @@ export class ObservableArray extends BaseObservableList { move(fromIdx, toIdx) { if (fromIdx < this._items.length && toIdx < this._items.length) { - const item = this._items.splice(fromIdx, 1); + const [item] = this._items.splice(fromIdx, 1); this._items.splice(toIdx, 0, item); this.emitMove(fromIdx, toIdx, item); } diff --git a/src/platform/web/ui/general/ListRange.ts b/src/platform/web/ui/general/ListRange.ts index a85a7323..3604e5f8 100644 --- a/src/platform/web/ui/general/ListRange.ts +++ b/src/platform/web/ui/general/ListRange.ts @@ -454,8 +454,101 @@ export function tests() { list.move(2, 3); assert(moved); }, + "queryMove with move from before to inside range": assert => { + const list = new ObservableArray(["a", "b", "c", "d", "e"]); + const range = new ListRange(2, 5, list.length); + let moved = false; + list.subscribe({ + onMove(fromIdx, toIdx, value) { + moved = true; + const result = range.queryMove(fromIdx, toIdx, value, list); + assert.deepEqual(result, { + type: ResultType.RemoveAndAdd, + removeIdx: 2, + addIdx: 3, + value: "a" + }); + } + }); + list.move(0, 3); // move "a" to after "d" + assert(moved); + }, + "queryMove with move from after to inside range": assert => { + const list = new ObservableArray(["a", "b", "c", "d", "e"]); + const range = new ListRange(0, 3, list.length); + let moved = false; + list.subscribe({ + onMove(fromIdx, toIdx, value) { + moved = true; + const result = range.queryMove(fromIdx, toIdx, value, list); + assert.deepEqual(result, { + type: ResultType.RemoveAndAdd, + removeIdx: 2, + addIdx: 1, + value: "e" + }); + } + }); + list.move(4, 1); // move "e" to before "b" + assert(moved); + }, + "queryMove with move inside range to after": assert => { + const list = new ObservableArray(["a", "b", "c", "d", "e"]); + const range = new ListRange(0, 3, list.length); + let moved = false; + list.subscribe({ + onMove(fromIdx, toIdx, value) { + moved = true; + const result = range.queryMove(fromIdx, toIdx, value, list); + assert.deepEqual(result, { + type: ResultType.RemoveAndAdd, + removeIdx: 1, + addIdx: 2, + value: "d" + }); + } + }); + list.move(1, 3); // move "b" to after "d" + assert(moved); + }, + "queryMove with move inside range to before": assert => { + const list = new ObservableArray(["a", "b", "c", "d", "e"]); + const range = new ListRange(2, 5, list.length); + let moved = false; + list.subscribe({ + onMove(fromIdx, toIdx, value) { + moved = true; + const result = range.queryMove(fromIdx, toIdx, value, list); + assert.deepEqual(result, { + type: ResultType.RemoveAndAdd, + removeIdx: 3, + addIdx: 2, + value: "b" + }); + } + }); + list.move(3, 0); // move "d" to before "a" + assert(moved); + }, + "queryMove with move from before range to after": assert => { + const list = new ObservableArray(["a", "b", "c", "d", "e"]); + const range = new ListRange(1, 4, list.length); + let moved = false; + list.subscribe({ + onMove(fromIdx, toIdx, value) { + moved = true; + const result = range.queryMove(fromIdx, toIdx, value, list); + assert.deepEqual(result, { + type: ResultType.RemoveAndAdd, + removeIdx: 1, + addIdx: 3, + value: "e" + }); + } + }); + list.move(0, 4); // move "a" to after "e" + assert(moved); + }, }; } - -// TODO: test with view larger than space needed by list From 7897ea88cd71ce4ea04cfa95119489a60811cefa Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 14:24:43 +0100 Subject: [PATCH 21/29] add some spaces and comments --- src/platform/web/ui/general/LazyListView.ts | 2 +- src/platform/web/ui/general/ListRange.ts | 2 ++ src/platform/web/ui/session/rightpanel/MemberListView.js | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.ts b/src/platform/web/ui/general/LazyListView.ts index f8aa80cc..2d8891ce 100644 --- a/src/platform/web/ui/general/LazyListView.ts +++ b/src/platform/web/ui/general/LazyListView.ts @@ -34,7 +34,7 @@ export class LazyListView extends ListView { private scrollContainer?: HTMLElement; constructor( - {itemHeight, overflowMargin = 5, overflowItems = 20,...options}: IOptions, + {itemHeight, overflowMargin = 5, overflowItems = 20, ...options}: IOptions, childCreator: (value: T) => V ) { super(options, childCreator); diff --git a/src/platform/web/ui/general/ListRange.ts b/src/platform/web/ui/general/ListRange.ts index 3604e5f8..2b70a987 100644 --- a/src/platform/web/ui/general/ListRange.ts +++ b/src/platform/web/ui/general/ListRange.ts @@ -67,6 +67,8 @@ interface RemoveResult { removeIdx: number; } +// need to repeat the fields from RemoveResult and AddResult here +// to make the discriminated union work interface RemoveAndAddResult { type: ResultType.RemoveAndAdd; newRange?: ListRange; diff --git a/src/platform/web/ui/session/rightpanel/MemberListView.js b/src/platform/web/ui/session/rightpanel/MemberListView.js index 6096f919..a4a4e78c 100644 --- a/src/platform/web/ui/session/rightpanel/MemberListView.js +++ b/src/platform/web/ui/session/rightpanel/MemberListView.js @@ -17,7 +17,7 @@ limitations under the License. import {LazyListView} from "../../general/LazyListView"; import {MemberTileView} from "./MemberTileView.js"; -export class MemberListView extends LazyListView{ +export class MemberListView extends LazyListView { constructor(vm) { super({ list: vm.memberTileViewModels, From c64a9c1e23d64f198bb688c776b880c8ab4394e3 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 14:25:00 +0100 Subject: [PATCH 22/29] snowpack/esbuild 0.9 doesn't support override keyword --- src/platform/web/ui/general/LazyListView.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/web/ui/general/LazyListView.ts b/src/platform/web/ui/general/LazyListView.ts index 2d8891ce..deceac08 100644 --- a/src/platform/web/ui/general/LazyListView.ts +++ b/src/platform/web/ui/general/LazyListView.ts @@ -63,7 +63,8 @@ export class LazyListView extends ListView { } } - override async loadList() { + // override + async loadList() { /* Wait two frames for the return from mount() to be inserted into DOM. This should be enough, but if this gives us trouble we can always use From 4be2f12a14ee8b571c19ac82af20067c25a66d4f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 14:25:22 +0100 Subject: [PATCH 23/29] subscribe before calling list.length --- src/platform/web/ui/general/LazyListView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/web/ui/general/LazyListView.ts b/src/platform/web/ui/general/LazyListView.ts index deceac08..a29e434c 100644 --- a/src/platform/web/ui/general/LazyListView.ts +++ b/src/platform/web/ui/general/LazyListView.ts @@ -76,10 +76,10 @@ export class LazyListView extends ListView { if (!this._list) { return; } + this._subscription = this._list.subscribe(this); const visibleRange = this._getVisibleRange(); this.renderRange = visibleRange.expand(this.overflowItems); this._childInstances = []; - this._subscription = this._list.subscribe(this); this.reRenderFullRange(this.renderRange); } From 9557178ffbb7303b1a74d3a46d03616e3256c9b5 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 14:25:35 +0100 Subject: [PATCH 24/29] padding needs to be on ul, not scroll container, or the list blows up --- src/platform/web/ui/general/LazyListView.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.ts b/src/platform/web/ui/general/LazyListView.ts index a29e434c..4686c5ea 100644 --- a/src/platform/web/ui/general/LazyListView.ts +++ b/src/platform/web/ui/general/LazyListView.ts @@ -131,7 +131,7 @@ export class LazyListView extends ListView { private adjustPadding(range: ListRange) { const paddingTop = range.start * this.itemHeight; const paddingBottom = (range.totalLength - range.end) * this.itemHeight; - const style = this.scrollContainer!.style; + const style = this._listElement!.style; style.paddingTop = `${paddingTop}px`; style.paddingBottom = `${paddingBottom}px`; } @@ -153,8 +153,8 @@ export class LazyListView extends ListView { return this.scrollContainer; } - private get _listElement(): Element | undefined { - return super.root(); + private get _listElement(): HTMLElement | undefined { + return super.root() as HTMLElement | undefined; } onAdd(idx: number, value: T) { From 35fb84c2759b34dc7ef26dfa894b8d4ab700373b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 14:26:15 +0100 Subject: [PATCH 25/29] remove old js lazylist --- .../web/ui/general/foo-LazyListView.js | 287 ------------------ 1 file changed, 287 deletions(-) delete mode 100644 src/platform/web/ui/general/foo-LazyListView.js diff --git a/src/platform/web/ui/general/foo-LazyListView.js b/src/platform/web/ui/general/foo-LazyListView.js deleted file mode 100644 index 3213ffee..00000000 --- a/src/platform/web/ui/general/foo-LazyListView.js +++ /dev/null @@ -1,287 +0,0 @@ -/* -Copyright 2021 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import {el} from "./html"; -import {mountView} from "./utils"; -import {ListView} from "./ListView"; -import {insertAt} from "./utils"; -import {ItemRange, ScrollDirection} from "./ItemRange.js"; - -export class LazyListView extends ListView { - constructor({itemHeight, overflowMargin = 5, overflowItems = 20,...options}, childCreator) { - super(options, childCreator); - this._itemHeight = itemHeight; - this._overflowMargin = overflowMargin; - this._overflowItems = overflowItems; - } - - _getVisibleRange() { - const length = this._list ? this._list.length : 0; - const scrollTop = this._parent.scrollTop; - const topCount = Math.min(Math.max(0, Math.floor(scrollTop / this._itemHeight)), length); - const itemsAfterTop = length - topCount; - const visibleItems = this._height !== 0 ? Math.ceil(this._height / this._itemHeight) : 0; - const renderCount = Math.min(visibleItems, itemsAfterTop); - const bottomCount = itemsAfterTop - renderCount; - return new ItemRange(topCount, renderCount, bottomCount); - } - - _renderIfNeeded() { - const range = this._getVisibleRange(); - const intersectRange = range.expand(this._overflowMargin); - const renderRange = range.expand(this._overflowItems); - // only update render Range if the new range + overflowMargin isn't contained by the old anymore - if (!this._renderRange.contains(intersectRange)) { - console.log("new", renderRange); - console.log("current", this._renderRange); - console.log("diff", this._renderRange.diff(renderRange)); - this._renderElementsInRange(renderRange); - } - } - - async _initialRender() { - /* - Wait two frames for the return from mount() to be inserted into DOM. - This should be enough, but if this gives us trouble we can always use - MutationObserver. - */ - await new Promise(r => requestAnimationFrame(r)); - await new Promise(r => requestAnimationFrame(r)); - - this._height = this._parent.clientHeight; - if (this._height === 0) { console.error("LazyListView could not calculate parent height."); } - const initialRange = this._getVisibleRange(); - const initialRenderRange = initialRange.expand(this._overflowItems); - this._renderRange = new ItemRange(0, 0, initialRange.bottomCount + 1); - this._renderElementsInRange(initialRenderRange); - } - - _itemsFromList({start, end}) { - const array = []; - let i = 0; - for (const item of this._list) { - if (i >= start && i <= end) { - array.push(item); - } - i = i + 1; - } - return array; - } - - _itemAtIndex(idx) { - let i = 0; - for (const item of this._list) { - if (i === idx) { - return item; - } - i = i + 1; - } - return null; - } - - _adjustPadding(range) { - const { topCount, bottomCount } = range; - const paddingTop = topCount * this._itemHeight; - const paddingBottom = bottomCount * this._itemHeight; - this._root.style.paddingTop = `${paddingTop}px`; - this._root.style.paddingBottom = `${paddingBottom}px`; - } - - _renderedFragment(items, childInstanceModifier) { - const fragment = document.createDocumentFragment(); - for (const item of items) { - const view = this._childCreator(item); - childInstanceModifier(view); - fragment.appendChild(mountView(view, this._mountArgs)); - } - return fragment; - } - - _renderElementsInRange(range) { - const diff = this._renderRange.diff(range); - const renderedItems = this._itemsFromList(diff.toAdd); - this._adjustPadding(range); - const {start, end} = diff.toRemove; - const normalizedStart = this._renderRange.normalize(start); - this._childInstances.splice(normalizedStart, end - start + 1).forEach(child => this._removeChild(child)); - - if (diff.scrollDirection === ScrollDirection.downwards) { - const fragment = this._renderedFragment(renderedItems, view => this._childInstances.push(view)); - this._root.appendChild(fragment); - } - else { - const fragment = this._renderedFragment(renderedItems, view => this._childInstances.unshift(view)); - this._root.insertBefore(fragment, this._root.firstChild); - } - this._renderRange = range; - } - - mount() { - const root = super.mount(); - this._subscription = this._list.subscribe(this); - this._childInstances = []; - this._parent = el("div", {className: "LazyListParent"}, root); - /* - Hooking to scroll events can be expensive. - Do we need to do more (like event throttling)? - */ - this._parent.addEventListener("scroll", () => this._renderIfNeeded()); - this._initialRender(); - return this._parent; - } - - update(attributes) { - this._renderRange = null; - super.update(attributes); - this._childInstances = []; - this._initialRender(); - } - - loadList() { - // We don't render the entire list; so nothing to see here. - } - - _removeChild(child) { - child.root().remove(); - child.unmount(); - } - - onAdd(idx, value) { - const {topCount, renderCount, bottomCount} = this._renderRange; - if (this._renderRange.containsIndex(idx)) { - this.onBeforeListChanged(); - const normalizedIdx = this._renderRange.normalize(idx); - if (bottomCount === 0) { - /* - If we're at the bottom of the list, we need to render the additional item - without removing another item from the list. - We can't increment topCount because the index topCount is not affected by the - add operation (and any modification will thus break ItemRange.normalize()). - We can't increment bottomCount because there's not enough items left to trigger - a further render. - */ - this._renderRange = new ItemRange(topCount, renderCount + 1, bottomCount); - } - else { - // Remove the last element, render the new element - this._removeChild(this._childInstances.pop()); - this._renderRange = new ItemRange(topCount, renderCount, bottomCount + 1); - } - super.onAdd(normalizedIdx, value, true); - this.onListChanged(); - } - else { - this._renderRange = idx < topCount ? new ItemRange(topCount + 1, renderCount, bottomCount): - new ItemRange(topCount, renderCount, bottomCount + 1); - } - this._adjustPadding(this._renderRange); - } - - onRemove(idx, value) { - const {topCount, renderCount, bottomCount} = this._renderRange; - if (this._renderRange.containsIndex(idx)) { - this.onBeforeListChanged(); - const normalizedIdx = this._renderRange.normalize(idx); - super.onRemove(normalizedIdx, value, true); - if (bottomCount === 0) { - // See onAdd for explanation - this._renderRange = new ItemRange(topCount, renderCount - 1, bottomCount); - } - else { - const child = this._childCreator(this._itemAtIndex(this._renderRange.lastIndex)); - this._childInstances.push(child); - this._root.appendChild(mountView(child, this._mountArgs)); - this._renderRange = new ItemRange(topCount, renderCount, bottomCount - 1); - } - this.onListChanged(); - } - else { - this._renderRange = idx < topCount ? new ItemRange(topCount - 1, renderCount, bottomCount): - new ItemRange(topCount, renderCount, bottomCount - 1); - } - this._adjustPadding(this._renderRange); - } - - onUpdate(idx, value, params) { - if (this._renderRange.containsIndex(idx)) { - const normalizedIdx = this._renderRange.normalize(idx); - super.onUpdate(normalizedIdx, value, params); - } - } - - recreateItem(idx, value) { - if (this._renderRange.containsIndex(idx)) { - const normalizedIdx = this._renderRange.normalize(idx); - super.recreateItem(normalizedIdx, value) - } - } - - /** - * Render additional element from top or bottom to offset the outgoing element - */ - _renderExtraOnMove(fromIdx, toIdx) { - const {topCount, renderCount} = this._renderRange; - if (toIdx < fromIdx) { - // Element is moved up the list, so render element from top boundary - const index = topCount; - const child = this._childCreator(this._itemAtIndex(index)); - this._childInstances.unshift(child); - this._root.insertBefore(mountView(child, this._mountArgs), this._root.firstChild); - } - else { - // Element is moved down the list, so render element from bottom boundary - const index = topCount + renderCount - 1; - const child = this._childCreator(this._itemAtIndex(index)); - this._childInstances.push(child); - this._root.appendChild(mountView(child, this._mountArgs)); - } - } - - /** - * Remove an element from top or bottom to make space for the incoming element - */ - _removeElementOnMove(fromIdx, toIdx) { - // If element comes from the bottom, remove element at bottom and vice versa - const child = toIdx < fromIdx ? this._childInstances.pop() : this._childInstances.shift(); - this._removeChild(child); - } - - onMove(fromIdx, toIdx, value) { - const fromInRange = this._renderRange.containsIndex(fromIdx); - const toInRange = this._renderRange.containsIndex(toIdx); - const normalizedFromIdx = this._renderRange.normalize(fromIdx); - const normalizedToIdx = this._renderRange.normalize(toIdx); - if (fromInRange && toInRange) { - super.onMove(normalizedFromIdx, normalizedToIdx, value); - } - else if (fromInRange && !toInRange) { - this.onBeforeListChanged(); - const [child] = this._childInstances.splice(normalizedFromIdx, 1); - this._removeChild(child); - this._renderExtraOnMove(fromIdx, toIdx); - this.onListChanged(); - } - else if (!fromInRange && toInRange) { - this.onBeforeListChanged(); - const child = this._childCreator(value); - this._removeElementOnMove(fromIdx, toIdx); - this._childInstances.splice(normalizedToIdx, 0, child); - insertAt(this._root, normalizedToIdx, mountView(child, this._mountArgs)); - this.onListChanged(); - } - } - -} From e34a92e2ec2917addede180d12f1a34b0d39181d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 14:30:11 +0100 Subject: [PATCH 26/29] fix copyright --- src/platform/web/ui/general/LazyListView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/web/ui/general/LazyListView.ts b/src/platform/web/ui/general/LazyListView.ts index 4686c5ea..81b57264 100644 --- a/src/platform/web/ui/general/LazyListView.ts +++ b/src/platform/web/ui/general/LazyListView.ts @@ -1,5 +1,5 @@ /* -Copyright 2020 Bruno Windels +Copyright 2021 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From 7b38df45da8d9e6e66e1a15c87082274cf813f02 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 14:31:23 +0100 Subject: [PATCH 27/29] i think this is fine now? --- src/platform/web/ui/general/ListRange.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/platform/web/ui/general/ListRange.ts b/src/platform/web/ui/general/ListRange.ts index 2b70a987..80cd17e8 100644 --- a/src/platform/web/ui/general/ListRange.ts +++ b/src/platform/web/ui/general/ListRange.ts @@ -161,7 +161,6 @@ export class ListRange extends Range { return {type: ResultType.Move, fromIdx, toIdx}; } } else { - // TODO const addIdx = this.clampIndex(toIdx); const removeIdx = this.clampIndex(fromIdx); const addValue = addIdx === toIdx ? value : getIteratorValueAtIdx(list[Symbol.iterator](), addIdx)!; From e4be1702c4f6a0cd952c05bc053986622086d911 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 14:32:42 +0100 Subject: [PATCH 28/29] add comment for future test --- src/platform/web/ui/general/ListRange.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/web/ui/general/ListRange.ts b/src/platform/web/ui/general/ListRange.ts index 80cd17e8..4f62ba21 100644 --- a/src/platform/web/ui/general/ListRange.ts +++ b/src/platform/web/ui/general/ListRange.ts @@ -550,6 +550,7 @@ export function tests() { list.move(0, 4); // move "a" to after "e" assert(moved); }, - + // would be good to test here what multiple mutations look like with executing the result of queryXXX + // on an array, much like we do in the view. }; } From f444160c6a29b4766551a20c7364df001a78789a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 23 Nov 2021 14:33:27 +0100 Subject: [PATCH 29/29] feels ok without overflow margin for now --- src/platform/web/ui/general/LazyListView.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/platform/web/ui/general/LazyListView.ts b/src/platform/web/ui/general/LazyListView.ts index 81b57264..1696f044 100644 --- a/src/platform/web/ui/general/LazyListView.ts +++ b/src/platform/web/ui/general/LazyListView.ts @@ -22,7 +22,6 @@ import {IView} from "./types"; export interface IOptions extends IParentOptions { itemHeight: number; - overflowMargin?: number; overflowItems?: number; } @@ -34,13 +33,12 @@ export class LazyListView extends ListView { private scrollContainer?: HTMLElement; constructor( - {itemHeight, overflowMargin = 5, overflowItems = 20, ...options}: IOptions, + {itemHeight, overflowItems = 20, ...options}: IOptions, childCreator: (value: T) => V ) { super(options, childCreator); this.itemHeight = itemHeight; this.overflowItems = overflowItems; - // TODO: this.overflowMargin = overflowMargin; } handleEvent(e: Event) {