refactor: change to logic to minimize changes to existing tests

Author: tomivirkki

packages/vaadin-crud/src/vaadin-crud-grid.js

@@ -54,7 +54,7 @@ class CrudGridElement extends IncludedMixin(GridElement) {
 
   /** @private */
   __onItemsChange(items) {
-    if ((!this.dataProvider || this.dataProvider.__arrayDataProvider) && !this.include && items && items[0]) {
+    if ((!this.dataProvider || this.dataProvider == this._arrayDataProvider) && !this.include && items && items[0]) {
       this._configure(items[0]);
     }
   }
@@ -91,7 +91,7 @@ class CrudGridElement extends IncludedMixin(GridElement) {
    * @private
    */
   _dataProviderChanged(dataProvider, oldDataProvider) {
-    if (dataProvider.__arrayDataProvider) {
+    if (this._arrayDataProvider == dataProvider) {
       super._dataProviderChanged(dataProvider, oldDataProvider);
     } else if (this.__dataProviderWrapper != dataProvider) {
       this.innerHTML = '';

packages/vaadin-grid/src/arrayDataProvider.js

@@ -106,7 +106,7 @@ function filter(items, filters) {
 }
 
 export const createArrayDataProvider = (allItems) => {
-  const arrayDataProvider = (params, callback) => {
+  return (params, callback) => {
     let items = allItems || [];
     if (params.filters && checkPaths(params.filters, 'filtering', items)) {
       items = filter(items, params.filters);
@@ -116,14 +116,10 @@ export const createArrayDataProvider = (allItems) => {
       items = multiSort(items, params.sortOrders);
     }
 
-    const start = params.page * params.pageSize;
-    const end = start + params.pageSize;
+    const count = Math.min(items.length, params.pageSize);
+    const start = params.page * count;
+    const end = start + count;
     const slice = items.slice(start, end);
     callback(slice, items.length);
   };
-
-  // Mark the data provider as an array data provider
-  // (needed by the selection column which has logic depending on it)
-  arrayDataProvider.__arrayDataProvider = true;
-  return arrayDataProvider;
 };

packages/vaadin-grid/src/vaadin-grid-a11y-mixin.js

@@ -10,7 +10,7 @@
 export const A11yMixin = (superClass) =>
   class A11yMixin extends superClass {
     static get observers() {
-      return ['_a11yUpdateGridSize(_effectiveSize, _columnTree, _columnTree.*)'];
+      return ['_a11yUpdateGridSize(size, _columnTree, _columnTree.*)'];
     }
 
     /** @private */
@@ -26,15 +26,15 @@ export const A11yMixin = (superClass) =>
     }
 
     /** @private */
-    _a11yUpdateGridSize(effectiveSize, _columnTree) {
-      if (effectiveSize === undefined || _columnTree === undefined) {
+    _a11yUpdateGridSize(size, _columnTree) {
+      if (size === undefined || _columnTree === undefined) {
         return;
       }
 
       const bodyColumns = _columnTree[_columnTree.length - 1];
       this.$.table.setAttribute(
         'aria-rowcount',
-        effectiveSize + this._a11yGetHeaderRowCount(_columnTree) + this._a11yGetFooterRowCount(_columnTree)
+        size + this._a11yGetHeaderRowCount(_columnTree) + this._a11yGetFooterRowCount(_columnTree)
       );
       this.$.table.setAttribute('aria-colcount', (bodyColumns && bodyColumns.length) || 0);
 
@@ -52,10 +52,7 @@ export const A11yMixin = (superClass) =>
     /** @protected */
     _a11yUpdateFooterRows() {
       Array.from(this.$.footer.children).forEach((footerRow, index) =>
-        footerRow.setAttribute(
-          'aria-rowindex',
-          this._a11yGetHeaderRowCount(this._columnTree) + this._effectiveSize + index + 1
-        )
+        footerRow.setAttribute('aria-rowindex', this._a11yGetHeaderRowCount(this._columnTree) + this.size + index + 1)
       );
     }
 

packages/vaadin-grid/src/vaadin-grid-array-data-provider-mixin.d.ts

@@ -12,6 +12,8 @@ declare interface ArrayDataProviderMixin<TItem> {
    * instances.
    */
   items: TItem[] | null | undefined;
+
+  _arrayDataProvider(opts: GridDataProviderParams<TItem> | null, cb: GridDataProviderCallback<TItem> | null): void;
 }
 
 export { ArrayDataProviderMixin, ArrayDataProviderMixinConstructor };

packages/vaadin-grid/src/vaadin-grid-array-data-provider-mixin.js

@@ -23,15 +23,54 @@ export const ArrayDataProviderMixin = (superClass) =>
     }
 
     static get observers() {
-      return ['__itemsChanged(items)'];
+      return ['__dataProviderOrItemsChanged(dataProvider, items, isAttached, items.*, _filters, _sorters)'];
     }
 
-    /** @private */
-    __itemsChanged(items) {
-      if (Array.isArray(items)) {
-        this.dataProvider = createArrayDataProvider(items, {});
-      } else if (items === null) {
-        this.dataProvider = null;
+    __setArrayDataProvider(items) {
+      const arrayDataProvider = createArrayDataProvider(this.items, {});
+      arrayDataProvider.__items = items;
+      this.setProperties({
+        _arrayDataProvider: arrayDataProvider,
+        size: items.length,
+        dataProvider: arrayDataProvider
+      });
+    }
+
+    __unsetArrayDataProvider() {
+      this.setProperties({
+        _arrayDataProvider: undefined,
+        items: undefined // TODO: selection column just checks for grid.items in some logic. Add a test which adds items and then a custom dp.
+      });
+    }
+
+    __dataProviderOrItemsChanged(dataProvider, items, isAttached) {
+      if (!isAttached) {
+        return;
+      }
+
+      if (this._arrayDataProvider) {
+        // Has an items array data provider beforehand
+
+        if (dataProvider !== this._arrayDataProvider) {
+          // A custom data provider was set externally
+          this.__unsetArrayDataProvider();
+        } else if (!items) {
+          // The items array was unset
+          this.__unsetArrayDataProvider();
+          this.dataProvider = undefined;
+          this.size = 0;
+          this.clearCache();
+        } else if (this._arrayDataProvider.__items === items) {
+          // The items array was modified
+          this.clearCache();
+          this.size = this._effectiveSize;
+        } else {
+          // The items array was replaced
+          this.__setArrayDataProvider(items);
+        }
+      } else if (items) {
+        // There was no array data provider before items was set
+        this.__setArrayDataProvider(items);
       }
     }
   };

packages/vaadin-grid/src/vaadin-grid-data-provider-mixin.js

@@ -203,6 +203,14 @@ export const DataProviderMixin = (superClass) =>
           type: Object,
           notify: true,
           value: () => []
+        },
+
+        /**
+         * @private
+         */
+        __expandedKeys: {
+          type: Object,
+          value: () => new Set()
         }
       };
     }

packages/vaadin-grid/src/vaadin-grid-filter-mixin.js

@@ -21,10 +21,6 @@ export const FilterMixin = (superClass) =>
       };
     }
 
-    static get observers() {
-      return ['__applyFilters(isAttached)'];
-    }
-
     /** @protected */
     ready() {
       super.ready();
@@ -62,7 +58,6 @@ export const FilterMixin = (superClass) =>
     __applyFilters() {
       if (this.dataProvider && this.isAttached) {
         this.clearCache();
-        this._ensureFirstPageLoaded();
       }
     }
 

packages/vaadin-grid/src/vaadin-grid-selection-column.js

@@ -270,13 +270,13 @@ class GridSelectionColumnElement extends GridColumnElement {
   }
 
   /** @private */
-  _onDataProviderChanged() {
+  __onDataProviderChanged() {
     this.__selectAllHidden = !this.__gridUsesArrayDataProvider();
   }
 
   /**
-   * Assuming the grid uses an array data provider, fetches all the filtered items
-   * from it and invokes the callback with the resulting array.
+   * Assuming the grid uses an items array data provider, fetches all the filtered items
+   * from the data provider and invokes the callback with the resulting array.
    *
    * @private
    **/
@@ -296,7 +296,7 @@ class GridSelectionColumnElement extends GridColumnElement {
    * @private
    **/
   __gridUsesArrayDataProvider() {
-    return this._grid.dataProvider && this._grid.dataProvider.__arrayDataProvider;
+    return this._grid._arrayDataProvider;
   }
 }
 

packages/vaadin-grid/src/vaadin-grid.js

@@ -526,6 +526,7 @@ class GridElement extends ElementMixin(
       const cellCoordinates = this.__getBodyCellCoordinates(cell);
 
       virtualizer.size = effectiveSize;
+      virtualizer.flush();
 
       // If the focused cell's parent row got hidden by the size change, focus the corresponding new cell
       cellCoordinates && cell.parentElement.hidden && this.__focusBodyCell(cellCoordinates);

packages/vaadin-grid/test/array-data-provider.test.js

@@ -1,7 +1,7 @@
 import { expect } from '@esm-bundle/chai';
 import sinon from 'sinon';
 import { click, fixtureSync, nextFrame } from '@vaadin/testing-helpers';
-import { flushGrid, getCellContent, getRows, getRowCells } from './helpers.js';
+import { flushGrid, getCellContent, getRows, getRowCells, getBodyCellContent } from './helpers.js';
 import '../vaadin-grid.js';
 import '../vaadin-grid-filter.js';
 import '../vaadin-grid-sorter.js';
@@ -85,10 +85,10 @@ describe('array data provider', () => {
       expect(grid.dataProvider).to.equal(grid._arrayDataProvider);
     });
 
-    it('should not override custom data provider', () => {
+    it('should override custom data provider', () => {
       const ds = (grid.dataProvider = () => {});
       grid.items = [1, 2, 3];
-      expect(grid.dataProvider).to.equal(ds);
+      expect(grid.dataProvider).not.to.equal(ds);
     });
 
     it('should handle new array of same length', () => {
@@ -249,3 +249,40 @@ describe('invalid paths', () => {
     });
   });
 });
+
+describe('items with a custom data provider', () => {
+  let grid;
+  const dataProvider = (params, callback) => callback([{ value: 'foo' }], 1);
+  const items = [{ value: 'bar' }];
+
+  beforeEach(() => {
+    grid = fixtureSync(`
+      <vaadin-grid>
+        <vaadin-grid-column path="value"></vaadin-grid-column>
+      </vaadin-grid>
+    `);
+  });
+
+  it('use the items array', () => {
+    grid.dataProvider = dataProvider;
+    grid.items = items;
+    flushGrid(grid);
+    expect(getBodyCellContent(grid, 0, 0).textContent).to.equal('bar');
+    expect(grid.dataProvider).not.to.equal(dataProvider);
+  });
+
+  it('should use the data provider', () => {
+    grid.items = items;
+    grid.dataProvider = dataProvider;
+    flushGrid(grid);
+    expect(getBodyCellContent(grid, 0, 0).textContent).to.equal('foo');
+    expect(grid.items).to.be.undefined;
+  });
+
+  it('should unset the data provider', () => {
+    grid.items = items;
+    grid.items = null;
+    flushGrid(grid);
+    expect(grid.dataProvider).to.be.undefined;
+  });
+});