fix: fixed offset to work more reliably and added some docs (#908)

* fixed offset to work more reliably and added some docs

* fixed silly mistake

* cleaned up a bit

* removed duplicated header

* changed selector to find the right element

* removed unnecessary check

* added support for using a css variable for offset

* some small performance and best practices fixed, added support for stylesheets to determine offset

* updated readme to have more about offset and the new css feature

* adding an offset attribute observer

* removing some console logs

* fixing some if else logic

* adding additional tests

* adding a couple of enhancements to the demo file

* switching from --pfe-jump-links-nav--offset to --pfe-jump-links-panel--offset

We agreed that it makes more sense to have the offset on the panel instead of the nav since the panel is watching for scroll events.

* Update elements/pfe-jump-links/README.md

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

* Update elements/pfe-jump-links/README.md

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

* Update elements/pfe-jump-links/README.md

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

* Update elements/pfe-jump-links/src/pfe-jump-links.js

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

* Update elements/pfe-jump-links/src/pfe-jump-links.js

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

* Update elements/pfe-jump-links/src/pfe-jump-links.js

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

* Update elements/pfe-jump-links/src/pfe-jump-links.js

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

* Update elements/pfe-jump-links/src/pfe-jump-links.js

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

* updating components with pfe-c prefixes

Co-authored-by: Daniel Faucette <[email protected]>
Co-authored-by: Kendall Totten <[email protected]>
Co-authored-by: Kyle Buchanan <[email protected]>
Co-authored-by: [ Cassondra ] <[email protected]>

Author: 4 people

elements/pfe-jump-links/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 1.0.0-prerelease.50 ( TBD )
+
+Tag: [1.0.0-prerelease.50](https://github.com/patternfly/patternfly-elements/releases/tag/1.0.0-prerelease.50)
+
+- [code](url) offset - fixed offset attribute to work more reliably and updated docs
+
 ## 1.0.0-prerelease.39 ( TBD )
 
 Tag: [1.0.0-prerelease.39](https://github.com/patternfly/patternfly-elements/releases/tag/1.0.0-prerelease.39)

elements/pfe-jump-links/README.md

@@ -69,11 +69,20 @@ No extra roles or aria labels are required because we're using standard html tag
 
 ## Slots
 
-- `namedSlot`: The only slot this component has is the heading. The rest of the component works by creating a mirror shadowRoot based on the Light DOM markup. 
+
+- `pfe-jump-links-nav--heading`:  The rest of the component works by creating a mirror shadowRoot based on the Light DOM markup. 
+
 
 ## Attributes
 
-- `attr`: autobuild Flips the switch on the component to create its own markup for the navigation. If you use this attribute, then no experience is given for non-JS runtimes and the component will crawl the associated panel to come up with its own list of links.
+
+- `pfe-c-autobuild`: Flips the switch on the component to create its own markup for the navigation. You can add `pfe-jump-links-panel__section` to each section that should show in the nav. If you want to use sub-sections add `has-sub-section` to the parent section that should always show and the `sub-section` class to the children of that section. If you use this attribute, keep in mind that non-JavaScript environments (some search engines, browsers with JS disabled) will not see the proper markup.
+
+- `sr-text`: This attribute is read when the component upgrades to provide the innerText of the heading. If there is no `sr-text` attribute then the component defaults to "JUMP TO SECTION". This attribute is to enable translations and internationalization.
+
+- `pfe-c-offset`: This attribute determines the distance from the top of the browser window to trigger a switch from one link being active to the next. For instance `pfe-c-offset="600"` would mean that threshold flips at 600px from the top. The default is set at 200, and if you desire 200px then you can leave this attribute off. The `pfe-c-offset` attribute should be placed on `pfe-jump-links-panel`. There is a css solution to control the offset, however the attribute value takes precedence over css. To read more about a css solution see below.
+
+- `style="--pfe-jump-links-panel--offset: {integer};"`: You can control offset in your styling layer as well. This value can be set directly on the component inside a style attribute, e.g. `style="--pfe-jump-links-panel--offset: 100;"` or using the appropriate selector in another file. Please note that adding an attribute will take precedence over a css value. At the moment only integer values passed to this key are valid. No other values are supported. This means that passing "300px", "2rem","calc(100% - 12px)" will all result in JavaScript errors. You should pass a number that correlates to pixels. To read about the `pfe-c-offset` attribute, see above.
 
 ## Events
 This component fires an event when a link is made active.

elements/pfe-jump-links/demo/index.html

@@ -61,7 +61,36 @@ <h1><code>pfe-jump-links</code></h1>
       top: 0;
     }
 
+    .special {
+      --pfe-jump-links-panel--offset: 200;
+    }
+
+    /* 
+     * Fixing the issue where the headers are under the stick nav
+     * https://stackoverflow.com/questions/4086107/fixed-page-header-overlaps-in-page-anchors
+     */
+    #horizontalJumpLinks .pfe-jump-links-panel__section::before {
+      content: "";
+      display: block;
+      height: 96px;
+      margin: -96px 0 0;
+    }
 
+    @media(max-width: 992px) {
+      /*
+       * fix for when we have a mobile nav that is sticky
+       */
+      .pfe-jump-links-panel__section::before {
+        content: "";
+        display: block;
+        height: 68px;
+        margin: -68px 0 0;
+      }
+
+      .special{
+        --pfe-jump-links-panel--offset: 220;
+      }   
+    }
   </style>
 
   <h2 style="text-align: center; margin: 60px auto;">PFE Jump Links - standard</h2>
@@ -81,7 +110,7 @@ <h4 slot="pfe-jump-links-nav--heading">Jump to section</h4>
       </pfe-jump-links-nav>
     </div>
     <div class="pfe-l-grid__item pfe-m-12-col pfe-m-9-col-on-lg">
-      <pfe-jump-links-panel scrolltarget="jumplinks1">
+      <pfe-jump-links-panel class="special" pfe-c-scrolltarget="jumplinks1">
         <div>
           <h1 class="pfe-jump-links-panel__section" id="section1">Section 1</h1>
           <p>Lorem ipsum dolor amet umami vaporware actually church-key keytar, hell of roof party unicorn helvetica
@@ -324,7 +353,7 @@ <h4 slot="pfe-jump-links-nav--heading">Jump to section</h4>
       </pfe-jump-links-nav>
     </div>
     <div class="pfe-l-grid__item pfe-m-12-col pfe-m-9-col-on-lg">
-      <pfe-jump-links-panel scrolltarget="jumplinks2">
+      <pfe-jump-links-panel pfe-c-scrolltarget="jumplinks2">
         <div>
           <h1 class="pfe-jump-links-panel__section" id="section10">Section 1</h1>
           <p>Lorem ipsum dolor amet umami vaporware actually church-key keytar, hell of roof party unicorn helvetica
@@ -554,7 +583,7 @@ <h4 slot="pfe-jump-links-nav--heading" id="pfe-jump-links-nav--heading">Jump to
       </pfe-jump-links-nav>
     </div>
     <div class="pfe-l-grid__item pfe-m-12-col pfe-m-9-col-on-lg">
-      <pfe-jump-links-panel scrolltarget="jumplinks8">
+      <pfe-jump-links-panel pfe-c-scrolltarget="jumplinks8">
         <div>
           <h1 class="pfe-jump-links-panel__section" id="section6">Section 6</h1>
           <p>Lorem ipsum dolor amet umami vaporware actually church-key keytar, hell of roof party unicorn helvetica
@@ -776,8 +805,8 @@ <h1 class="pfe-jump-links-panel__section" id="section1000">Section 10</h1>
       </pfe-jump-links-panel>
     </div>
   </div>
-  <div>
-    <pfe-jump-links-nav horizontal id="jumplinks9" offset="30" sr-text="Page navigation">
+  <div id="horizontalJumpLinks">
+    <pfe-jump-links-nav pfe-c-horizontal id="jumplinks9" sr-text="Page navigation">
       <h4 slot="pfe-jump-links-nav--heading">Jump to section</h4>
       <img src="https://static.redhat.com/libs/redhat/brand-assets/2/corp/logo--200.png" alt="" slot="pfe-jump-links-nav--logo">
       <ul>
@@ -790,7 +819,7 @@ <h4 slot="pfe-jump-links-nav--heading">Jump to section</h4>
       <pfe-cta pfe-priority="primary" class="PFElement" slot="pfe-jump-links-nav--cta" pfelement="" has_link=""
         on="dark"> <a href="https://www.redhat.com#primary">Link</a> </pfe-cta>
     </pfe-jump-links-nav>
-    <pfe-jump-links-panel scrolltarget="jumplinks9" style="margin-bottom: 500px;">
+    <pfe-jump-links-panel pfe-c-scrolltarget="jumplinks9" style="margin-bottom: 500px;">
       <div style="margin-bottom: 500px;">
         <h1 class="pfe-jump-links-panel__section" id="section100">Section 1</h1>
         <p>Lorem ipsum dolor amet umami vaporware actually church-key keytar, hell of roof party unicorn helvetica lomo
@@ -999,7 +1028,7 @@ <h4 slot="pfe-jump-links-nav--heading">Jump to section</h4>
         </pfe-jump-links-nav>
       </div>
       <div class="pfe-l-grid__item pfe-m-12-col pfe-m-9-col-on-lg">
-        <pfe-jump-links-panel scrolltarget="jumplinks1000">
+        <pfe-jump-links-panel pfe-c-scrolltarget="jumplinks1000">
           <div>
             <h1 class="pfe-jump-links-panel__section" id="section10000">Section 1</h1>
             <p>Lorem ipsum dolor amet umami vaporware actually church-key keytar, hell of roof party unicorn helvetica

elements/pfe-jump-links/src/pfe-jump-links-nav.html

@@ -1,4 +1,4 @@
-<!-- ${this.hasAttribute("horizontal") ? `` :
+<!-- ${this.hasAttribute("pfe-c-horizontal") ? `` :
 `<pfe-accordion>
   <pfe-accordion-header>
   </pfe-accordion-header>
@@ -21,13 +21,13 @@ <h2 class="sr-only" hidden>Page navigation</h2>
       : ``
       }
     </nav>
-    ${this.hasAttribute("horizontal") ? `` :
+    ${this.hasAttribute("pfe-c-horizontal") ? `` :
   `</pfe-accordion-panel>
 </pfe-accordion>`
 }
   old version^^ -->
 
-  ${this.hasAttribute("horizontal") ? `` :
+  ${this.hasAttribute("pfe-c-horizontal") ? `` :
   `<pfe-accordion>
     <pfe-accordion-header>
     </pfe-accordion-header>
@@ -40,7 +40,7 @@ <h2 class="sr-only" hidden>Page navigation</h2>
         <div id="container"></div>
         <slot class="pfe-jump-links-nav__cta" name="pfe-jump-links-nav--cta"></slot>
       </nav>
-      ${this.hasAttribute("horizontal") ? `` :
+      ${this.hasAttribute("pfe-c-horizontal") ? `` :
       `</pfe-accordion-panel>
   </pfe-accordion>`
   }

elements/pfe-jump-links/src/pfe-jump-links-nav.scss

@@ -284,7 +284,7 @@ nav {
   display: none;
 }
 
-:host([horizontal]) {
+:host([pfe-c-horizontal]) {
   padding: 0;
   top: 0;
   width: 100%;

elements/pfe-jump-links/src/pfe-jump-links.js

@@ -69,7 +69,9 @@ class PfeJumpLinksNav extends PFElement {
     this._menuContainer = this.shadowRoot.querySelector("#container");
     this._observer = new MutationObserver(this._mutationCallback);
     this._reportHeight = this._reportHeight.bind(this);
-    this.panel = document.querySelector(`[scrolltarget=${this.id}]`);
+    this.panel = document.querySelector(`[pfe-c-scrolltarget=${this.id}]`);
+
+    window.addEventListener("resize", () => {});
   }
 
   connectedCallback() {
@@ -95,23 +97,22 @@ class PfeJumpLinksNav extends PFElement {
         }
 
         let html = "";
-        if (this.querySelector(".pfe-jump-links-nav--heading")) {
-          html = this.querySelector(".pfe-jump-links-nav--heading").cloneNode(
-            true
-          );
+        if (this.querySelector("[slot='pfe-jump-links-nav--heading']")) {
+          html = this.querySelector(
+            "[slot='pfe-jump-links-nav--heading']"
+          ).cloneNode(true);
         }
-        if (!this.hasAttribute("horizontal")) {
-          if (html) {
-            this.shadowRoot
-              .querySelector("pfe-accordion-header")
-              .appendChild(html);
-          }
+        if (!this.hasAttribute("pfe-c-horizontal") && html !== "") {
+          this.shadowRoot
+            .querySelector("pfe-accordion-header")
+            .appendChild(html);
         } else {
+          const heading = document.createElement("h3");
+          heading.id = "pfe-jump-links-nav--heading";
+
           this.shadowRoot
             .querySelector("pfe-accordion-header")
-            .appendChild(
-              document.createElement("h3#pfe-jump-links-nav--heading")
-            );
+            .appendChild(heading);
           this.shadowRoot.querySelector(
             "#pfe-jump-links-nav--heading"
           ).innerHTML = "Jump to section";
@@ -122,7 +123,7 @@ class PfeJumpLinksNav extends PFElement {
 
     this._observer.observe(this, pfeJumpLinksNavObserverConfig);
 
-    this.panel = document.querySelector(`[scrolltarget="${this.id}"]`);
+    this.panel = document.querySelector(`[pfe-c-scrolltarget="${this.id}"]`);
 
     this.panel.addEventListener(
       PfeJumpLinksPanel.events.change,
@@ -147,7 +148,9 @@ class PfeJumpLinksNav extends PFElement {
     const buildLinkList = () => {
       let linkList = ``;
       if (!this.panel) {
-        this.panel = document.querySelector(`[scrolltarget="${this.id}"]`);
+        this.panel = document.querySelector(
+          `[pfe-c-scrolltarget="${this.id}"]`
+        );
       }
       let panelSections = this.panel.querySelectorAll(
         ".pfe-jump-links-panel__section"
@@ -234,14 +237,14 @@ class PfeJumpLinksNav extends PFElement {
     }
     if (
       (this.has_slot("logo") || this.has_slot("link")) &&
-      !this.hasAttribute("horizontal")
+      !this.hasAttribute("pfe-c-horizontal")
     ) {
       console.warn(
         `${PfeJumpLinks.tag}: logo and link slots NOT supported in vertical jump links`
       );
     }
     if (this.children[1].tagName !== "UL") {
-      if (!this.hasAttribute("horizontal")) {
+      if (!this.hasAttribute("pfe-c-horizontal")) {
         console.warn(
           `${PfeJumpLinks.tag}: The top-level list of links MUST be a <ul>`
         );
@@ -284,30 +287,40 @@ class PfeJumpLinksPanel extends PFElement {
     };
   }
 
+  get offsetValue() {
+    return this.sectionMargin || this.customVar;
+  }
+
   static get PfeType() {
     return PFElement.PfeTypes.Content;
   }
 
+  static get observedAttributes() {
+    return ["pfe-c-offset"];
+  }
+
   constructor() {
     super(PfeJumpLinksPanel, { type: PfeJumpLinksPanel.PfeType });
     this._init = this._init.bind(this);
     this._slot = this.shadowRoot.querySelector("slot");
     this._slot.addEventListener("slotchange", this._init);
     this._scrollCallback = this._scrollCallback.bind(this);
     this._mutationCallback = this._mutationCallback.bind(this);
+    this._handleResize = this._handleResize.bind(this);
     this._observer = new MutationObserver(this._mutationCallback);
     this.currentActive = null;
-    this.sectionMargin = this.getAttribute("offset") || 200;
     this.currentActive = 0;
     this.current = -1;
-    this.nav = this._getNav();
+    window.addEventListener("resize", this._handleResize);
   }
 
   connectedCallback() {
     super.connectedCallback();
+    this.nav = this._getNav();
     this._init();
-
-    if (this.nav && this.nav.hasAttribute("autobuild")) {
+    this.sectionMargin = this.getAttribute("pfe-c-offset");
+    this.customVar = this.cssVariable("--pfe-jump-links-panel--offset") || 200;
+    if (this.nav && this.nav.hasAttribute("pfe-c-autobuild")) {
       this.nav._rebuildNav();
     }
 
@@ -318,11 +331,22 @@ class PfeJumpLinksPanel extends PFElement {
     this._observer.disconnect();
     window.removeEventListener("scroll");
     this._slot.removeEventListener("slotchange", this._init);
+    window.removeEventListener("resize", this._handleResize);
+  }
+
+  attributeChangedCallback(attr, oldVal, newVal) {
+    super.attributeChangedCallback(attr, oldVal, newVal);
+
+    switch (attr) {
+      case "pfe-c-offset":
+        this.sectionMargin = newVal;
+        break;
+    }
   }
 
   _init() {
     window.addEventListener("scroll", this._scrollCallback);
-    this.scrollTarget = this.getAttribute("scrolltarget");
+    this.scrollTarget = this.getAttribute("pfe-c-scrolltarget");
     this.JumpLinksNav = document.querySelector(`#${this.scrollTarget}`);
     this.sections = this.querySelectorAll(".pfe-jump-links-panel__section");
 
@@ -331,9 +355,15 @@ class PfeJumpLinksPanel extends PFElement {
     }
   }
 
+  _handleResize() {
+    this.nav._reportHeight();
+    this.sectionMargin = this.getAttribute("pfe-c-offset");
+    this.customVar = this.cssVariable("--pfe-jump-links-panel--offset") || 200;
+  }
+
   _getNav() {
     return document.querySelector(
-      `pfe-jump-links-nav#${this.getAttribute("scrolltarget")}`
+      `pfe-jump-links-nav#${this.getAttribute("pfe-c-scrolltarget")}`
     );
   }
 
@@ -405,7 +435,7 @@ class PfeJumpLinksPanel extends PFElement {
     //If we didn't get nav in the constructor, grab it now
     if (!this.nav) {
       this.nav = document.querySelector(
-        `pfe-jump-links-nav#${this.getAttribute("scrolltarget")}`
+        `pfe-jump-links-nav#${this.getAttribute("pfe-c-scrolltarget")}`
       );
     }
     //If we want the nav to be built automatically, re-init panel and rebuild nav
@@ -423,7 +453,6 @@ class PfeJumpLinksPanel extends PFElement {
   _scrollCallback() {
     let sections;
     let menu_links;
-    let sectionMargin;
     //Check sections to make sure we have them (if not, get them)
     if (!this.sections || typeof this.sections === "undefined") {
       this.sections = this.querySelectorAll(".pfe-jump-links-panel__section");
@@ -436,17 +465,11 @@ class PfeJumpLinksPanel extends PFElement {
       menu_links = this.menu_links;
     }
 
-    if (!this.sectionMargin) {
-      sectionMargin = 200;
-    } else {
-      sectionMargin = this.sectionMargin;
-    }
-
     // Make an array from the node list
     const sectionArr = [...sections];
     // Get all the sections that match this point in the scroll
     const matches = sectionArr
-      .filter(section => window.scrollY >= section.offsetTop - sectionMargin)
+      .filter(section => window.scrollY >= section.offsetTop - this.offsetValue)
       .reverse();
 
     //Identify the last one queried as the current section