fix: Content set, Safari 14.1.1. / WebKitGTK 2.32.0 (#1650)

* fix: Update content set to workaround Safari 14.1.1, WebKitGTK 2.32.0 bug

* fix: Update commented out line

* fix: Correct commented out break

* fix: Update changelog

* fix: Revert unrelated linting updates

* fix: Set resize observer on this

* fix: Switch to onresize event; update container query

* fix: Add debounce

* fix: Remove commented out resize

* fix: Rework building content

* fix: Revert breakpoint change

* fix: Build wrapper no longer used

* fix: Update tests to use promises

* fix: IE11 bug fix

* fix: Updated baselines for IE11

* fix: Update readme

* fix: Add IE11 note to docs

Author: castastrophe

CHANGELOG-1.x.md

@@ -1,3 +1,7 @@
+# 1.9.1 (2021-05-27)
+
+- [](https://github.com/patternfly/patternfly-elements/commit/) fix: pfe-content-set, Safari 14.1.1 / WebKitGTK 2.32.0 bug fix
+
 # 1.9.0 (2021-05-26)
 
 - [cb12965](https://github.com/patternfly/patternfly-elements/commit/cb12965db958aaded5baf5ba8867532346d63f03) feat: pfe-progress-steps (new) (#1436)

elements/pfe-content-set/README.md

@@ -1,12 +1,14 @@
 
-# PatternFly Content set Element
+# PatternFly Content set
 
 ## Overview
 
 `pfe-content-set` is a combo component, which brings together the utilities of the accordion and tabs components. Effectively both of these components do the same job, which is to encapsulate chunks of information under headings for easier browsing. Hiding some information and allowing the user to toggle through the headings to show other bits of information.
 
 Since tabs can pose a layout issue on mobile because of the lack of horizontal space, this component will first assess the width of the parent container. If the width of pfe-content-set is less than or equal to 700px, the component will render the content within the `<pfe-accordion>` component. If it is larger than this value, the content will be rendered within the `<pfe-tabs>` component.
 
+_Note:_ In IE11, the component will only render as accordions and never as tabs.
+
 ## Dependencies
 
 Requires both the `pfe-accordion` and `pfe-tabs` components, as well as the base `pfelement`.

elements/pfe-content-set/docs/index.md

@@ -20,6 +20,8 @@ Content set brings together the utilities of the accordion and tabs components.
 
 Since tabs can pose a layout issue on mobile because of the lack of horizontal space, this component will first assess the width of the parent container. If the width of pfe-content-set is less than or equal to 700px, the component will render the content within the `<pfe-accordion>` component. If it is larger than this value, the content will be rendered within the `<pfe-tabs>` component.
 
+_Note:_ In IE11, the component will only render as accordions and never as tabs.
+
 ### Large screens
 On large screens the content will display as tabs.
 

elements/pfe-content-set/src/pfe-content-set.js

@@ -174,16 +174,16 @@ class PfeContentSet extends PFElement {
    */
   get view() {
     if (!this._rendered) return;
-
-    return this.shadowRoot.querySelector(`#container > ${this.expectedTag}`);
+    return this.shadowRoot.querySelector(this.expectedTag);
   }
 
   /**
    * Getter: should this be rendered as a tabset based on the breakpoint size
    * @returns {boolean} Is this a tabset?
    */
   get expectedTag() {
-    return this.isTab ? PfeTabs.tag : PfeAccordion.tag;
+    if (this.isIE11) return "pfe-accordion";
+    else return this.isTab ? "pfe-tabs" : "pfe-accordion";
   }
 
   /**
@@ -236,7 +236,6 @@ class PfeContentSet extends PFElement {
     this._resizeHandler = this._resizeHandler.bind(this);
 
     this._build = this._build.bind(this);
-    this._buildWrapper = this._buildWrapper.bind(this);
     this._buildSets = this._buildSets.bind(this);
 
     this._isHeader = this._isHeader.bind(this);
@@ -252,26 +251,37 @@ class PfeContentSet extends PFElement {
     this._updateBreakpoint = this._updateBreakpoint.bind(this);
 
     this._observer = new MutationObserver(this._mutationHandler);
-    if (window.ResizeObserver) this._resizeObserver = new ResizeObserver(this._resizeHandler);
   }
 
   connectedCallback() {
     super.connectedCallback();
 
+    // this.setAttribute("hidden", "");
+
     // Validate that the light DOM data exists before building
-    if (this.hasValidLightDOM) {
-      this._build();
+    if (this.hasValidLightDOM) this._build();
 
-      if (!this.isIE11 && window.ResizeObserver && this.parentElement) {
-        this._resizeObserver.observe(this.parentElement);
-      }
-    } else if (!this.isIE11) this._observer.observe(this, CONTENT_MUTATION_CONFIG);
+    if (!this.isIE11) {
+      window.addEventListener("resize", () => {
+        clearTimeout(this._resizeHandler._tId);
+        this._resizeHandler._tId = setTimeout(this._resizeHandler, 100);
+      });
+
+      this._observer.observe(this, CONTENT_MUTATION_CONFIG);
+    }
   }
 
   disconnectedCallback() {
     super.disconnectedCallback();
-    this._observer.disconnect();
-    if (window.ResizeObserver) this._resizeObserver.disconnect();
+
+    if (!this.isIE11) {
+      this._observer.disconnect();
+
+      window.removeEventListener("resize", () => {
+        clearTimeout(this._resizeHandler._tId);
+        this._resizeHandler._tId = setTimeout(this._resizeHandler, 100);
+      });
+    }
   }
 
   /**
@@ -448,67 +458,51 @@ class PfeContentSet extends PFElement {
    * Manage the building of the rendering component
    * Optionally accepts the input of new nodes added to the DOM
    */
-  _build(addedNodes) {
-    // @TODO: Add back a promise here post-IE11
-    let view = this.view;
-    if (!view || view.tag !== this.expectedTag) {
-      view = this._buildWrapper();
-    }
-
-    // Disconnect the observer while we parse it
-    this._observer.disconnect();
-
-    let tag = view.tag || view.tagName.toLowerCase();
-    const template = tag === "pfe-tabs" ? PfeTabs.contentTemplate : PfeAccordion.contentTemplate;
-
-    let rawSets = null;
-    if (addedNodes) rawSets = addedNodes;
-    if (!rawSets && [...this.children].length) rawSets = this.children;
-
-    // Clear out the content of the host if we're using the full child list
-    if (!addedNodes && rawSets) view.innerHTML = "";
+  // @TODO: Add back a promise here post-IE11
+  _build() {
+    const addedNodes = this.children;
 
     // If sets is not null, build them using the template
-    if (rawSets) {
-      let sets = this._buildSets(rawSets, template);
-      if (sets) view.appendChild(sets);
-    }
-
-    // @todo find out why we need this shim
-    // Shadydom breaks if we use innerHTML to set the new content but Selenium will infinitely
-    // loop in out tests if we use appendChild.
-    if (window.ShadyDOM) this.shadowRoot.querySelector(`#container`).appendChild(view);
-    else this.shadowRoot.querySelector(`#container`).innerHTML = view.outerHTML;
-
-    Promise.all([customElements.whenDefined(tag)]).then(() => {
-      this.cascadeProperties();
-
-      this.resetContext();
-
-      // Attach the mutation observer
-      if (!this.isIE11) this._observer.observe(this, CONTENT_MUTATION_CONFIG);
-
-      return;
-    });
-  }
-
-  /*
-   * Note: be sure to disconnect the observer before running this
-   */
-  _buildWrapper() {
-    // If the upgraded component matches the tag name of the expected rendering component, return now;
-    if (this.view) return this.view;
+    if (addedNodes.length > 0) {
+      Promise.all([customElements.whenDefined(this.expectedTag)]).then(() => {
+        const template = this.expectedTag === "pfe-tabs" ? PfeTabs.contentTemplate : PfeAccordion.contentTemplate;
+        const sets = this._buildSets(addedNodes, template);
+        const container = this.shadowRoot.querySelector("#container");
+
+        if (sets && container) {
+          if (!this.isIE11) {
+            // Disconnect the observer while we parse it
+            this._observer.disconnect();
+
+            // This does not work in IE11 for some reason
+            container.innerHTML = sets.outerHTML;
+
+            // Context is irrelevant in IE11
+            this.resetContext();
+          } else {
+            container.innerHTML = "";
+            container.appendChild(sets);
+
+            // In IE11, we need to hide the light DOM headers (b/c they're copied into shadow DOM on accordion)
+            [...this.querySelectorAll("[pfe-content-set--header]")].map(item => {
+              item.style.display = "none";
+            });
+          }
 
-    // Create the rendering element
-    let newEl = document.createElement(this.expectedTag);
-    newEl.id = this.id || this.pfeId || this.randomId;
+          this.cascadeProperties();
+          this.removeAttribute("hidden");
 
-    // Return the new element so that the content can be injected
-    return newEl;
+          // Attach the mutation observer
+          if (!this.isIE11) this._observer.observe(this, CONTENT_MUTATION_CONFIG);
+        }
+      });
+    } else {
+      this.setAttribute("hidden", "");
+    }
   }
 
   _buildSets(sets, template) {
-    let fragment = document.createDocumentFragment();
+    let tagElement = document.createElement(this.expectedTag);
 
     for (let i = 0; i < sets.length; i = i + 2) {
       let header = sets[i];
@@ -544,13 +538,13 @@ class PfeContentSet extends PFElement {
           // Capture the ID from the region or the pfe-id if they exist
           if (region.id || region.getAttribute("pfe-id")) piece.id = region.id || region.getAttribute("pfe-id");
 
-          // Attach the template item to the fragment
-          fragment.appendChild(piece);
+          // Attach the template item to the element tag
+          tagElement.appendChild(piece);
         });
       }
     }
 
-    return fragment;
+    return tagElement;
   }
 
   _copyToId(oldVal, newVal) {

elements/pfe-content-set/src/polyfills--pfe-content-set.js

@@ -8,36 +8,3 @@ if (window.NodeList && !NodeList.prototype.forEach) {
     }
   };
 }
-
-// @POLYFILL  Object.prototype.assign()
-// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign#Polyfill
-if (typeof Object.assign !== "function") {
-  // Must be writable: true, enumerable: false, configurable: true
-  Object.defineProperty(Object, "assign", {
-    value: function assign(target, varArgs) {
-      // .length of function is 2
-      "use strict";
-      if (target === null || target === undefined) {
-        throw new TypeError("Cannot convert undefined or null to object");
-      }
-
-      var to = Object(target);
-
-      for (var index = 1; index < arguments.length; index++) {
-        var nextSource = arguments[index];
-
-        if (nextSource !== null && nextSource !== undefined) {
-          for (var nextKey in nextSource) {
-            // Avoid bugs when hasOwnProperty is shadowed
-            if (Object.prototype.hasOwnProperty.call(nextSource, nextKey)) {
-              to[nextKey] = nextSource[nextKey];
-            }
-          }
-        }
-      }
-      return to;
-    },
-    writable: true,
-    configurable: true
-  });
-}