fix: [pfe-primary-detail] usable in IE11 (#1351) (#1352)

* 1351: Fixed primary-detail so it's usable in IE11

Avoided replacing nav item markup, which was causing bugs and duplicate content in IE11
Added CSS for IE

* Removing duplicate code

* Update 11ty documentation after latest merge from master

* Remove console log

* Update changelog and baseline images

* Newline in CHANGELOG fix

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

Author: 3 people

CHANGELOG-1.x.md

@@ -1,3 +1,7 @@
+# 1.5.2 (2021)
+
+- [](https://github.com/patternfly/patternfly-elements/commit/) fix: pfe-primary-detail IE11 rendering fix
+
 # 1.5.1 (2021-04-15)
 
 - [48b3c30](https://github.com/patternfly/patternfly-elements/commit/48b3c305367d41fefbb1b01fb3d9189bf96a85f5) fix: adjust word-count calculation on pfe-readtime

elements/pfe-primary-detail/docs/index.md

@@ -18,8 +18,6 @@ tags:
 ## Overview
 A primary-detail layout is an interface that shows a list of items and the corresponding details of the selected item. This component is an implementation of one of the "Primary detail simple list in card" from [Patternfly React](https://www.patternfly.org/v4/demos/primary-detail).
 
-<br><br><br><br>
-
 <pfe-primary-detail>
   <h3 slot="details-nav">Section 1: Infrastructure and Management</h3>
   <div slot="details">
@@ -64,6 +62,11 @@ A primary-detail layout is an interface that shows a list of items and the corre
 <pfe-primary-detail>
 :::
 
+::: section
+## Accessibility
+The provided markup should be semantic so that if the component can't load, the user still has an appropriate experience.  Once it upgrades, the appropriate tab interactions and markup for assistive tech is added to the component.
+:::
+
 ::: section
 ## Installation
 ```shell
@@ -176,24 +179,16 @@ npm install @patternfly/pfe-primary-detail
 ## Slots
 For this component to work, there should be an equal number of `details-nav` and `details` slotted elements.
 
-### details-nav
-Should be added to each heading, it will build the nav that shows the related content.
-
-### details
-Should be added to the content, which should directly follow the heading it relates to.
-
-### details-nav--header
-In case content needs to be added at the top of the nav area. Will not be matched up with `details` content.
-
-### details-nav--footer
-In case contnet needs to be added at the bottom of the nav. Will not be matched up with `details` content.
+- **details-nav**: Added to each heading; builds the navigation that shows the related content.
+- **details**: Added to the content, directly following the heading to which it relates.
+- **details-nav--header**: In case content needs to be added at the top of the navigation area; this will not be matched up with `details` content.
+- **details-nav--footer**: In case content needs to be added at the bottom of the navigation; will not be matched up with `details` content.
 :::
 
 ::: section
 ## Attributes
 
-### consistent-height
-Makes sure the primary details element doesn't change height when a different `details` item is shown.
+- **consistent-height**: Makes sure the primary details element doesn't change height when a different `details` item is shown.
 :::
 
 ::: section

elements/pfe-primary-detail/package.json

@@ -4,6 +4,7 @@
   "pfelement": {
     "className": "PfePrimaryDetail",
     "elementName": "pfe-primary-detail",
+    "preview": "pfe-primary-detail.png",
     "files": {
       "styles": "./src/pfe-primary-detail.scss",
       "template": "./src/pfe-primary-detail.html",

elements/pfe-primary-detail/pfe-primary-detail.png

@@ -10,8 +10,8 @@ const lightDomObserverConfig = {
   childList: true
 };
 
-// @todo Add keyboard controls for arrows?
-// @todo Add functions to open a specific item by index or ID
+// @TODO Add keyboard controls for arrows?
+// @TODO Add functions to open a specific item by index or ID
 class PfePrimaryDetail extends PFElement {
   static get tag() {
     return "pfe-primary-detail";
@@ -88,6 +88,7 @@ class PfePrimaryDetail extends PFElement {
 
   constructor() {
     super(PfePrimaryDetail, { type: PfePrimaryDetail.PfeType });
+    this.isIE = !!window.MSInputMethodContext && !!document.documentMode;
 
     this._handleHideShow = this._handleHideShow.bind(this);
     this._initDetailsNav = this._initDetailsNav.bind(this);
@@ -120,7 +121,10 @@ class PfePrimaryDetail extends PFElement {
     this._observer.observe(this, lightDomObserverConfig);
 
     // Set first item as active for initial load
-    this._handleHideShow({ target: this._slots.detailsNav[0] });
+    this._handleHideShow({
+      target: this._slots.detailsNav[0],
+      pfeInitializing: true
+    });
   }
 
   disconnectedCallback() {
@@ -140,43 +144,30 @@ class PfePrimaryDetail extends PFElement {
    */
   _initDetailsNav(detailNavElement, index) {
     // Don't re-init anything that's been initialized already
-    if (detailNavElement.tagName === "BUTTON" && detailNavElement.dataset.index && detailNavElement.id) {
+    if (detailNavElement.hasAttribute("role") && detailNavElement.dataset.index && detailNavElement.id) {
       // Make sure the data-index attribute is up to date in case order has changed
       detailNavElement.dataset.index = index;
       return;
     }
 
-    let attr = detailNavElement.attributes;
-    const toggle = document.createElement("button");
-
-    toggle.innerHTML = detailNavElement.innerHTML;
-
-    // Copy over attributes from original element that aren't in denyList
-    [...attr].forEach(detailNavElement => {
-      if (!denyListAttributes.includes(detailNavElement.name)) {
-        toggle.setAttribute(detailNavElement.name, detailNavElement.value);
-      }
-    });
-
     // Set data-index attribute
-    toggle.dataset.index = index;
+    detailNavElement.dataset.index = index;
 
     // If the detailNavElement does not have a ID, set a unique ID
     if (!detailNavElement.id) {
-      toggle.setAttribute(
+      detailNavElement.setAttribute(
         "id",
         `pfe-detail-toggle-${Math.random()
           .toString(36)
           .substr(2, 9)}`
       );
     }
 
-    toggle.setAttribute("role", "tab");
-    toggle.setAttribute("aria-selected", "false");
+    detailNavElement.setAttribute("role", "tab");
+    detailNavElement.setAttribute("aria-selected", "false");
 
-    toggle.addEventListener("click", this._handleHideShow);
-    this._slots.detailsNav[index] = toggle;
-    detailNavElement.replaceWith(toggle);
+    detailNavElement.addEventListener("click", this._handleHideShow);
+    this._slots.detailsNav[index] = detailNavElement;
   }
 
   /**
@@ -217,6 +208,7 @@ class PfePrimaryDetail extends PFElement {
 
     // Leave a reliable indicator that this has been initialized so we don't do it again
     detail.dataset.processed = true;
+    detail.hidden = true;
   }
 
   /**
@@ -264,13 +256,21 @@ class PfePrimaryDetail extends PFElement {
       return;
     }
 
-    const currentToggle = this._slots.detailsNav.find(
-      toggle => toggle.hasAttribute("aria-selected") && toggle.getAttribute("aria-selected") === "true"
-    );
+    let currentToggle = null;
+
+    // Find the active toggle by looking through them all and finding the ones with aria-selected set
+    for (let index = 0; index < this._slots.detailsNav.length; index++) {
+      const toggle = this._slots.detailsNav[index];
+      if (toggle.hasAttribute("aria-selected") && toggle.getAttribute("aria-selected") === "true") {
+        currentToggle = toggle;
+        break;
+      }
+    }
 
     // Get details elements
     const nextDetails = this._slots.details[parseInt(nextToggle.dataset.index)];
 
+
     if (currentToggle) {
       const currentDetails = this._slots.details[parseInt(currentToggle.dataset.index)];
 
@@ -279,6 +279,7 @@ class PfePrimaryDetail extends PFElement {
 
       // Remove Current Detail's attributes
       currentDetails.setAttribute("aria-hidden", "true");
+      currentDetails.hidden = true;
 
       this.emitEvent(PfePrimaryDetail.events.hiddenTab, {
         detail: {
@@ -293,6 +294,7 @@ class PfePrimaryDetail extends PFElement {
 
     // Add active attributes to Next Details
     nextDetails.setAttribute("aria-hidden", "false");
+    nextDetails.hidden = false;
 
     this.emitEvent(PfePrimaryDetail.events.shownTab, {
       detail: {
@@ -301,12 +303,14 @@ class PfePrimaryDetail extends PFElement {
       }
     });
 
-    // Set focus to pane
-    const firstFocusableElement = nextDetails.querySelector(
-      'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
-    );
-    if (firstFocusableElement) {
-      firstFocusableElement.focus();
+    // Set focus to pane if this isn't initialization
+    if (typeof e.pfeInitializing === "undefined") {
+      const firstFocusableElement = nextDetails.querySelector(
+        'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
+      );
+      if (firstFocusableElement) {
+        firstFocusableElement.focus();
+      }
     }
   }
 }

elements/pfe-primary-detail/src/pfe-primary-detail.js

@@ -26,11 +26,14 @@ $LOCAL-VARIABLES: (
 }
 
 :host {
-  display: grid;
-  grid-template-columns: pfe-local(GridTemplateColumns);
-  border: pfe-local(Border);
-  align-items: stretch;
-  justify-content: stretch;
+  display: flex;
+  @supports (display: grid) {
+    display: grid;
+    grid-template-columns: pfe-local(GridTemplateColumns);
+    border: pfe-local(Border);
+    align-items: stretch;
+    justify-content: stretch;
+  }
 }
 
 :host([hidden]) {
@@ -42,6 +45,7 @@ $LOCAL-VARIABLES: (
   flex-direction: column;
   padding: 9px 0;
   border-right: pfe-local(Border);
+  overflow: hidden;
 }
 
 ::slotted([slot="details-nav"]) {
@@ -69,10 +73,13 @@ $LOCAL-VARIABLES: (
   background: pfe-local(Background, $region: details);
 
   :host([consistent-height]) & {
-    display: grid;
-    grid-template: 1fr / 1fr;
-    justify-items: start;
-    align-items: start;
+    display: flex;
+    @supports (display: grid) {
+      display: grid;
+      grid-template: 1fr / 1fr;
+      justify-items: start;
+      align-items: start;
+    }
   }
 }
 

elements/pfe-primary-detail/src/pfe-primary-detail.scss

@@ -124,12 +124,6 @@ suite("<pfe-primary-detail>", () => {
         "Toggle's role is not set to tab"
       );
 
-      assert.strictEqual(
-        toggle.tagName,
-        'BUTTON',
-        "Toggle's tag is not button"
-      );
-
       // Get the details (controlled) element
       const controlledElement = document.getElementById(ariaControls);
       assert.isNotNull(
@@ -196,7 +190,7 @@ suite("<pfe-primary-detail>", () => {
     // Another test should have failed if this was an issue,
     // BUT want to make sure that the element with ID is processed and the ID was maintained
     assert.isTrue(
-      detailNavWithId.tagName === 'BUTTON',
+      detailNavWithId.hasAttribute('data-index'),
       "Detail nav appears to be in default state and wasn't processed?"
     );
 
@@ -213,7 +207,7 @@ suite("<pfe-primary-detail>", () => {
 
     flush(() => {
       // test dynamically prepended content
-      const firstNavItem = defaultWrapper.querySelector("button");
+      const firstNavItem = defaultWrapper.querySelector("[role=tab]");
       const firstDetail = defaultWrapper.querySelector("div");
       const firstDetailMenuItem = firstDetail.querySelector("li a");
       const prependedDetailFirstMenuItem = prependedDetail.querySelector("li a");
@@ -251,7 +245,7 @@ suite("<pfe-primary-detail>", () => {
       // test dynamically appened content
       activeToggle = defaultWrapper.querySelector('[aria-selected="true"]');
 
-      const lastNavItem = defaultWrapper.querySelector("button:last-of-type");
+      const lastNavItem = defaultWrapper.querySelector("h3:last-of-type");
       const lastDetail = defaultWrapper.querySelector("div:last-of-type");
       const lastDetailMenuItem = lastDetail.querySelector("li a");
       const appendedDetailFirstMenuItem = appendedDetail.querySelector("li a");
@@ -267,7 +261,7 @@ suite("<pfe-primary-detail>", () => {
         appendedDetailFirstMenuItem.textContent,
         "Dynamically appended detail menu does not equal the text of the last detail menu item"
       );
-      
+
       assert.strictEqual(
         lastNavItem.dataset.index,
         "5",