From 7efe83e460547c60e3146ee2de3fa18d943657f7 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Thu, 9 Nov 2023 13:51:56 -0500 Subject: [PATCH] fix(navigation): fix tab order and layout shift (#1793) * fix(navigation): toggle property that impacts the accessibility tree * fix(navigation): maintain scrollbar space when toggling the menu, don't collapse the space occupied by the scrollbar --- apps/site/assets/css/_global-navigation.scss | 9 ++++---- apps/site/assets/ts/app/global-navigation.ts | 22 +++++++++++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/apps/site/assets/css/_global-navigation.scss b/apps/site/assets/css/_global-navigation.scss index 277dadea7d..b096d54e4f 100644 --- a/apps/site/assets/css/_global-navigation.scss +++ b/apps/site/assets/css/_global-navigation.scss @@ -24,8 +24,9 @@ header { } [data-nav-open] { - body { - overflow: hidden; // prevent scrolling + .body-wrapper { + overflow: inherit; // same value as on body + position: relative; // so children elements using `position: absolute` are in the right place } .m-menu--cover { @@ -133,7 +134,7 @@ nav.m-menu--desktop { .m-menu--desktop__menu { background-color: $brand-primary-lightest-contrast; box-shadow: 0 7px 14px 0 $gray-shadow; - height: 0; // start off closed + display: none; // start off closed left: 0; overflow: hidden; position: absolute; @@ -143,7 +144,7 @@ nav.m-menu--desktop { // opened .m-menu--desktop__toggle[aria-expanded='true'] + & { - height: initial; + display: initial; } } diff --git a/apps/site/assets/ts/app/global-navigation.ts b/apps/site/assets/ts/app/global-navigation.ts index f2f78839f0..87f35fe85c 100644 --- a/apps/site/assets/ts/app/global-navigation.ts +++ b/apps/site/assets/ts/app/global-navigation.ts @@ -154,7 +154,6 @@ export function setup(rootElement: HTMLElement): void { if (aMenuIsBeingExpanded) { // eslint-disable-next-line no-param-reassign rootElement.dataset.navOpen = "true"; - disableBodyScroll(header); if (observedDataAttributes.includes(TOGGLE_NAMES.mobile)) { // eslint-disable-next-line no-param-reassign header.dataset.navOpen = "true"; @@ -164,6 +163,7 @@ export function setup(rootElement: HTMLElement): void { } else if (observedDataAttributes.includes(TOGGLE_NAMES.search)) { // eslint-disable-next-line no-param-reassign header.dataset.searchOpen = "true"; + disableBodyScroll(header); } } else { // only do this if no other menu is expanded @@ -190,6 +190,26 @@ export function setup(rootElement: HTMLElement): void { aMenuIsBeingExpanded && observedDataAttributes.includes(TOGGLE_NAMES.desktop) ) { + // Disable scrolling the page, but accomodate any visible scrollbars in + // order to avoid horizontal shift in the layout when scrolling becomes + // disabled. This additionally requires adjusting the width of the veil, + // to maintain a pleasing appearance. + disableBodyScroll(header, { reserveScrollBarGap: true }); + const cover = rootElement.querySelector("[data-nav='veil']"); + if ( + cover && + !cover.style.paddingRight && + rootElement.dataset.navOpen === "true" + ) { + const body = rootElement.querySelector("body"); + // this was added by { reserveScrollBarGap: true } + const paddingRight = body?.style.paddingRight; + if (paddingRight && paddingRight !== "") { + // add same 'padding' for veil by substracting from width + cover.style.width = `calc(100% - ${paddingRight})`; + } + } + const thisMenu = mutations.map(({ target }) => (target as Element).getAttribute("aria-controls") )[0];