fix: on mutation, apply cascade to new elements only (#1440)

* fix: on mutation, cascade to new elements only

When a cascade is triggered by a mutation observer, copy attributes down
to the newly created elements only.  Previously, I had code implemented
that tried to do this but it wasn't working.  It was finding which
cascade selectors matched the new elements, but then copied the
attributes to _all_ elements that matched those selectors.

* update changelog

* move IE11 observer reattach to same branch as detach

* update changelog

* Update CHANGELOG-1.x.md

* fix incorrect iteration over cascading attrNames

* Update CHANGELOG-1.x.md

Co-authored-by: [ Cassondra ] <[email protected]>

* add jsdoc params to _cascadeAttribute

* Update elements/pfelement/src/pfelement.js

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: [ Cassondra ] <[email protected]>

Author: 3 people

CHANGELOG-1.x.md

@@ -1,7 +1,8 @@
 # 1.10.1 (2021)
 
-- [](https://github.com/patternfly/patternfly-elements/commit/) fix: pfelement - only resetContext on a nested child element during contextUpdate if resetContext is available on the child
-- [fdc60ab](https://github.com/patternfly/patternfly-elements/commit/) fix: pfe-tabs issue with active border color on horizontal/wind (#1585)
+- [b717636](https://github.com/patternfly/patternfly-elements/commit/b717636782ebce33b4549ba9f68ffff09c036889) fix: pfelement - only resetContext on a nested child element during contextUpdate if resetContext is available on the child
+- [7f9c30e](https://github.com/patternfly/patternfly-elements/commit/7f9c30e0a312e538b1787a91824c6e84d1aa261a) fix: pfe-tabs issue with active border color on horizontal/wind (#1585)
+- [](https://github.com/patternfly/patternfly-elements/commit/) fix: on mutation, apply cascade to new elements only
 
 # 1.10.0 (2021-07-08)
 

elements/pfelement/src/pfelement.js

@@ -467,7 +467,7 @@ class PFElement extends HTMLElement {
       // If the property/attribute pair has a cascade target, copy the attribute to the matching elements
       // Note: this handles the cascading of new/updated attributes
       if (propDef.cascade) {
-        this._copyAttribute(attr, this._pfeClass._convertSelectorsToArray(propDef.cascade));
+        this._cascadeAttribute(attr, this._pfeClass._convertSelectorsToArray(propDef.cascade));
       }
     }
   }
@@ -561,33 +561,34 @@ class PFElement extends HTMLElement {
 
       let selectors = Object.keys(cascade);
       // Find out if anything in the nodeList matches any of the observed selectors for cacading properties
-      if (nodeList) {
-        selectors = [];
-        [...nodeList].forEach((nodeItem) => {
-          Object.keys(cascade).map((selector) => {
-            // if this node has a match function (i.e., it's an HTMLElement, not
-            // a text node), see if it matches the selector, otherwise drop it (like it's hot).
-            if (nodeItem.matches && nodeItem.matches(selector)) {
-              selectors.push(selector);
-            }
-          });
-        });
-      }
-
-      // If a match was found, cascade each attribute to the element
       if (selectors) {
-        const components = selectors
-          .filter((item) => item.slice(0, prefix.length + 1) === `${prefix}-`)
-          .map((name) => customElements.whenDefined(name));
-
-        if (components)
-          Promise.all(components).then(() => {
-            this._copyAttributes(selectors, cascade);
+        if (nodeList) {
+          [...nodeList].forEach(nodeItem => {
+            selectors.forEach(selector => {
+              // if this node has a match function (i.e., it's an HTMLElement, not
+              // a text node), see if it matches the selector, otherwise drop it (like it's hot).
+              if (nodeItem.matches && nodeItem.matches(selector)) {
+                let attrNames = cascade[selector];
+                // each selector can match multiple properties/attributes, so
+                // copy each of them
+                attrNames.forEach(attrName => this._copyAttribute(attrName, nodeItem));
+              }
+            });
           });
-        else this._copyAttributes(selectors, cascade);
+        } else {
+          // If a match was found, cascade each attribute to the element
+          const components = selectors
+            .filter(item => item.slice(0, prefix.length + 1) === `${prefix}-`)
+            .map(name => customElements.whenDefined(name));
+
+          if (components)
+            Promise.all(components).then(() => {
+              this._cascadeAttributes(selectors, cascade);
+            });
+          else this._cascadeAttributes(selectors, cascade);
+        }
       }
 
-      // @TODO This is here for IE11 processing; can move this after deprecation
       if (this._rendered && this._cascadeObserver)
         this._cascadeObserver.observe(this, {
           attributes: true,
@@ -662,7 +663,8 @@ class PFElement extends HTMLElement {
     for (let mutation of mutationsList) {
       // If a new node is added, attempt to cascade attributes to it
       if (mutation.type === "childList" && mutation.addedNodes.length) {
-        this.cascadeProperties(mutation.addedNodes);
+        const nonTextNodes = [...mutation.addedNodes].filter(n => n.nodeType !== HTMLElement.TEXT_NODE);
+        this.cascadeProperties(nonTextNodes);
       }
     }
   }
@@ -954,23 +956,39 @@ class PFElement extends HTMLElement {
     return propName;
   }
 
-  _copyAttributes(selectors, set) {
-    selectors.forEach((selector) => {
-      set[selector].forEach((attr) => {
-        this._copyAttribute(attr, selector);
+  _cascadeAttributes(selectors, set) {
+    selectors.forEach(selector => {
+      set[selector].forEach(attr => {
+        this._cascadeAttribute(attr, selector);
       });
     });
   }
 
-  _copyAttribute(name, to) {
+  /**
+   * Trigger a cascade of the named attribute to any child elements that match
+   * the `to` selector.  The selector can match elements in the light DOM and
+   * shadow DOM.
+   * @param {String} name The name of the attribute to cascade (not necessarily the same as the property name).
+   * @param {String} to A CSS selector that matches the elements that should received the cascaded attribute.  The selector will be applied within `this` element's light and shadow DOM trees.
+   */
+  _cascadeAttribute(name, to) {
     const recipients = [...this.querySelectorAll(to), ...this.shadowRoot.querySelectorAll(to)];
-    const value = this.getAttribute(name);
-    const fname = value == null ? "removeAttribute" : "setAttribute";
+
     for (const node of recipients) {
-      node[fname](name, value);
+      this._copyAttribute(name, node);
     }
   }
 
+  /**
+   * Copy the named attribute to a target element.
+   */
+  _copyAttribute(name, el) {
+    this.log(`copying ${name} to ${el}`);
+    const value = this.getAttribute(name);
+    const fname = value == null ? "removeAttribute" : "setAttribute";
+    el[fname](name, value);
+  }
+
   static _convertSelectorsToArray(selectors) {
     if (selectors) {
       if (typeof selectors === "string") return selectors.split(",");