diff --git a/src/vaadin-grid-column-group.html b/src/vaadin-grid-column-group.html index 9543c04bd..0927b2bd8 100644 --- a/src/vaadin-grid-column-group.html +++ b/src/vaadin-grid-column-group.html @@ -85,11 +85,8 @@ static get observers() { return [ - '_updateVisibleChildColumns(_childColumns)', - '_childColumnsChanged(_childColumns)', '_groupFrozenChanged(frozen, _rootColumns)', - '_groupHiddenChanged(hidden, _rootColumns)', - '_visibleChildColumnsChanged(_visibleChildColumns)', + '_groupHiddenChanged(hidden)', '_colSpanChanged(_colSpan, _headerCell, _footerCell)', '_groupOrderChanged(_order, _rootColumns)', '_groupReorderStatusChanged(_reorderStatus, _rootColumns)', @@ -117,9 +114,12 @@ */ _columnPropChanged(path, value) { if (path === 'hidden') { - this._preventHiddenCascade = true; + // Prevent synchronization of the hidden state to child columns. + // If the group is currently auto-hidden, and one column is made visible, + // we don't want the other columns to become visible as well. + this._preventHiddenSynchronization = true; this._updateVisibleChildColumns(this._childColumns); - this._preventHiddenCascade = false; + this._preventHiddenSynchronization = false; } if (/flexGrow|width|hidden|_childColumns/.test(path)) { @@ -191,15 +191,9 @@ /** @private */ _updateVisibleChildColumns(childColumns) { - this._visibleChildColumns = Array.prototype.filter.call(childColumns, col => !col.hidden); - } - - /** @private */ - _childColumnsChanged(childColumns) { - if (!this._autoHidden && this.hidden) { - Array.prototype.forEach.call(childColumns, column => column.hidden = true); - this._updateVisibleChildColumns(childColumns); - } + this._visibleChildColumns = Array.prototype.filter.call(childColumns, (col) => !col.hidden); + this._colSpan = this._visibleChildColumns.length; + this._updateAutoHidden(); } /** @protected */ @@ -233,26 +227,31 @@ } /** @private */ - _groupHiddenChanged(hidden, rootColumns) { - if (rootColumns && !this._preventHiddenCascade) { - this._ignoreVisibleChildColumns = true; - rootColumns.forEach(column => column.hidden = hidden); - this._ignoreVisibleChildColumns = false; + _groupHiddenChanged(hidden) { + // When initializing the hidden property, only sync hidden state to columns + // if group is actually hidden. Otherwise, we could override a hidden column + // to be visible. + // We always want to run this though if the property is actually changed. + if (hidden || this.__groupHiddenInitialized) { + this._synchronizeHidden(); } + this.__groupHiddenInitialized = true; + } - this._columnPropChanged('hidden'); + /** @private */ + _updateAutoHidden() { + const wasAutoHidden = this._autoHidden; + this._autoHidden = (this._visibleChildColumns || []).length === 0; + // Only modify hidden state if group was auto-hidden, or becomes auto-hidden + if (wasAutoHidden || this._autoHidden) { + this.hidden = this._autoHidden; + } } /** @private */ - _visibleChildColumnsChanged(visibleChildColumns) { - this._colSpan = visibleChildColumns.length; - - if (!this._ignoreVisibleChildColumns) { - if (visibleChildColumns.length === 0) { - this._autoHidden = this.hidden = true; - } else if (this.hidden && this._autoHidden) { - this._autoHidden = this.hidden = false; - } + _synchronizeHidden() { + if (this._childColumns && !this._preventHiddenSynchronization) { + this._childColumns.forEach((column) => (column.hidden = this.hidden)); } } @@ -283,10 +282,14 @@ if (info.addedNodes.filter(this._isColumnElement).length > 0 || info.removedNodes.filter(this._isColumnElement).length > 0) { - this._preventHiddenCascade = true; + // Prevent synchronization of the hidden state to child columns. + // If the group is currently auto-hidden, and a visible column is added, + // we don't want the other columns to become visible as well. + this._preventHiddenSynchronization = true; this._rootColumns = this._getChildColumns(this); this._childColumns = this._rootColumns; - this._preventHiddenCascade = false; + this._updateVisibleChildColumns(this._childColumns); + this._preventHiddenSynchronization = false; // Update the column tree with microtask timing to avoid shady style scope issues Polymer.Async.microTask.run(() => { diff --git a/src/vaadin-grid-column.html b/src/vaadin-grid-column.html index 6a5601e24..ad105380b 100644 --- a/src/vaadin-grid-column.html +++ b/src/vaadin-grid-column.html @@ -313,10 +313,6 @@ /** @private */ __setColumnTemplateOrRenderer(template, renderer, cells) { - // no renderer or template needed in a hidden column - if (this.hidden) { - return; - } if (template && renderer) { throw new Error('You should only use either a renderer or a template'); } diff --git a/test/column-group.html b/test/column-group.html index a03072bbc..54fb16ab3 100644 --- a/test/column-group.html +++ b/test/column-group.html @@ -99,21 +99,21 @@ expect(columns[1].frozen).to.be.true; }); - it('should hide group column', () => { + it('should hide when all columns are hidden', () => { columns[0].hidden = true; columns[1].hidden = true; expect(group.hidden).to.be.true; }); - it('should unhide group column', () => { + it('should unhide when making child column visible', () => { group.hidden = true; columns[0].hidden = false; expect(group.hidden).to.be.false; }); - it('should not unhide other columns', () => { + it('should not unhide other columns when making a column visible', () => { group.hidden = true; columns[0].hidden = false; @@ -127,10 +127,7 @@ expect(columns[0].hidden).to.be.true; expect(columns[1].hidden).to.be.true; expect(group.hidden).to.be.true; - }); - it('should propagate hidden to child columns 2', () => { - group.hidden = true; group.hidden = false; expect(columns[0].hidden).to.be.false; @@ -138,7 +135,7 @@ expect(group.hidden).to.be.false; }); - it('should hide the group', () => { + it('should hide when removing all child columns', () => { group.removeChild(columns[0]); group.removeChild(columns[1]); Polymer.flush(); @@ -147,7 +144,7 @@ expect(group.hidden).to.be.true; }); - it('should unhide the group', () => { + it('should unhide when adding a visible column', () => { group.removeChild(columns[0]); group.removeChild(columns[1]); group._observer.flush(); @@ -159,7 +156,7 @@ expect(group.hidden).to.be.false; }); - it('should not unhide the group', () => { + it('should not unhide when adding a hidden column', () => { group.removeChild(columns[0]); group.removeChild(columns[1]); group._observer.flush(); @@ -171,6 +168,21 @@ expect(group.hidden).to.be.true; }); + // Regression test for https://github.com/vaadin/flow-components/issues/2959 + it('should not unhide columns when attached to DOM', () => { + const group = document.createElement('vaadin-grid-column-group'); + const visibleColumn = document.createElement('vaadin-grid-column'); + const hiddenColumn = document.createElement('vaadin-grid-column'); + hiddenColumn.hidden = true; + + group.appendChild(visibleColumn); + group.appendChild(hiddenColumn); + document.body.appendChild(group); + group._observer.flush(); + + expect(hiddenColumn.hidden).to.be.true; + }); + it('should calculate column group width after hiding a column', () => { columns[0].hidden = true;