Merge pull request #25 from dhilt/issue-23-Adapter-remove-in-buffer-s…

…hould-use-Adapter-update-API Adapter.remove should use Adapter.update for in-buffer deleting
dhilt · Sep 18, 2021 · b34e369 · b34e369
2 parents d3c9d44 + 2d11e99
commit b34e369
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 114 deletions.
diff --git a/demo/index.html b/demo/index.html
@@ -12,7 +12,7 @@
         font-size: small;
       }
     </style>
-    <script src="https://cdn.jsdelivr.net/npm/vscroll@1.3.2/dist/bundles/vscroll.umd.js"></script>
+    <script src="https://cdn.jsdelivr.net/npm/vscroll@1.3.4/dist/bundles/vscroll.umd.js"></script>
   </head>
   <body>
 

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "vscroll",
-  "version": "1.3.2",
+  "version": "1.3.4",
   "description": "Virtual scroll engine",
   "main": "dist/bundles/vscroll.umd.js",
   "module": "dist/bundles/vscroll.esm5.js",
@@ -32,7 +32,7 @@
     "prepack": "npm run build"
   },
   "dependencies": {
-    "tslib": "^2.1.0"
+    "tslib": "^2.3.0"
   },
   "devDependencies": {
     "@babel/core": "^7.12.10",

diff --git a/src/classes/buffer.ts b/src/classes/buffer.ts
@@ -202,7 +202,7 @@ export class Buffer<Data> {
     }
   }
 
-  append(count: number, fixRight: boolean): void {
+  appendVirtually(count: number, fixRight: boolean): void {
     if (fixRight) {
       this.items.forEach(item => item.updateIndex(item.$index - count));
       this.cache.shiftIndexes(-count);
@@ -211,7 +211,7 @@ export class Buffer<Data> {
     this.shiftExtremum(count, fixRight);
   }
 
-  prepend(count: number, fixRight: boolean): void {
+  prependVirtually(count: number, fixRight: boolean): void {
     if (!fixRight) {
       this.items.forEach(item => item.updateIndex(item.$index + count));
       this.cache.shiftIndexes(count);
@@ -220,9 +220,7 @@ export class Buffer<Data> {
     this.shiftExtremum(count, fixRight);
   }
 
-  removeItems(indexes: number[], fixRight: boolean, virtual = false): void {
-    const result: Item<Data>[] = [];
-    const toRemove: number[] = virtual ? indexes : [];
+  removeVirtually(indexes: number[], fixRight: boolean): void {
     const length = this.items.length;
     let shifted = false;
     for (
@@ -231,31 +229,18 @@ export class Buffer<Data> {
       fixRight ? i-- : i++
     ) {
       const item = this.items[i];
-      if (!virtual && indexes.indexOf(item.$index) >= 0) {
-        toRemove.push(item.$index);
-        continue;
-      }
-      const diff = toRemove.reduce((acc, index) => acc + (fixRight
+      const diff = indexes.reduce((acc, index) => acc + (fixRight
         ? (item.$index < index ? 1 : 0)
         : (item.$index > index ? -1 : 0)
       ), 0);
       shifted = shifted || !!diff;
       item.updateIndex(item.$index + diff);
-      if (!virtual) {
-        if (fixRight) {
-          result.unshift(item);
-        } else {
-          result.push(item);
-        }
-      }
     }
-    this.shiftExtremum(-toRemove.length, fixRight);
-    if (!virtual) {
-      this.items = result;
-    } else if (shifted) {
+    this.shiftExtremum(-indexes.length, fixRight);
+    if (shifted) {
       this.items = [...this.items];
     }
-    this.cache.removeItems(toRemove, fixRight);
+    this.cache.removeItems(indexes, fixRight);
   }
 
   updateItems(

diff --git a/src/classes/buffer/cache.ts b/src/classes/buffer/cache.ts
@@ -103,7 +103,7 @@ export class Cache<Data = unknown> {
         itemCache.data = item.data;
       }
       if (itemCache.size !== item.size) { // size changes
-        if (itemCache.size !== void 0) {
+        if (!isNaN(itemCache.size)) {
           this.defaultSize.setExisted(itemCache, item);
         } else {
           this.defaultSize.setNew(item);
@@ -139,7 +139,7 @@ export class Cache<Data = unknown> {
     let min = Infinity, max = -Infinity;
     this.items.forEach(item => {
       if (toRemove.some(index => index === item.$index)) {
-        if (item.size !== void 0) {
+        if (!isNaN(item.size)) {
           this.defaultSize.setRemoved(item);
         }
         return;

diff --git a/src/classes/domRoutines.ts b/src/classes/domRoutines.ts
@@ -39,16 +39,20 @@ export class Routines {
     element[this.horizontal ? 'scrollLeft' : 'scrollTop'] = value;
   }
 
-  getParams(element: HTMLElement, doNotBind?: boolean): ClientRect {
+  getParams(element: HTMLElement, doNotBind?: boolean): DOMRect {
     this.checkElement(element);
     if (this.window && doNotBind) {
+      const { clientWidth, clientHeight, clientLeft, clientTop } = element;
       return {
-        'height': element.clientHeight,
-        'width': element.clientWidth,
-        'top': element.clientTop,
-        'bottom': element.clientTop + element.clientHeight,
-        'left': element.clientLeft,
-        'right': element.clientLeft + element.clientWidth
+        'height': clientHeight,
+        'width': clientWidth,
+        'top': clientTop,
+        'bottom': clientTop + clientHeight,
+        'left': clientLeft,
+        'right': clientLeft + clientWidth,
+        'x': clientLeft,
+        'y': clientTop,
+        'toJSON': () => null,
       };
     }
     return element.getBoundingClientRect();

diff --git a/src/processes/adapter/append.ts b/src/processes/adapter/append.ts
@@ -49,10 +49,10 @@ export default class Append extends BaseAdapterProcessFactory(AdapterProcess.app
       const padding = prepend ? paddings.backward : paddings.forward;
       padding.size += size;
       if (prepend) {
-        buffer.prepend(items.length, fixRight);
+        buffer.prependVirtually(items.length, fixRight);
         scroller.viewport.scrollPosition += size;
       } else {
-        buffer.append(items.length, fixRight);
+        buffer.appendVirtually(items.length, fixRight);
       }
       scroller.logger.log(() => `buffer.${[absIndexToken]} value is set to ${buffer[absIndexToken]}`);
       scroller.logger.stat(`after virtual ${prepend ? 'prepend' : 'append'}`);

diff --git a/src/processes/adapter/remove.ts b/src/processes/adapter/remove.ts
@@ -1,7 +1,8 @@
 import { Scroller } from '../../scroller';
+import Update from './update';
 import { BaseAdapterProcessFactory, AdapterProcess, ProcessStatus } from '../misc/index';
 import { Direction } from '../../inputs/index';
-import { AdapterRemoveOptions, ItemsPredicate } from '../../interfaces/index';
+import { AdapterRemoveOptions, AdapterUpdateOptions, ItemsPredicate } from '../../interfaces/index';
 
 export default class Remove extends BaseAdapterProcessFactory(AdapterProcess.remove) {
 
@@ -18,16 +19,27 @@ export default class Remove extends BaseAdapterProcessFactory(AdapterProcess.rem
     });
   }
 
-  static doRemove(scroller: Scroller, params: AdapterRemoveOptions, sequenceOnly = false): boolean {
+  static doRemove(scroller: Scroller, params: AdapterRemoveOptions): boolean {
     const { fetch } = scroller.state;
     fetch.firstVisible.index = NaN;
-    const bufferRemoveList = Remove.removeBufferedItems(scroller, params);
-    if (params.indexes && params.indexes.length) { // to avoid duplicate buffer-virtual removals
-      params.indexes = params.indexes.filter(i => !bufferRemoveList.includes(i));
+    const removed = Remove.removeBufferedItems(scroller, params);
+    const shouldBuffered = removed.length > 0;
+    if (shouldBuffered) {
+      // exclude just removed in-buffer indexes
+      if (params.indexes && params.indexes.length) {
+        params.indexes = params.indexes.filter(i => !removed.includes(i));
+      }
+      // shift virtual indexes that remain
+      if (params.indexes && params.indexes.length) {
+        const diffLeft = (params.increase ? 1 : 0) * removed.length;
+        const diffRight = (params.increase ? 0 : -1) * removed.length;
+        params.indexes = params.indexes.map(index =>
+          index + (index < removed[0] ? diffLeft : diffRight)
+        );
+      }
     }
-    const shouldRemoveBuffered = bufferRemoveList.length > 0;
-    const shouldRemoveVirtual = Remove.removeVirtualItems(scroller, params, sequenceOnly);
-    if (!shouldRemoveBuffered && !shouldRemoveVirtual) {
+    const shouldVirtual = Remove.removeVirtualItems(scroller, params);
+    if (!shouldBuffered && !shouldVirtual) {
       return false;
     }
     if (!isNaN(fetch.firstVisible.index)) {
@@ -45,66 +57,19 @@ export default class Remove extends BaseAdapterProcessFactory(AdapterProcess.rem
     const newPredicate: ItemsPredicate = item =>
       (predicate && predicate(item)) ||
       (!!indexes && indexes.includes(item.$index));
-    return Remove.runPredicateOverBuffer(scroller, newPredicate, !!increase);
-  }
 
-  static runPredicateOverBuffer(scroller: Scroller, predicate: ItemsPredicate, increase: boolean): number[] {
-    const { viewport, buffer, buffer: { items }, state: { fetch: { firstVisible } } } = scroller;
-
-    // get items to remove
-    const clipList = [];
-    for (let i = 0; i < items.length; i++) {
-      const item = items[i];
-      if (predicate(item.get())) {
-        clipList.push(item);
-        item.toRemove = true;
-      } else if (clipList.length) {
-        break; // allow only first strict uninterrupted sequence
-      }
-    }
-    if (!clipList.length) {
-      return [];
-    }
-
-    // what item should be shown after remove (1-4)
-    const firstClipIndex = clipList[0].$index, lastClipIndex = clipList[clipList.length - 1].$index;
-    // 1) current first visible item will remain
-    const { index: firstIndex, diff } = viewport.getEdgeVisibleItem(buffer.items, Direction.backward);
-    if (firstIndex < firstClipIndex || firstIndex > lastClipIndex) {
-      firstVisible.index = firstIndex;
-      if (!isNaN(firstIndex)) {
-        firstVisible.delta = - buffer.getSizeByIndex(firstIndex) + diff;
-      }
-    }
-    // 2) next after the last removed item
-    if (isNaN(firstVisible.index) && lastClipIndex < buffer.finiteAbsMaxIndex) {
-      firstVisible.index = lastClipIndex + 1;
-    }
-    // 3) prev before the first removed item
-    if (isNaN(firstVisible.index) && firstClipIndex > buffer.finiteAbsMinIndex) {
-      firstVisible.index = firstClipIndex - 1;
-    }
-    // 4) prev before the first removed item
-    if (isNaN(firstVisible.index)) {
-      firstVisible.index = buffer.finiteAbsMinIndex;
-    }
-
-    // logical removal
-    const indexListToRemove = clipList.map(item => item.$index);
-    scroller.logger.log(() =>
-      `going to remove ${clipList.length} item(s) from Buffer: [${indexListToRemove.join(',')}]`
+    const indexesToRemove: number[] = scroller.buffer.items.reduce((acc, item) =>
+      newPredicate(item) ? [...acc, item.$index] : acc, [] as number[]
     );
-    buffer.removeItems(indexListToRemove, increase, false);
-    buffer.checkDefaultSize();
-    Remove.shiftFirstVisibleIndex(scroller, indexListToRemove, increase);
-
-    // physical removal (hiding)
-    clipList.forEach(item => item.hide());
-
-    return indexListToRemove;
+    const updateOptions: AdapterUpdateOptions = {
+      predicate: item => !newPredicate(item),
+      fixRight: increase
+    };
+    Update.doUpdate(scroller, updateOptions);
+    return indexesToRemove;
   }
 
-  static removeVirtualItems(scroller: Scroller, params: AdapterRemoveOptions, sequenceOnly: boolean): boolean {
+  static removeVirtualItems(scroller: Scroller, params: AdapterRemoveOptions): boolean {
     const { indexes, increase } = params;
     if (!indexes || !indexes.length) {
       return false;
@@ -114,7 +79,6 @@ export default class Remove extends BaseAdapterProcessFactory(AdapterProcess.rem
     // get items to remove
     const { finiteAbsMinIndex, firstIndex, finiteAbsMaxIndex, lastIndex } = buffer;
     const toRemove = [];
-    let last = NaN;
     for (let i = 0, len = indexes.length; i < len; i++) {
       const index = indexes[i];
       if (index >= finiteAbsMinIndex && !isNaN(firstIndex) && index < firstIndex) {
@@ -124,11 +88,6 @@ export default class Remove extends BaseAdapterProcessFactory(AdapterProcess.rem
       } else {
         continue;
       }
-      if (sequenceOnly && !isNaN(last) && Math.abs(last - index) > 1) {
-        // allow only first strict uninterrupted sequence
-        break;
-      }
-      last = index;
     }
 
     if (!toRemove.length) {
@@ -146,7 +105,7 @@ export default class Remove extends BaseAdapterProcessFactory(AdapterProcess.rem
 
     // virtual removal
     scroller.logger.log(() => `going to remove ${toRemove.length} item(s) virtually`);
-    buffer.removeItems(toRemove, !!increase, true);
+    buffer.removeVirtually(toRemove, !!increase);
     buffer.checkDefaultSize();
     Remove.shiftFirstVisibleIndex(scroller, toRemove, !!increase);
 

diff --git a/src/version.ts b/src/version.ts
@@ -1,4 +1,4 @@
 export default {
   name: 'vscroll',
-  version: '1.3.2'
+  version: '1.3.4'
 };
diff --git a/tests/buffer.spec.ts b/tests/buffer.spec.ts
@@ -79,12 +79,12 @@ const checkAppend = (params: BufferAppendConfig) => () => {
   const firstId = buffer.items[0].data.id;
   let indexShift = 0;
   if (params.prepend) {
-    buffer.prepend(params.amount, params.fixRight);
+    buffer.prependVirtually(params.amount, params.fixRight);
     if (!params.fixRight) {
       indexShift += params.amount;
     }
   } else {
-    buffer.append(params.amount, params.fixRight);
+    buffer.appendVirtually(params.amount, params.fixRight);
     if (params.fixRight) {
       indexShift -= params.amount;
     }