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() { - - } -}