From 780aac84b6d8af40263e9cc4f78a4fdd8a05e0df Mon Sep 17 00:00:00 2001
From: Ugur Saglam <106508695+ugur-vaadin@users.noreply.github.com>
Date: Tue, 16 May 2023 00:00:28 +0300
Subject: [PATCH] fix: do not recalculate flex basis for hidden rows (#159)

* fix: do not recalculate flex basis for hidden rows

* fix: use class name for assertion

* fix: use board instead of row for tests

* fix: remove redraw call

* test: add test for setting a resized hidden row visible again

* test: manually trigger iron-resize after setting width

* fix: use resize event instead of iron resize

* fix: set visibility correctly

* refactor: extract should recalculate logic

* fix: update the logic to determine visibility

* fix: fix typo

* test: add test for hiding row via an ancestor
---
 src/vaadin-board-row.html          | 35 ++++++++++++++++++++++++---
 test/vaadin-board_redraw_test.html | 38 ++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/src/vaadin-board-row.html b/src/vaadin-board-row.html
index fa7ae36..d01cada 100644
--- a/src/vaadin-board-row.html
+++ b/src/vaadin-board-row.html
@@ -228,9 +228,7 @@
         _recalculateFlexBasis(forceResize) {
           const width = this.getBoundingClientRect().width;
           const breakpoints = this._measureBreakpointsInPx();
-          if (forceResize || width != this._oldWidth
-              || breakpoints.smallSize != this._oldBreakpoints.smallSize
-              || breakpoints.mediumSize != this._oldBreakpoints.mediumSize) {
+          if (forceResize || this._shouldRecalculate(width, breakpoints)) {
             const nodes = this.$.insertionPoint.assignedNodes({ flatten: true });
             const isElementNode = function (node) {
               return !((node.nodeType === node.TEXT_NODE)
@@ -253,6 +251,37 @@
           }
         }
 
+        /** @private */
+        _shouldRecalculate(width, breakpoints) {
+          // Should not recalculate if row is invisible
+          if (this._isElementHidden()) {
+            return false;
+          }
+          return (
+            width !== this._oldWidth ||
+            breakpoints.smallSize !== this._oldBreakpoints.smallSize ||
+            breakpoints.mediumSize !== this._oldBreakpoints.mediumSize
+          );
+        }
+
+        /** @private */
+        _isElementHidden() {
+          // `offsetParent` is `null` when the element itself
+          // or one of its ancestors is hidden with `display: none`.
+          // https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetParent
+          // However `offsetParent` is also null when the element is using fixed
+          // positioning, so additionally check if the element takes up layout space.
+          if (this.offsetParent === null && this.clientWidth === 0 && this.clientHeight === 0) {
+            return true;
+          }
+          // Check inline style first to save a re-flow.
+          if (this.style.visibility === 'hidden' || this.style.display === 'none') {
+            return true;
+          }
+          const computedStyle = window.getComputedStyle(this);
+          return computedStyle.visibility === 'hidden' || computedStyle.display === 'none';
+        }
+
         /**
          * Measure the breakpoints in pixels.
          *
diff --git a/test/vaadin-board_redraw_test.html b/test/vaadin-board_redraw_test.html
index 2632b6d..a7811d0 100644
--- a/test/vaadin-board_redraw_test.html
+++ b/test/vaadin-board_redraw_test.html
@@ -124,6 +124,44 @@
             assert.approximately(rect.top, (Math.floor(i / 2) * rowRect.height / 2) + rowRect.top, 1);
           }
         });
+
+        test("Resize event should not trigger recalculating flex basis for a directly hidden row", function () {
+          const board = fixture("board-1-row-4-items");
+          const row = board.querySelector("#top");
+
+          const initialClassName = row.className;
+
+          row.style.display = "none";
+          row.style.width = "100px";
+          row.fire("resize");
+
+          assert.equal(initialClassName, row.className);
+        });
+
+        test("Resize event should not trigger recalculating flex basis for a row hidden through an ancestor", function () {
+          const board = fixture("board-1-row-4-items");
+          const row = board.querySelector("#top");
+
+          const initialClassName = row.className;
+
+          board.style.display = "none";
+          row.style.width = "100px";
+          row.fire("resize");
+
+          assert.equal(initialClassName, row.className);
+        });
+
+        test("Resize event should trigger recalculating flex basis for a visible row", function () {
+          const board = fixture("board-1-row-4-items");
+          const row = board.querySelector("#top");
+
+          const initialClassName = row.className;
+
+          row.style.width = "100px";
+          row.fire("resize");
+
+          assert.notEqual(initialClassName, row.className);
+        });
       });
     </script>
   </body>