From 1e0ce3f14e132d3104956b5722510e74cd95e7b0 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Tue, 7 Nov 2023 20:46:45 -0500 Subject: [PATCH 1/2] fix(navigation): toggle property that impacts the accessibility tree --- apps/site/assets/css/_global-navigation.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/site/assets/css/_global-navigation.scss b/apps/site/assets/css/_global-navigation.scss index 277dadea7d..a572c67896 100644 --- a/apps/site/assets/css/_global-navigation.scss +++ b/apps/site/assets/css/_global-navigation.scss @@ -133,7 +133,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 +143,7 @@ nav.m-menu--desktop { // opened .m-menu--desktop__toggle[aria-expanded='true'] + & { - height: initial; + display: initial; } } From 308f68c80f0fd2429c09d66af8ffc9ca39510e10 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Wed, 8 Nov 2023 18:39:55 -0500 Subject: [PATCH 2/2] 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 | 5 +++-- apps/site/assets/ts/app/global-navigation.ts | 22 +++++++++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/apps/site/assets/css/_global-navigation.scss b/apps/site/assets/css/_global-navigation.scss index a572c67896..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 { 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];