fix: prevent overriding column hidden state when attaching column group (CP: 14.8) (#2216)

* fix: prevent overriding column hidden state when attaching column group (#3805) (CP: 14.8)

Fixes the column group to not synchronize its hidden state to child columns when it is attached, and visible.

Fixes vaadin/flow-components#2959
Cherry-pick of: vaadin/web-components#3805

* remove obsolete hidden check

Author: sissbruecker

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

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');
       }

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,18 +127,15 @@
         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;
         expect(columns[1].hidden).to.be.false;
         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;