Skip to content

Commit

Permalink
refactor: remove wildcards from complex observers in grid (#6677)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomivirkki authored Oct 20, 2023
1 parent f1d6c02 commit 78c6e52
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 49 deletions.
2 changes: 1 addition & 1 deletion packages/grid/src/vaadin-grid-a11y-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { iterateChildren, iterateRowCells } from './vaadin-grid-helpers.js';
export const A11yMixin = (superClass) =>
class A11yMixin extends superClass {
static get observers() {
return ['_a11yUpdateGridSize(size, _columnTree, _columnTree.*)'];
return ['_a11yUpdateGridSize(size, _columnTree)'];
}

/** @private */
Expand Down
2 changes: 1 addition & 1 deletion packages/grid/src/vaadin-grid-array-data-provider-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const ArrayDataProviderMixin = (superClass) =>
}

static get observers() {
return ['__dataProviderOrItemsChanged(dataProvider, items, isAttached, items.*, _filters, _sorters)'];
return ['__dataProviderOrItemsChanged(dataProvider, items, isAttached, _filters, _sorters, items.*)'];
}

/** @private */
Expand Down
24 changes: 12 additions & 12 deletions packages/grid/src/vaadin-grid-column-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,21 +241,21 @@ export const ColumnBaseMixin = (superClass) =>

static get observers() {
return [
'_widthChanged(width, _headerCell, _footerCell, _cells.*)',
'_frozenChanged(frozen, _headerCell, _footerCell, _cells.*)',
'_frozenToEndChanged(frozenToEnd, _headerCell, _footerCell, _cells.*)',
'_flexGrowChanged(flexGrow, _headerCell, _footerCell, _cells.*)',
'_textAlignChanged(textAlign, _cells.*, _headerCell, _footerCell)',
'_orderChanged(_order, _headerCell, _footerCell, _cells.*)',
'_widthChanged(width, _headerCell, _footerCell, _cells)',
'_frozenChanged(frozen, _headerCell, _footerCell, _cells)',
'_frozenToEndChanged(frozenToEnd, _headerCell, _footerCell, _cells)',
'_flexGrowChanged(flexGrow, _headerCell, _footerCell, _cells)',
'_textAlignChanged(textAlign, _cells, _headerCell, _footerCell)',
'_orderChanged(_order, _headerCell, _footerCell, _cells)',
'_lastFrozenChanged(_lastFrozen)',
'_firstFrozenToEndChanged(_firstFrozenToEnd)',
'_onRendererOrBindingChanged(_renderer, _cells, _bodyContentHidden, _cells.*, path)',
'_onRendererOrBindingChanged(_renderer, _cells, _bodyContentHidden, path)',
'_onHeaderRendererOrBindingChanged(_headerRenderer, _headerCell, path, header)',
'_onFooterRendererOrBindingChanged(_footerRenderer, _footerCell)',
'_resizableChanged(resizable, _headerCell)',
'_reorderStatusChanged(_reorderStatus, _headerCell, _footerCell, _cells.*)',
'_hiddenChanged(hidden, _headerCell, _footerCell, _cells.*)',
'_rowHeaderChanged(rowHeader, _cells.*)',
'_reorderStatusChanged(_reorderStatus, _headerCell, _footerCell, _cells)',
'_hiddenChanged(hidden, _headerCell, _footerCell, _cells)',
'_rowHeaderChanged(rowHeader, _cells)',
'__headerFooterPartNameChanged(_headerCell, _footerCell, headerPartName, footerPartName)',
];
}
Expand Down Expand Up @@ -444,11 +444,11 @@ export const ColumnBaseMixin = (superClass) =>

/** @private */
_rowHeaderChanged(rowHeader, cells) {
if (!cells.value) {
if (!cells) {
return;
}

cells.value.forEach((cell) => {
cells.forEach((cell) => {
cell.setAttribute('role', rowHeader ? 'rowheader' : 'gridcell');
});
}
Expand Down
17 changes: 8 additions & 9 deletions packages/grid/src/vaadin-grid-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,7 @@ export const GridMixin = (superClass) =>
),
) {
static get observers() {
return [
'_columnTreeChanged(_columnTree, _columnTree.*)',
'_flatSizeChanged(_flatSize, __virtualizer, _hasData, _columnTree)',
];
return ['_columnTreeChanged(_columnTree)', '_flatSizeChanged(_flatSize, __virtualizer, _hasData, _columnTree)'];
}

static get properties() {
Expand Down Expand Up @@ -510,9 +507,11 @@ export const GridMixin = (superClass) =>
}

if (this._columnTree) {
this._columnTree[this._columnTree.length - 1].forEach(
(c) => c.isConnected && c.notifyPath && c.notifyPath('_cells.*', c._cells),
);
this._columnTree[this._columnTree.length - 1].forEach((c) => {
if (c.isConnected && c._cells) {
c._cells = [...c._cells];
}
});
}

this.__afterCreateScrollerRowsDebouncer = Debouncer.debounce(
Expand Down Expand Up @@ -677,8 +676,8 @@ export const GridMixin = (superClass) =>
detailsCell._vacant = false;
}

if (column.notifyPath && !noNotify) {
column.notifyPath('_cells.*', column._cells);
if (!noNotify) {
column._cells = [...column._cells];
}
} else {
// Header & footer
Expand Down
9 changes: 2 additions & 7 deletions packages/grid/src/vaadin-grid-row-details-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const RowDetailsMixin = (superClass) =>

static get observers() {
return [
'_detailsOpenedItemsChanged(detailsOpenedItems.*, rowDetailsRenderer)',
'_detailsOpenedItemsChanged(detailsOpenedItems, rowDetailsRenderer)',
'_rowDetailsRendererChanged(rowDetailsRenderer)',
];
}
Expand Down Expand Up @@ -90,12 +90,7 @@ export const RowDetailsMixin = (superClass) =>
}

/** @private */
_detailsOpenedItemsChanged(changeRecord, rowDetailsRenderer) {
// Skip to avoid duplicate work of both `.splices` and `.length` updates.
if (changeRecord.path === 'detailsOpenedItems.length' || !changeRecord.value) {
return;
}

_detailsOpenedItemsChanged(detailsOpenedItems, rowDetailsRenderer) {
iterateChildren(this.$.items, (row) => {
// Re-renders the row to possibly close the previously opened details.
if (row.hasAttribute('details-opened')) {
Expand Down
2 changes: 1 addition & 1 deletion packages/grid/src/vaadin-grid-selection-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const SelectionMixin = (superClass) =>
}

static get observers() {
return ['__selectedItemsChanged(itemIdPath, selectedItems.*)'];
return ['__selectedItemsChanged(itemIdPath, selectedItems)'];
}

/**
Expand Down
16 changes: 0 additions & 16 deletions packages/grid/test/array-data-provider.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,6 @@ describe('array data provider', () => {
expect(getBodyCellContent(grid, 1, 0).textContent).to.equal('baz');
});

it('should be observed for shift', () => {
grid.unshift('items', {
name: {
first: 'a',
last: 'b',
},
});
expect(grid.size).to.equal(3);
expect(getBodyCellContent(grid, 0, 0).textContent).to.equal('a');
});

it('should be observed for mutation', () => {
grid.set('items.0.name.first', 'new');
expect(getBodyCellContent(grid, 0, 0).textContent).to.equal('new');
});

it('should handle null', () => {
grid.items = null;
expect(grid.size).to.equal(0);
Expand Down
4 changes: 2 additions & 2 deletions packages/grid/test/selection.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('selection', () => {

describe('selectedItems', () => {
it('should reflect changes', () => {
grid.push('selectedItems', cachedItems[1]);
grid.selectedItems = [...grid.selectedItems, cachedItems[1]];
expect(rows[1].hasAttribute('selected')).to.be.true;
});

Expand Down Expand Up @@ -250,7 +250,7 @@ describe('multi selection column', () => {
it('should have bound the body checkbox to selected items', () => {
const selectCheckbox = firstBodyCheckbox;

grid.push('selectedItems', cachedItems[0]);
grid.selectedItems = [...grid.selectedItems, cachedItems[0]];

expect(selectCheckbox.checked).to.be.true;
});
Expand Down

0 comments on commit 78c6e52

Please sign in to comment.