From 25abc751879b2f7adddc47cbfb3930d256d93cef Mon Sep 17 00:00:00 2001
From: Joan Perals Tresserra <jotresse@amazon.com>
Date: Tue, 12 Nov 2024 10:21:07 +0100
Subject: [PATCH] fix: Prevent scrollbar from overlapping side navigation
 content

---
 .../navigation-with-scrollbar.page.tsx        | 55 ++++++++++++++++
 src/__integ__/use-browser-with-scrollbars.ts  | 66 +++++++++++++++++++
 src/app-layout/__integ__/navigation.test.ts   | 34 ++++++++++
 .../navigation/index.tsx                      | 34 +++++-----
 .../navigation/styles.scss                    | 24 +++----
 5 files changed, 183 insertions(+), 30 deletions(-)
 create mode 100644 pages/app-layout/navigation-with-scrollbar.page.tsx
 create mode 100644 src/__integ__/use-browser-with-scrollbars.ts
 create mode 100644 src/app-layout/__integ__/navigation.test.ts

diff --git a/pages/app-layout/navigation-with-scrollbar.page.tsx b/pages/app-layout/navigation-with-scrollbar.page.tsx
new file mode 100644
index 0000000000..19d74ee74f
--- /dev/null
+++ b/pages/app-layout/navigation-with-scrollbar.page.tsx
@@ -0,0 +1,55 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+import React from 'react';
+
+import AppLayout from '~components/app-layout';
+import Select from '~components/select';
+import SideNavigation, { SideNavigationProps } from '~components/side-navigation';
+
+import labels from './utils/labels';
+
+const items: SideNavigationProps.Item[] = new Array(50).fill(null).map((_, index) => ({
+  type: 'link',
+  text: `Link to page ${index + 1} with long enough text to wrap`,
+  href: '#',
+}));
+
+const itemsControl = (
+  <Select
+    options={[
+      { value: 'option1', label: 'Option 1' },
+      { value: 'option2', label: 'Option 2' },
+    ]}
+    selectedOption={{ value: 'option1', label: 'Option 1' }}
+    onChange={() => null}
+  />
+);
+
+export default function SideNavigationPage() {
+  const [open, setOpen] = React.useState(true);
+
+  return (
+    <AppLayout
+      navigationOpen={open}
+      onNavigationChange={({ detail }) => {
+        setOpen(detail.open);
+      }}
+      ariaLabels={labels}
+      navigation={
+        <SideNavigation
+          header={{
+            href: '#/',
+            text: 'Header title',
+          }}
+          items={items}
+          itemsControl={itemsControl}
+        />
+      }
+      content={
+        <>
+          <h1>App Layout with scrollable Side navigation</h1>
+        </>
+      }
+    />
+  );
+}
diff --git a/src/__integ__/use-browser-with-scrollbars.ts b/src/__integ__/use-browser-with-scrollbars.ts
new file mode 100644
index 0000000000..aa88056c02
--- /dev/null
+++ b/src/__integ__/use-browser-with-scrollbars.ts
@@ -0,0 +1,66 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+import useBrowser from '@cloudscape-design/browser-test-tools/use-browser';
+
+// Workaround until scrollbars are generally shown in tests (AWSUI-59983)
+
+interface TestFunction {
+  (browser: WebdriverIO.Browser): Promise<void> | void;
+}
+
+const options = {
+  capabilities: {
+    'goog:chromeOptions': {
+      args: [
+        // Same array as in
+        // https://github.com/cloudscape-design/browser-test-tools/blob/4aaed9e410b13e05a7d5dbace17231776d250b97/src/browsers/capabilities.ts
+        // but without --hide-scrollbar.
+        '--disable-background-timer-throttling',
+        '--disable-breakpad',
+        '--disable-client-side-phishing-detection',
+        '--disable-cloud-import',
+        '--disable-default-apps',
+        '--disable-dev-shm-usage',
+        '--disable-extensions',
+        '--disable-gesture-typing',
+        '--disable-hang-monitor',
+        '--disable-infobars',
+        '--disable-notifications',
+        '--disable-offer-store-unmasked-wallet-cards',
+        '--disable-offer-upload-credit-cards',
+        '--disable-popup-blocking',
+        '--disable-print-preview',
+        '--disable-prompt-on-repost',
+        '--disable-setuid-sandbox',
+        '--disable-speech-api',
+        '--disable-sync',
+        '--disable-tab-for-desktop-share',
+        '--disable-translate',
+        '--disable-voice-input',
+        '--disable-wake-on-wifi',
+        '--disk-cache-size=33554432',
+        '--enable-async-dns',
+        '--enable-simple-cache-backend',
+        '--enable-tcp-fast-open',
+        '--enable-webgl',
+        '--ignore-gpu-blacklist',
+        '--media-cache-size=33554432',
+        '--metrics-recording-only',
+        '--mute-audio',
+        '--no-default-browser-check',
+        '--no-first-run',
+        '--no-pings',
+        '--no-zygote',
+        '--password-store=basic',
+        '--prerender-from-omnibox=disabled',
+        '--no-sandbox',
+        '--disable-gpu',
+        '--headless=new',
+      ],
+    },
+  },
+};
+
+export default function (testFn: TestFunction): () => Promise<void> {
+  return useBrowser(options, testFn);
+}
diff --git a/src/app-layout/__integ__/navigation.test.ts b/src/app-layout/__integ__/navigation.test.ts
new file mode 100644
index 0000000000..9560b12cac
--- /dev/null
+++ b/src/app-layout/__integ__/navigation.test.ts
@@ -0,0 +1,34 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import { BasePageObject } from '@cloudscape-design/browser-test-tools/page-objects';
+
+import createWrapper from '../../../lib/components/test-utils/selectors';
+import useBrowserWithScrollbars from '../../__integ__/use-browser-with-scrollbars';
+import { getUrlParams, Theme } from './utils';
+
+const wrapper = createWrapper().findAppLayout().findNavigation();
+
+function setupTest(testFn: (page: BasePageObject) => Promise<void>, theme: Theme) {
+  return useBrowserWithScrollbars(async browser => {
+    const page = new BasePageObject(browser);
+    const params = getUrlParams(theme);
+    await browser.url(`#/light/app-layout/navigation-with-scrollbar?${params}`);
+    await page.waitForVisible(createWrapper().findSideNavigation().toSelector());
+    await testFn(page);
+  })();
+}
+
+describe('Navigation slot', () => {
+  describe('has expected inline size when a scrollbar is present', () => {
+    test.each(['classic', 'refresh-toolbar'] as const)('%s', theme =>
+      setupTest(async page => {
+        const { width } = await page.getBoundingBox(wrapper.toSelector());
+        expect(width).toBe(280);
+        const navigation = wrapper.findSideNavigation();
+        const navigationWidth = (await page.getBoundingBox(navigation.toSelector())).width;
+        expect(navigationWidth).toBeLessThan(280);
+      }, theme)
+    );
+  });
+});
diff --git a/src/app-layout/visual-refresh-toolbar/navigation/index.tsx b/src/app-layout/visual-refresh-toolbar/navigation/index.tsx
index d4afe826da..22fef387a1 100644
--- a/src/app-layout/visual-refresh-toolbar/navigation/index.tsx
+++ b/src/app-layout/visual-refresh-toolbar/navigation/index.tsx
@@ -44,25 +44,27 @@ export function AppLayoutNavigationImplementation({ appLayoutInternals }: AppLay
   };
 
   return (
-    <nav
-      aria-label={ariaLabels?.navigation ?? undefined}
-      className={clsx(
-        styles.navigation,
-        {
-          [styles['is-navigation-open']]: navigationOpen,
-          [testutilStyles['drawer-closed']]: !navigationOpen,
-        },
-        testutilStyles.navigation,
-        sharedStyles['with-motion-horizontal']
-      )}
-      aria-hidden={!navigationOpen}
-      onClick={onNavigationClick}
+    <div
+      className={clsx(styles['navigation-container'], sharedStyles['with-motion-horizontal'], {
+        [styles['is-navigation-open']]: navigationOpen,
+      })}
       style={{
         blockSize: drawerHeight,
         insetBlockStart: drawerTopOffset,
       }}
     >
-      <div className={clsx(styles['content-container'], styles['animated-content'])}>
+      <nav
+        aria-label={ariaLabels?.navigation ?? undefined}
+        className={clsx(
+          styles.navigation,
+          {
+            [testutilStyles['drawer-closed']]: !navigationOpen,
+          },
+          testutilStyles.navigation
+        )}
+        aria-hidden={!navigationOpen}
+        onClick={onNavigationClick}
+      >
         <div className={clsx(styles['hide-navigation'])}>
           <InternalButton
             ariaLabel={ariaLabels?.navigationClose ?? undefined}
@@ -75,8 +77,8 @@ export function AppLayoutNavigationImplementation({ appLayoutInternals }: AppLay
           />
         </div>
         {navigation}
-      </div>
-    </nav>
+      </nav>
+    </div>
   );
 }
 
diff --git a/src/app-layout/visual-refresh-toolbar/navigation/styles.scss b/src/app-layout/visual-refresh-toolbar/navigation/styles.scss
index 7804f0dd22..9a0d4039cf 100644
--- a/src/app-layout/visual-refresh-toolbar/navigation/styles.scss
+++ b/src/app-layout/visual-refresh-toolbar/navigation/styles.scss
@@ -8,16 +8,13 @@
 @use '../../../internal/generated/custom-css-properties/index.scss' as custom-props;
 @use '../../constants.scss' as constants;
 
-.navigation {
+// This wrapper clips the actual navigation content during the enter animation.
+.navigation-container {
   position: sticky;
   z-index: constants.$drawer-z-index;
   background-color: awsui.$color-background-container-content;
   inset-block-end: 0;
-  block-size: 100%;
   overflow-x: hidden;
-  overflow-y: auto;
-  /* stylelint-disable-next-line plugin/no-unsupported-browser-features */
-  overscroll-behavior-y: contain;
   word-wrap: break-word;
   pointer-events: auto;
   display: flex;
@@ -30,17 +27,16 @@
     display: none;
   }
 
-  & > .content-container {
+  & > .navigation {
     flex-grow: 1;
-  }
-
-  /*
-  A non-semantic node is added with a fixed width equal to the final Navigation
-  width. This will create the visual appearance of horizontal movement and
-  prevent unwanted text wrapping.
-  */
-  > .animated-content {
+    block-size: 100%;
+    overflow-y: auto;
+    /* stylelint-disable-next-line plugin/no-unsupported-browser-features */
+    overscroll-behavior-y: contain;
+    // Needs to have a fixed width during the enter animation, in which it will be cropped by its parent.
     inline-size: var(#{custom-props.$navigationWidth});
+    // Necessary for the hide-navigation button to be positioned correctly when a scrollbar is present.
+    position: relative;
   }
 
   // The Navigation drawer will take up the entire viewport on mobile