fix: remove navigation report height for specific id (#746)

* feat(pfe-navigation): report pfe-nav's height in a global CSS variable

* update changelog

* change the height var to be more BEMy

* set nav height variable to use px

* fixed merge conflict

* fixed variable name in readme

* fixed typo in readme

* fixed nav test to use correct variable name

* set height custom property to use underscores only if region is declared.

* added px value back into variable.

* removed px string from custom height property to match clientHeight.

* Updated nav readme, fixed typo and replaced ellipses with colon

* removing logic for setting css variable for a nav with an id

* removing attribute listener for id again

* code revisions

Co-authored-by: Michael Clayton <[email protected]>
Co-authored-by: Chris Doherty <[email protected]>
Co-authored-by: Kendall Totten <[email protected]>

Author: 4 people

elements/pfe-navigation/README.md

@@ -21,14 +21,14 @@ All slots are optional and can be left off if not needed. Slots prefixed with `m
     ```
     <a style="--pfe-navigation__logo--MinWidth: 160px" slot="logo" href="https://company.com"><img class="logo" src="https://via.placeholder.com/150x50.png" title="Company logo" /></a>
     ```
-- `search`: This slot can accept any html mark-up, however the `pfe-navigation-item` is recommended. When assigned to a navigation-item component, this slot renders a dropdown in the navigation for the search functionality. Inside the tray for the slot, we recommend tagging the search form or search functionality that includes the input and submit button with the attribute `pfe-navigation--mobile-search`. The element inside the search slot that is tagged with this attribute will be copied into the mobile menu and appear above the accordion for the main navigation. Example: 
+- `search`: This slot can accept any html mark-up, however the `pfe-navigation-item` is recommended. When assigned to a navigation-item component, this slot renders a dropdown in the navigation for the search functionality. Inside the tray for the slot, we recommend tagging the search form or search functionality that includes the input and submit button with the attribute `pfe-navigation--mobile-search`. The element inside the search slot that is tagged with this attribute will be copied into the mobile menu and appear above the accordion for the main navigation. Example:
 
     ```
     <pfe-navigation-item slot="search" pfe-icon="web-search">
       <h2 slot="trigger"><a href="#url-to-search-page">Search</a></h2>
     </pfe-navigation-item>
     ```
-- `language`: This slot can accept any html mark-up, however the `pfe-navigation-item` is recommended. When assigned to a navigation-item component, this slot renders the dropdown for the user to select the site language. Example: 
+- `language`: This slot can accept any html mark-up, however the `pfe-navigation-item` is recommended. When assigned to a navigation-item component, this slot renders the dropdown for the user to select the site language. Example:
 
     ```
     <pfe-navigation-item slot="language" pfe-icon="web-globe" slot="trigger">
@@ -40,7 +40,7 @@ All slots are optional and can be left off if not needed. Slots prefixed with `m
     ```
     <a href="/url-to-language-page" slot="mobile-language" pfe-icon="web-globe" hidden>English</a>
     ```
-- `login`: This slot can accept any html mark-up, however the `pfe-navigation-item` is recommended. When assigned to a navigation-item component, this slot renders the dropdown for the user to log into the site. Example: 
+- `login`: This slot can accept any html mark-up, however the `pfe-navigation-item` is recommended. When assigned to a navigation-item component, this slot renders the dropdown for the user to log into the site. Example:
 
     ```
     <pfe-navigation-item slot="login" pfe-icon="web-user">
@@ -51,7 +51,7 @@ All slots are optional and can be left off if not needed. Slots prefixed with `m
     ```
     <a href="/login" slot="mobile-login" pfe-icon="web-user" hidden>Login/Register</a>
     ```
-- `site-switcher`: This slot can accept any html mark-up, however the `pfe-navigation-item` is recommended. When assigned to a navigation-item component, this slot renders the dropdown for the site switcher, allowing the user to navigate an ecosystem of websites. Example: 
+- `site-switcher`: This slot can accept any html mark-up, however the `pfe-navigation-item` is recommended. When assigned to a navigation-item component, this slot renders the dropdown for the site switcher, allowing the user to navigate an ecosystem of websites. Example:
 
     ```
     <pfe-navigation-item slot="site-switcher" pfe-icon="web-grid-3x3">
@@ -68,8 +68,7 @@ All slots are optional and can be left off if not needed. Slots prefixed with `m
 
 ### CSS Variables
 
- - `--pfe-navigation--Height--actual`: when pfe-navigation initializes, it will create a global (on `body`) CSS variable that contains the height of the pfe-navigation item.  The primary uses for this is calculating the offset for anchor links, and for positioning a sticky sub-header below the pfe-navigation.  Note that multiple pfe-navigation elements will write the same variable, unless:
- - `--pfe-navigation__${ID}--Height--actual`: like the previous variable, but if the pfe-navigation has an `id` attribute, it will be appended to the CSS variable name.  This makes it possible to distinguish the heights of multiple pfe-navigation elements on the same page.
+ - `--pfe-navigation--Height--actual`: When `pfe-navigation` initializes, it will create a global (on `body`) CSS variable that contains the height of the `pfe-navigation` element. Possible uses include calculating the offset for anchor links or positioning a sticky sub-header below the `pfe-navigation`.  Note that multiple `pfe-navigation` elements will write the same variable.
 
 ---
 

elements/pfe-navigation/src/pfe-navigation.js

@@ -55,7 +55,7 @@ class PfeNavigation extends PFElement {
   }
 
   static get observedAttributes() {
-    return ["pfe-full-width", "id"];
+    return ["pfe-full-width"];
   }
 
   constructor() {
@@ -165,15 +165,6 @@ class PfeNavigation extends PFElement {
     this._observer.disconnect();
   }
 
-  attributeChangedCallback(name, oldValue, newValue) {
-    super.attributeChangedCallback(name, oldValue, newValue);
-    switch (name) {
-      case "id":
-        this._reportHeight();
-        break;
-    }
-  }
-
   _resizeHandler(event) {
     // Set the visibility of items
     this._setVisibility(this.offsetWidth);
@@ -379,16 +370,9 @@ class PfeNavigation extends PFElement {
    * Used to position sticky subnavigation items under this.
    *
    * The name of the global CSS variable is `--pfe-navigation--Height--actual`.
-   * If this nav has an `id` attribute, the id will be appended to the variable
-   * name to distinguish it from other pfe-navigation items on the page
-   * (unlikely, but imagine a demo page with 20 example pfe-navigation elements
-   * in different states, and one genuine pfe-navigation element at the top
-   * used for actual navigation).
    */
   _reportHeight() {
-    const cssVarName = `--pfe-navigation${
-      this.id ? `__${this.id}--` : "--"
-    }Height--actual`;
+    const cssVarName = `--${this.tag}--Height--actual`;
     const height = this.clientHeight + "px";
     document.body.style.setProperty(cssVarName, height);
   }

elements/pfe-navigation/test/pfe-navigation_test.html

@@ -612,14 +612,6 @@ <h3 slot="trigger"><a href="#">Products</a></h3>
           // check the exact height (race condition over which writes its var
           // last).  Just check that the var was created.
           assert.isDefined(reportedHeight, "pfe-navigation without id reports height to default css variable");
-
-          // should also create a namespaced var if the nav has an ID
-          pfeNavigation.setAttribute("id", "ABCDEFG");
-
-          const reportedHeightNamespaced = document.body.style.getPropertyValue("--pfe-navigation__ABCDEFG--Height--actual").replace("px", "");
-
-          assert.isDefined(reportedHeightNamespaced, "pfe-navigation with id reports height to namespaced css variable");
-          assert.equal(pfeNavigation.clientHeight.toString(), reportedHeightNamespaced, "pfe-navigation with id reports the correct height");
         });
       });