Sky Nav: Remove layout shift caused by progressive enhancement (#1947)

Author: gerardo-rodriguez

.changeset/stale-donuts-retire.md

@@ -0,0 +1,5 @@
+---
+'@cloudfour/patterns': minor
+---
+
+Remove small viewport Sky Nav layout shift caused by progressive enhancement

src/components/sky-nav/sky-nav.scss

@@ -265,6 +265,13 @@ $_masthead-height-sm: ms.step(7);
   @media (min-width: $_breakpoint-wide) {
     display: none;
   }
+
+  /**
+   * There is no need to show the menu toggle if JS is not available.
+   */
+  .no-js & {
+    display: none;
+  }
 }
 
 /**
@@ -345,6 +352,22 @@ $_masthead-height-sm: ms.step(7);
   list-style: none; /* 1 */
   padding-inline-start: 0; /* 1 */
 
+  /**
+   * The Sky Nav is progressively enhanced with JS at smaller viewports. By default
+   * it is open for when JS is not available. In cases where JS _is_ available,
+   * this causes a poor loading UX where the menu jumps from open to closed
+   * causing a large layout shift.
+   *
+   * To keep the progressive enhancement in place _and_ not have the layout shift,
+   * we hide the menu items during the "loading" state. The "loading" state
+   * happens after the UI renders and before the Sky Nav JS is loaded.
+   */
+  @media (width < $_breakpoint-wide) {
+    .is-loading & {
+      display: none;
+    }
+  }
+
   /**
    * When the layout is wide enough to expose these items…
    *

src/components/sky-nav/sky-nav.test.ts

@@ -21,7 +21,31 @@ const initSkyNavJS = (utils: PleasantestUtils, navButton: ElementHandle) =>
 test(
   'can be opened on small screens',
   withBrowser({ device: iPhone }, async ({ utils, screen, user, page }) => {
-    await utils.injectHTML(await skyNavMarkup({ includeMainDemo: true, menu }));
+    // The rendered markup needs to be captured so we can pass it into the
+    // `page.evaluate` function below
+    const renderedMarkup = await skyNavMarkup({
+      includeMainDemo: true,
+      menu,
+    });
+    // Pleasantest uses `document.innerHTML` to inject the markup into the DOM,
+    // but that means inline scripts are not executed.
+    // @see https://github.com/cloudfour/pleasantest/issues/526
+    //
+    // > HTML5 specifies that a <script> tag inserted with innerHTML should not execute.
+    // @see https://developer.mozilla.org/en-US/docs/Web/API/Element/innerHTML#security_considerations
+    //
+    // This causes this test to fail because the Sky Nav template has an inline
+    // script. To get around that, the code below uses `document.write` instead
+    // which _will_ execute the inline script.
+    //
+    // Using `document.write` is discouraged. For the purposes of this test,
+    // though, it is okay.
+    // @see https://developer.mozilla.org/en-US/docs/Web/API/Document/write
+    await page.evaluate((renderedMarkup) => {
+      document.body.innerHTML = '';
+      document.write(renderedMarkup);
+    }, renderedMarkup);
+
     await loadGlobalCSS(utils);
     const navButton = await screen.getByRole('button', {
       name: /toggle main menu/i,

src/components/sky-nav/sky-nav.ts

@@ -21,6 +21,11 @@ export const initSkyNav = (navButton: HTMLButtonElement) => {
     '(prefers-reduced-motion: reduce)'
   );
 
+  // The Sky Nav component has inline synchronous JS logic to add an `is-loading`
+  // state to remove the layout shift at smaller viewports. That state no longer
+  // applies at this point since the Sky Nav JS has loaded & is ready to take over.
+  navWrapper.classList.remove('is-loading');
+
   /**
    * Update Menu Layout
    * Sets visibility of menu & navButton for small vs large screen layouts.

src/components/sky-nav/sky-nav.twig

@@ -1,7 +1,12 @@
 {% set main_id = main_id|default('main') %}
 {% set _main_href = '#' ~ main_id %}
 
-<div class="c-sky-nav js-sky-nav{% if class %} {{ class }}{% endif %}">
+{#
+  The Sky Nav component uses progressive enhancement and assumes JS is not
+  available as the default. The `no-js` CSS class is the hook to style properly
+  in that use case.
+#}
+<div class="c-sky-nav js-sky-nav no-js{% if class %} {{ class }}{% endif %}">
   {# https://webaim.org/techniques/skipnav/ #}
   {% embed '@cloudfour/components/button/button.twig' with {
     class: 'c-sky-nav__skip',
@@ -72,6 +77,26 @@
     </nav>
   </header>
 </div>
+<script>
+  {#
+    The Sky Nav is progressively enhanced with JS at smaller viewports.
+    By default it is open for when JS is not available. In cases where JS
+    _is_ available, this causes a poor loading UX where the menu jumps from
+    open to closed causing a large layout shift.
+
+    To keep the progressive enhancement in place _and_ not have the layout
+    shift, the Sky Nav component assumes `no-js` to begin with. If JS is enabled:
+
+    1. Remove the `no-js` state (which removes no-JS menu styles)
+    2. Add the `is-loading` state. This happens before the Sky Nav JS has loaded
+       allowing us a hook to hide the menu items to remove the layout shift. Once
+       the Sky Nav JS loads, it removes the `is-loading` state CSS class and the
+       menu functions as was originally designed.
+  #}
+  const skyNav = document.querySelector('.js-sky-nav');
+  skyNav.classList.remove('no-js'); // 1
+  skyNav.classList.add('is-loading'); // 2
+</script>
 
 {#
   For some reason Twig.js really doesn't like including this template in a demo,