From 0da8645704d34645249ec9552279f02da97fbe40 Mon Sep 17 00:00:00 2001
From: Trevor Burnham <trevorburnham@gmail.com>
Date: Sat, 5 Apr 2025 18:06:23 -0400
Subject: [PATCH] fix: Implement TopNavigation breakpoints in CSS

This commit switches from JavaScript-based breakpoints to CSS container
queries for TopNavigation, making its appearance consistent when
rendered with SSR.

To support container queries, we need `container-type: inline-size;` on
the TopNavigation root element. This causes overflowing content to get
clipped (at least in Safari), so I've enabled `expandToViewport` to
render utility dropdowns in a portal. Note that OverflowMenu doesn't
need this treatment because it expands the height of TopNavigation
rather than overflowing.

Fixes #3337
---
 .../components/menu-dropdown/index.tsx        |  8 +-
 .../components/menu-dropdown/interfaces.ts    |  5 +-
 .../styles/foundation/breakpoints.scss        | 38 +++++++-
 src/test-utils/dom/top-navigation/index.ts    |  4 +-
 src/top-navigation/internal.tsx               | 96 ++++++-------------
 src/top-navigation/parts/utility.tsx          |  9 +-
 src/top-navigation/styles.scss                | 79 ++++++++-------
 src/top-navigation/use-top-navigation.ts      |  8 +-
 8 files changed, 130 insertions(+), 117 deletions(-)

diff --git a/src/internal/components/menu-dropdown/index.tsx b/src/internal/components/menu-dropdown/index.tsx
index cae25ab4d5..fb2a21bacf 100644
--- a/src/internal/components/menu-dropdown/index.tsx
+++ b/src/internal/components/menu-dropdown/index.tsx
@@ -31,6 +31,8 @@ export const ButtonTrigger = React.forwardRef(
       expanded,
       children,
       onClick,
+      // Used by TopNavigation to apply styles.
+      className,
     }: ButtonTriggerProps,
     ref: React.Ref<any>
   ) => {
@@ -40,7 +42,7 @@ export const ButtonTrigger = React.forwardRef(
       <button
         ref={ref}
         type="button"
-        className={clsx(styles.button, styles[`offset-right-${offsetRight}`], testUtilsClass, {
+        className={clsx(styles.button, styles[`offset-right-${offsetRight}`], testUtilsClass, className, {
           [styles.expanded]: expanded,
         })}
         aria-label={ariaLabel}
@@ -49,7 +51,7 @@ export const ButtonTrigger = React.forwardRef(
         disabled={disabled}
         onClick={event => {
           event.preventDefault();
-          onClick && onClick();
+          onClick?.();
         }}
       >
         {hasIcon && (
@@ -79,6 +81,7 @@ const MenuDropdown = ({
   badge,
   offsetRight,
   children,
+  className,
   ...props
 }: MenuDropdownProps) => {
   const baseProps = getBaseProps(props);
@@ -94,6 +97,7 @@ const MenuDropdown = ({
     return (
       <ButtonTrigger
         testUtilsClass={testUtilsClass}
+        className={className}
         ref={triggerRef}
         disabled={disabled}
         expanded={isOpen}
diff --git a/src/internal/components/menu-dropdown/interfaces.ts b/src/internal/components/menu-dropdown/interfaces.ts
index 03464bbe06..9e60df9382 100644
--- a/src/internal/components/menu-dropdown/interfaces.ts
+++ b/src/internal/components/menu-dropdown/interfaces.ts
@@ -5,6 +5,7 @@ import { IconProps } from '../../../icon/interfaces';
 
 export interface ButtonTriggerProps {
   testUtilsClass?: string;
+  className?: string;
   iconName?: IconProps.Name;
   iconUrl?: string;
   iconAlt?: string;
@@ -19,7 +20,8 @@ export interface ButtonTriggerProps {
   expanded?: boolean;
 }
 
-export interface MenuDropdownProps extends InternalButtonDropdownProps {
+export interface MenuDropdownProps extends Omit<InternalButtonDropdownProps, 'items'> {
+  items: InternalButtonDropdownProps['items'];
   iconName?: IconProps.Name;
   iconUrl?: string;
   iconAlt?: string;
@@ -27,4 +29,5 @@ export interface MenuDropdownProps extends InternalButtonDropdownProps {
   badge?: boolean;
   description?: string;
   offsetRight?: 'l' | 'xxl';
+  className?: string;
 }
diff --git a/src/internal/styles/foundation/breakpoints.scss b/src/internal/styles/foundation/breakpoints.scss
index 10b7137a8c..ff0ff4835d 100644
--- a/src/internal/styles/foundation/breakpoints.scss
+++ b/src/internal/styles/foundation/breakpoints.scss
@@ -3,7 +3,7 @@
  SPDX-License-Identifier: Apache-2.0
 */
 
-// Breakpoints
+// Media query breakpoints
 $breakpoint-xxx-small: 0;
 $breakpoint-xx-small: 576px;
 $breakpoint-x-small: 688px;
@@ -16,6 +16,18 @@ $breakpoint-xx-large: 2540px;
 $_smallest_breakpoint: $breakpoint-xxx-small;
 $_largest_breakpoint: $breakpoint-x-large;
 
+// Container breakpoints, matching the Grid component
+$container-breakpoint-default: -1;
+$container-breakpoint-xxs: 465px;
+$container-breakpoint-xs: 688px;
+$container-breakpoint-s: 912px;
+$container-breakpoint-m: 1120px;
+$container-breakpoint-l: 1320px;
+$container-breakpoint-xl: 1840px;
+
+$_container_smallest_breakpoint: $container-breakpoint-default;
+$_container_largest_breakpoint: $container-breakpoint-xl;
+
 // Media of at least the minimum breakpoint width. No query for the smallest breakpoint.
 // Makes the @content apply to the wider than given breakpoint.
 @mixin media-breakpoint-up($breakpoint) {
@@ -39,3 +51,27 @@ $_largest_breakpoint: $breakpoint-x-large;
     @content;
   }
 }
+
+// Container query for widths greater than the given breakpoint.
+// Matches the behavior of getMatchingBreakpoint in breakpoints.ts
+@mixin container-breakpoint-up($breakpoint) {
+  @if $breakpoint != $_container_smallest_breakpoint {
+    @container (min-width: #{$breakpoint + 1px}) {
+      @content;
+    }
+  } @else {
+    @content;
+  }
+}
+
+// Container query for widths less than or equal to the given breakpoint.
+// Matches the behavior of matchBreakpointMapping in breakpoints.ts
+@mixin container-breakpoint-down($breakpoint) {
+  @if $breakpoint != $_container_largest_breakpoint {
+    @container (max-width: #{$breakpoint}) {
+      @content;
+    }
+  } @else {
+    @content;
+  }
+}
diff --git a/src/test-utils/dom/top-navigation/index.ts b/src/test-utils/dom/top-navigation/index.ts
index 0b9e955047..f79ea0024f 100644
--- a/src/test-utils/dom/top-navigation/index.ts
+++ b/src/test-utils/dom/top-navigation/index.ts
@@ -101,10 +101,10 @@ export class TopNavigationMenuDropdownWrapper extends ButtonDropdownWrapper {
   }
 
   findTitle(): ElementWrapper | null {
-    return this.findByClassName(buttonDropdownStyles.title);
+    return createWrapper().findComponent(`.${buttonDropdownStyles.title}`, ElementWrapper);
   }
 
   findDescription(): ElementWrapper | null {
-    return this.findByClassName(buttonDropdownStyles.description);
+    return createWrapper().findComponent(`.${buttonDropdownStyles.description}`, ElementWrapper);
   }
 }
diff --git a/src/top-navigation/internal.tsx b/src/top-navigation/internal.tsx
index 9bd46108af..17070d6c85 100644
--- a/src/top-navigation/internal.tsx
+++ b/src/top-navigation/internal.tsx
@@ -34,14 +34,13 @@ export default function InternalTopNavigation({
 }: InternalTopNavigationProps) {
   checkSafeUrl('TopNavigation', identity.href);
   const baseProps = getBaseProps(restProps);
-  const { mainRef, virtualRef, breakpoint, responsiveState, isSearchExpanded, onSearchUtilityClick } = useTopNavigation(
-    { identity, search, utilities }
-  );
+  const { mainRef, virtualRef, responsiveState, isSearchExpanded, onSearchUtilityClick } = useTopNavigation({
+    identity,
+    search,
+    utilities,
+  });
   const [overflowMenuOpen, setOverflowMenuOpen] = useState(false);
   const overflowMenuTriggerRef = useRef<HTMLButtonElement>(null);
-  const isNarrowViewport = breakpoint === 'default';
-  const isMediumViewport = breakpoint === 'xxs';
-  const isLargeViewport = breakpoint === 's';
   const i18n = useInternalI18n('top-navigation');
 
   // ButtonDropdown supports checkbox items but we don't support these in TopNavigation. Shown an error in development mode
@@ -98,8 +97,6 @@ export default function InternalTopNavigation({
         className={clsx(styles['top-navigation'], {
           [styles.virtual]: isVirtual,
           [styles.hidden]: isVirtual,
-          [styles.narrow]: isNarrowViewport,
-          [styles.medium]: isMediumViewport,
         })}
       >
         <div className={styles['padding-box']}>
@@ -107,14 +104,7 @@ export default function InternalTopNavigation({
             <div className={clsx(styles.identity, !identity.logo && styles['no-logo'])}>
               <a className={styles['identity-link']} href={identity.href} onClick={onIdentityClick}>
                 {identity.logo && (
-                  <img
-                    role="img"
-                    src={identity.logo?.src}
-                    alt={identity.logo?.alt}
-                    className={clsx(styles.logo, {
-                      [styles.narrow]: isNarrowViewport,
-                    })}
-                  />
+                  <img role="img" src={identity.logo?.src} alt={identity.logo?.alt} className={styles.logo} />
                 )}
                 {showTitle && <span className={styles.title}>{identity.title}</span>}
               </a>
@@ -135,11 +125,7 @@ export default function InternalTopNavigation({
                 className={clsx(
                   styles['utility-wrapper'],
                   styles['utility-type-button'],
-                  styles['utility-type-button-link'],
-                  {
-                    [styles.narrow]: isNarrowViewport,
-                    [styles.medium]: isMediumViewport,
-                  }
+                  styles['utility-type-button-link']
                 )}
                 data-utility-special="search"
               >
@@ -163,69 +149,45 @@ export default function InternalTopNavigation({
                   (_utility, i) =>
                     isVirtual || !responsiveState.hideUtilities || responsiveState.hideUtilities.indexOf(i) === -1
                 )
-                .map((utility, i) => {
-                  const hideText = !!responsiveState.hideUtilityText;
-                  const isLast = (isVirtual || !showMenuTrigger) && i === utilities.length - 1;
-                  const offsetRight = isLast && isLargeViewport ? 'xxl' : isLast ? 'l' : undefined;
-
-                  return (
-                    <div
-                      key={i}
-                      className={clsx(
-                        styles['utility-wrapper'],
-                        styles[`utility-type-${utility.type}`],
-                        utility.type === 'button' && styles[`utility-type-button-${utility.variant ?? 'link'}`],
-                        {
-                          [styles.narrow]: isNarrowViewport,
-                          [styles.medium]: isMediumViewport,
-                        }
-                      )}
-                      data-utility-index={i}
-                      data-utility-hide={`${hideText}`}
-                    >
-                      <Utility hideText={hideText} definition={utility} offsetRight={offsetRight} />
-                    </div>
-                  );
-                })}
-
-            {isVirtual &&
-              utilities.map((utility, i) => {
-                const hideText = !responsiveState.hideUtilityText;
-                const isLast = !showMenuTrigger && i === utilities.length - 1;
-                const offsetRight = isLast && isLargeViewport ? 'xxl' : isLast ? 'l' : undefined;
-
-                return (
+                .map((utility, i) => (
                   <div
                     key={i}
                     className={clsx(
                       styles['utility-wrapper'],
                       styles[`utility-type-${utility.type}`],
-                      utility.type === 'button' && styles[`utility-type-button-${utility.variant ?? 'link'}`],
-                      {
-                        [styles.narrow]: isNarrowViewport,
-                        [styles.medium]: isMediumViewport,
-                      }
+                      utility.type === 'button' && styles[`utility-type-button-${utility.variant ?? 'link'}`]
                     )}
                     data-utility-index={i}
-                    data-utility-hide={`${hideText}`}
+                    data-utility-hide={`${!!responsiveState.hideUtilityText}`}
                   >
-                    <Utility hideText={hideText} definition={utility} offsetRight={offsetRight} />
+                    <Utility hideText={!!responsiveState.hideUtilityText} definition={utility} />
                   </div>
-                );
-              })}
+                ))}
+
+            {isVirtual &&
+              utilities.map((utility, i) => (
+                <div
+                  key={i}
+                  className={clsx(
+                    styles['utility-wrapper'],
+                    styles[`utility-type-${utility.type}`],
+                    utility.type === 'button' && styles[`utility-type-button-${utility.variant ?? 'link'}`]
+                  )}
+                  data-utility-index={i}
+                  data-utility-hide={`${!responsiveState.hideUtilityText}`}
+                >
+                  <Utility hideText={!responsiveState.hideUtilityText} definition={utility} />
+                </div>
+              ))}
 
             {showMenuTrigger && (
               <div
-                className={clsx(styles['utility-wrapper'], styles['utility-type-menu-dropdown'], {
-                  [styles.narrow]: isNarrowViewport,
-                  [styles.medium]: isMediumViewport,
-                })}
+                className={clsx(styles['utility-wrapper'], styles['utility-type-menu-dropdown'])}
                 data-utility-special="menu-trigger"
               >
                 <ButtonTrigger
                   expanded={overflowMenuOpen}
                   onClick={toggleOverflowMenu}
-                  offsetRight="l"
                   ref={!isVirtual ? overflowMenuTriggerRef : undefined}
                 >
                   {i18n('i18nStrings.overflowMenuTriggerText', i18nStrings?.overflowMenuTriggerText)}
diff --git a/src/top-navigation/parts/utility.tsx b/src/top-navigation/parts/utility.tsx
index 1c879d834d..99fdccaf71 100644
--- a/src/top-navigation/parts/utility.tsx
+++ b/src/top-navigation/parts/utility.tsx
@@ -1,7 +1,6 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 import React from 'react';
-import clsx from 'clsx';
 
 import { InternalButton } from '../../button/internal';
 import { isLinkItem } from '../../button-dropdown/utils/utils';
@@ -33,7 +32,7 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
     checkSafeUrl('TopNavigation', definition.href);
     if (definition.variant === 'primary-button') {
       return (
-        <span className={styles[`offset-right-${offsetRight}`]}>
+        <span>
           <InternalButton
             variant="primary"
             href={definition.href}
@@ -53,7 +52,7 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
                   <>
                     {' '}
                     <span
-                      className={clsx(styles['utility-button-external-icon'], styles[`offset-right-${offsetRight}`])}
+                      className={styles['utility-button-external-icon']}
                       aria-label={definition.externalIconAriaLabel}
                       role={definition.externalIconAriaLabel ? 'img' : undefined}
                     >
@@ -69,7 +68,7 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
     } else {
       // Link
       return (
-        <span className={styles[`offset-right-${offsetRight}`]}>
+        <span>
           <InternalLink
             variant="top-navigation"
             href={definition.href}
@@ -120,7 +119,9 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
         items={items}
         title={shouldShowTitle ? title : ''}
         ariaLabel={ariaLabel}
+        expandToViewport={true}
         offsetRight={offsetRight}
+        className={styles['utility-menu-dropdown-button']}
       >
         {!shouldHideText && definition.text}
       </MenuDropdown>
diff --git a/src/top-navigation/styles.scss b/src/top-navigation/styles.scss
index 99740f00f5..d7d46c2be2 100644
--- a/src/top-navigation/styles.scss
+++ b/src/top-navigation/styles.scss
@@ -6,10 +6,16 @@
 @use '../internal/styles' as styles;
 @use '../internal/styles/tokens' as awsui;
 @use '@cloudscape-design/component-toolkit/internal/focus-visible' as focus-visible;
+@use '../internal/styles/foundation/breakpoints' as breakpoints;
 
 .top-navigation {
   @include styles.styles-reset;
   background: awsui.$color-background-container-content;
+  container-type: inline-size;
+
+  // Workaround for bug with container overflow in Safari < 18.4
+  position: relative;
+  z-index: 1;
 
   > .padding-box {
     display: flex;
@@ -22,18 +28,49 @@
     padding-inline-start: awsui.$space-xxl;
   }
 
-  &.medium > .padding-box,
-  &.narrow > .padding-box {
-    padding-inline-start: awsui.$space-l;
+  @include breakpoints.container-breakpoint-up(breakpoints.$container-breakpoint-s) {
+    > .padding-box {
+      > .utilities > .utility-wrapper:last-of-type {
+        padding-inline-end: 0;
+
+        // stylelint-disable-next-line @cloudscape-design/no-implicit-descendant
+        .utility-menu-dropdown-button {
+          margin-inline-end: awsui.$space-xl;
+        }
+      }
+    }
   }
 
-  &.medium > .padding-box {
-    block-size: calc(#{awsui.$space-xxxl} + #{awsui.$space-scaled-xs});
-    padding-inline-end: 0;
+  @include breakpoints.container-breakpoint-down(breakpoints.$container-breakpoint-s) {
+    > .padding-box {
+      block-size: calc(#{awsui.$space-xxxl} + #{awsui.$space-scaled-xs});
+      padding-inline-start: awsui.$space-l;
+
+      > .utilities > .utility-wrapper:last-of-type {
+        padding-inline-end: 0;
+
+        // stylelint-disable-next-line @cloudscape-design/no-implicit-descendant
+        .utility-menu-dropdown-button {
+          margin-inline-end: awsui.$space-s;
+        }
+      }
+    }
   }
 
-  &.narrow > .padding-box {
-    block-size: awsui.$space-xxxl;
+  @include breakpoints.container-breakpoint-down(breakpoints.$container-breakpoint-xxs) {
+    > .padding-box {
+      block-size: awsui.$space-xxxl;
+      padding-inline-start: awsui.$space-l;
+
+      > .utilities {
+        padding-inline-start: 0;
+      }
+
+      // stylelint-disable-next-line @cloudscape-design/no-implicit-descendant
+      .logo {
+        max-block-size: awsui.$space-xl;
+      }
+    }
   }
 }
 
@@ -87,10 +124,6 @@
 
   // Setting an arbitrary min-width here discourages browser from lazy rendering
   min-inline-size: 10px;
-
-  &.narrow {
-    max-block-size: awsui.$space-xl;
-  }
 }
 
 .title {
@@ -129,11 +162,6 @@
 
   // Expand height of utilies fully so that the dropdown is anchored directly underneath it.
   block-size: 100%;
-
-  .medium > .padding-box > &,
-  .narrow > .padding-box > & {
-    padding-inline-start: 0;
-  }
 }
 
 .utility-wrapper {
@@ -170,11 +198,6 @@
   padding-block: 0;
   padding-inline: awsui.$space-s;
   align-items: stretch;
-
-  &:not(.narrow):last-of-type,
-  &:not(.medium):last-of-type {
-    padding-inline-end: 0;
-  }
 }
 
 .utility-type-button-primary-button {
@@ -194,18 +217,6 @@
   display: inline-block;
 }
 
-.offset-right-none {
-  margin-inline-end: 0;
-}
-
-.offset-right-l {
-  margin-inline-end: awsui.$space-xxs;
-}
-
-.offset-right-xxl {
-  margin-inline-end: awsui.$space-m;
-}
-
 /* Overflow Menu */
 .overflow-menu-drawer {
   position: fixed;
diff --git a/src/top-navigation/use-top-navigation.ts b/src/top-navigation/use-top-navigation.ts
index ed9f63987e..c5430bf16b 100644
--- a/src/top-navigation/use-top-navigation.ts
+++ b/src/top-navigation/use-top-navigation.ts
@@ -4,7 +4,6 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
 
 import { useContainerQuery } from '@cloudscape-design/component-toolkit';
 
-import { useContainerBreakpoints } from '../internal/hooks/container-queries';
 import { useMergeRefs } from '../internal/hooks/use-merge-refs';
 import { TopNavigationProps } from './interfaces';
 
@@ -41,7 +40,6 @@ interface UseTopNavigation {
   virtualRef: React.Ref<HTMLDivElement>;
 
   responsiveState: ResponsiveState;
-  breakpoint: 'default' | 'xxs' | 's';
   isSearchExpanded: boolean;
   onSearchUtilityClick: () => void;
 }
@@ -50,10 +48,9 @@ interface UseTopNavigation {
 const RESPONSIVENESS_BUFFER = 20;
 
 export function useTopNavigation({ identity, search, utilities }: UseTopNavigationParams): UseTopNavigation {
-  // Refs and breakpoints
+  // Refs
   const mainRef = useRef<HTMLElement | null>(null);
   const virtualRef = useRef<HTMLDivElement | null>(null);
-  const [breakpoint, breakpointRef] = useContainerBreakpoints(['xxs', 's']);
 
   // Responsiveness state
   // The component works by calculating the possible resize states that it can
@@ -149,13 +146,12 @@ export function useTopNavigation({ identity, search, utilities }: UseTopNavigati
     }
   }, [isSearchExpanded, mainRef]);
 
-  const mergedMainRef = useMergeRefs(mainRef, containerQueryRef, breakpointRef);
+  const mergedMainRef = useMergeRefs(mainRef, containerQueryRef);
 
   return {
     mainRef: mergedMainRef,
     virtualRef: onVirtualMount,
     responsiveState: responsiveState ?? responsiveStates[0],
-    breakpoint: breakpoint ?? 'default',
     isSearchExpanded: !!isSearchExpanded,
     onSearchUtilityClick: () => setSearchMinimized(isSearchMinimized => !isSearchMinimized),
   };