Fix SubNav regressions (#851)

primer · Dec 8, 2024 · 21b2584 · 21b2584
1 parent 1adc975
commit 21b2584
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 43 deletions.
diff --git a/packages/react/src/SubNav/SubNav.module.css b/packages/react/src/SubNav/SubNav.module.css
@@ -1,6 +1,7 @@
 .SubNav__container {
   position: absolute;
   width: 100%;
+  z-index: 2;
 }
 
 .SubNav__container--with-anchor-nav {
@@ -89,16 +90,50 @@
   z-index: 91; /* must be higher than subdomain nav bar */
 }
 
+.SubNav__anchor-menu-container {
+  opacity: 0;
+  background-color: transparent;
+  transform: translateY(var(--brand-animation-variant-slideInDown-distance));
+  transition: opacity var(--brand-animation-duration-default) var(--brand-animation-easing-default),
+    background-color var(--brand-animation-duration-default) var(--brand-animation-easing-default);
+}
+
+.SubNav__anchor-menu-outer-container--stuck .SubNav__anchor-menu-container {
+  transform: translateY(0);
+  opacity: 1;
+  background-color: var(--brand-color-canvas-default);
+}
+
+.SubNav__anchor-menu-outer-container {
+  height: 0;
+  transition: height var(--brand-animation-duration-default) var(--brand-animation-easing-default);
+}
+
+.SubNav__anchor-menu-outer-container--stuck {
+  height: auto;
+}
+
+.SubNav__sub-menu--anchor {
+  opacity: 0;
+  visibility: hidden;
+  transform: translateY(var(--brand-animation-variant-slideInDown-distance));
+  transition: opacity var(--brand-animation-duration-default) var(--brand-animation-easing-default),
+    height var(--brand-animation-duration-default) var(--brand-animation-easing-default),
+    transform var(--brand-animation-duration-default) var(--brand-animation-easing-default);
+}
+
+.SubNav__anchor-menu-outer-container--stuck .SubNav__sub-menu--anchor {
+  visibility: visible;
+  opacity: 1;
+  transform: translateY(0);
+}
+
 .SubNav__sub-menu--anchor {
   display: flex;
   padding-block-start: var(--base-size-12);
   padding-block-end: var(--base-size-20);
 }
 
-.SubNav__anchor-menu-outer-container--stuck {
-  background-color: var(--brand-color-canvas-default);
-}
-
 .SubNav__sub-menu--anchor .SubNav__sub-menu-list {
   display: inline-flex;
   list-style-type: none;
@@ -277,10 +312,10 @@
     position: relative;
     width: 100%;
     display: flex;
-
     order: 1;
     z-index: 9999;
     padding-inline: 0;
+    justify-content: end;
   }
 
   .SubNav__overlay-toggle-content {
@@ -340,7 +375,6 @@
   }
 
   .SubNav__anchor-menu-outer-container {
-    overflow: hidden;
     position: sticky;
     top: -1px;
   }
@@ -405,6 +439,10 @@
   top: calc(var(--base-size-4) / 4 * -1);
 }
 
+.SubNav__overlay-toggle-content--end {
+  justify-content: flex-end;
+}
+
 @media screen and (min-width: 48rem) {
   .SubNav {
     padding: var(--base-size-16) var(--base-size-24);

diff --git a/packages/react/src/SubNav/SubNav.module.css.d.ts b/packages/react/src/SubNav/SubNav.module.css.d.ts
@@ -14,8 +14,9 @@ declare const styles: {
   readonly "SubNav__sub-menu-toggle": string;
   readonly "SubNav__heading-separator": string;
   readonly "SubNav__anchor-menu-outer-container": string;
-  readonly "SubNav__sub-menu--anchor": string;
+  readonly "SubNav__anchor-menu-container": string;
   readonly "SubNav__anchor-menu-outer-container--stuck": string;
+  readonly "SubNav__sub-menu--anchor": string;
   readonly "SubNav__sub-menu-list": string;
   readonly "SubNav__link--is-in-view": string;
   readonly "SubNav__link-label": string;
@@ -30,8 +31,8 @@ declare const styles: {
   readonly "SubNav__action": string;
   readonly "SubNav__sub-menu": string;
   readonly "SubNav__sub-menu--dropdown": string;
-  readonly "SubNav__anchor-menu-container": string;
   readonly "SubNav__overlay-toggle-icon": string;
+  readonly "SubNav__overlay-toggle-content--end": string;
   readonly "SubNav__heading-label": string;
   readonly "SubNav__link--expanded": string;
   readonly "fade-in-down": string;

diff --git a/packages/react/src/SubNav/SubNav.stories.tsx b/packages/react/src/SubNav/SubNav.stories.tsx
@@ -16,13 +16,9 @@ const Template: StoryFn<typeof SubNav> = args => (
       <SubNav.Heading href="#">Heading</SubNav.Heading>
       <SubNav.Link href="#">Link </SubNav.Link>
       <SubNav.Link href="#">Link</SubNav.Link>
-      <SubNav.Link href="#" aria-current="page">
-        Link
-      </SubNav.Link>
       <SubNav.Link href="#">Link</SubNav.Link>
-      <SubNav.Link href="#" aria-current="false">
-        Link
-      </SubNav.Link>
+      <SubNav.Link href="#">Link</SubNav.Link>
+      <SubNav.Link href="#">Link</SubNav.Link>
       <SubNav.Link href="#">Link</SubNav.Link>
       <SubNav.Action href="#">Primary CTA</SubNav.Action>
     </SubNav>

diff --git a/packages/react/src/SubNav/SubNav.tsx b/packages/react/src/SubNav/SubNav.tsx
@@ -78,6 +78,7 @@ export const useSubNavContext = () => {
 }
 
 function SubNavProvider({children}: {children: React.ReactNode}) {
+  const anchoredNavOuterPortalRef = React.useRef<HTMLDivElement>(null)
   const anchoredNavPortalRef = React.useRef<HTMLDivElement>(null)
 
   const value = useMemo(
@@ -88,7 +89,7 @@ function SubNavProvider({children}: {children: React.ReactNode}) {
   )
 
   useEffect(() => {
-    const menuContainer = anchoredNavPortalRef.current
+    const menuContainer = anchoredNavOuterPortalRef.current
 
     const observer = new IntersectionObserver(
       ([entry]) => {
@@ -112,7 +113,7 @@ function SubNavProvider({children}: {children: React.ReactNode}) {
     <SubNavContext.Provider value={value}>
       {children}
 
-      <div className={styles['SubNav__anchor-menu-outer-container']}>
+      <div className={styles['SubNav__anchor-menu-outer-container']} ref={anchoredNavOuterPortalRef}>
         <div className={clsx(styles['SubNav__anchor-menu-container'])} ref={anchoredNavPortalRef} />
       </div>
     </SubNavContext.Provider>
@@ -217,12 +218,11 @@ const _SubNavRoot = memo(({id, children, className, 'data-testid': testId, fullW
     {heading: undefined, links: [], action: undefined},
   )
 
-  // The values are different types depending on whether a submenu is present
   const activeLinklabel =
     typeof activeLink?.props.children === 'string' ? activeLink.props.children : activeLink?.props.children[0]
+
   // needed to prevent rendering of anchor subnav inside the narrow <button> element
   const MaybeSubNav = activeLink?.props.children?.[1]?.props?.variant === 'anchor' && activeLink.props.children?.[1]
-
   return (
     <div className={clsx(styles['SubNav__container'], hasAnchoredNav && styles['SubNav__container--with-anchor-nav'])}>
       <SubNavProvider>
@@ -241,26 +241,29 @@ const _SubNavRoot = memo(({id, children, className, 'data-testid': testId, fullW
           <div ref={rootRef} className={styles['SubNav--header-container-outer']}>
             <div className={styles['SubNav__header-container']}>
               {HeadingChild && <div className={styles['SubNav__heading-container']}>{HeadingChild}</div>}
-              <span role="separator" className={styles['SubNav__heading-separator']} aria-hidden>
-                <svg
-                  xmlns="http://www.w3.org/2000/svg"
-                  width="8"
-                  height="16"
-                  viewBox="0 0 8 16"
-                  fill="none"
-                  aria-hidden
-                >
-                  <g clipPath="url(#clip0_50_1307)">
-                    <path d="M0 15.2992L5.472 0.701172H7.632L2.16 15.2992H0Z" fill="currentColor" />
-                  </g>
-                  <defs>
-                    <clipPath id="clip0_50_1307">
-                      <rect width="7.632" height="14.598" transform="translate(0 0.701172)" />
-                    </clipPath>
-                  </defs>
-                </svg>
-              </span>
-              {activeLink && activeLinklabel && !isLarge && (
+              {activeLinklabel && (
+                <span role="separator" className={styles['SubNav__heading-separator']} aria-hidden>
+                  <svg
+                    xmlns="http://www.w3.org/2000/svg"
+                    width="8"
+                    height="16"
+                    viewBox="0 0 8 16"
+                    fill="none"
+                    aria-hidden
+                  >
+                    <g clipPath="url(#clip0_50_1307)">
+                      <path d="M0 15.2992L5.472 0.701172H7.632L2.16 15.2992H0Z" fill="currentColor" />
+                    </g>
+                    <defs>
+                      <clipPath id="clip0_50_1307">
+                        <rect width="7.632" height="14.598" transform="translate(0 0.701172)" />
+                      </clipPath>
+                    </defs>
+                  </svg>
+                </span>
+              )}
+
+              {!isLarge && (
                 <button
                   className={clsx(
                     styles['SubNav__overlay-toggle'],
@@ -271,11 +274,18 @@ const _SubNavRoot = memo(({id, children, className, 'data-testid': testId, fullW
                   aria-expanded={isOpenAtNarrow ? 'true' : 'false'}
                   aria-controls={idForLinkContainer}
                 >
-                  <span className="visually-hidden">Navigation menu. Current page: </span>
-                  <span className={styles['SubNav__overlay-toggle-content']}>
-                    <Text as="span" size="200">
-                      {activeLinklabel}
-                    </Text>
+                  {activeLinklabel && <span className="visually-hidden">Navigation menu. Current page: </span>}
+                  <span
+                    className={clsx(
+                      styles['SubNav__overlay-toggle-content'],
+                      !activeLinklabel && styles['SubNav__overlay-toggle-content--end'],
+                    )}
+                  >
+                    {activeLinklabel && (
+                      <Text as="span" size="200">
+                        {activeLinklabel}
+                      </Text>
+                    )}
                     {isOpenAtNarrow ? (
                       <ChevronUpIcon className={styles['SubNav__overlay-toggle-icon']} size={24} />
                     ) : (

diff --git a/...ile-viewport-test-6184a--SubNav-Narrow-Anchor-Nav-Variant-Menu-Open-1-linux.png b/...ile-viewport-test-6184a--SubNav-Narrow-Anchor-Nav-Variant-Menu-Open-1-linux.png
diff --git a/...ile-viewport-test-d89c8-av-Variant-SubNav-Narrow-Anchor-Nav-Variant-1-linux.png b/...ile-viewport-test-d89c8-av-Variant-SubNav-Narrow-Anchor-Nav-Variant-1-linux.png
diff --git a/...pec.ts-snapshots/Visual-Comparison-SubNav-SubNav-Anchor-Nav-Variant-1-linux.png b/...pec.ts-snapshots/Visual-Comparison-SubNav-SubNav-Anchor-Nav-Variant-1-linux.png
diff --git a/...av.visual.spec.ts-snapshots/Visual-Comparison-SubNav-SubNav-Default-1-linux.png b/...av.visual.spec.ts-snapshots/Visual-Comparison-SubNav-SubNav-Default-1-linux.png