From 88e2063b8fc9aacf0438d739e0251e3c3c0c4c99 Mon Sep 17 00:00:00 2001
From: Avinash Dwarapu <dwaraa@amazon.com>
Date: Fri, 25 Aug 2023 16:23:03 +0200
Subject: [PATCH 1/8] revert: Revert "fix: Refactor and fix AppLayout and side
 navigation panel state and behaviour" (#1485)

---
 pages/side-navigation/app-layout.page.tsx     |  11 +-
 src/app-layout/__tests__/common.test.tsx      | 333 +++++++++---------
 src/app-layout/__tests__/mobile.test.tsx      | 128 ++-----
 src/app-layout/__tests__/utils.tsx            |   6 +-
 src/app-layout/index.tsx                      |  20 +-
 src/app-layout/visual-refresh/context.tsx     |  22 +-
 .../expandable-section-header.tsx             |   3 +-
 src/side-navigation/internal.tsx              |   3 +
 8 files changed, 207 insertions(+), 319 deletions(-)

diff --git a/pages/side-navigation/app-layout.page.tsx b/pages/side-navigation/app-layout.page.tsx
index cc20c50f7f..e202c6e7f8 100644
--- a/pages/side-navigation/app-layout.page.tsx
+++ b/pages/side-navigation/app-layout.page.tsx
@@ -4,7 +4,6 @@ import React from 'react';
 import SideNavigation, { SideNavigationProps } from '~components/side-navigation';
 import AppLayout from '~components/app-layout';
 import Badge from '~components/badge';
-import labels from '../app-layout/utils/labels';
 
 import logoSmall from './logos/logo-small.svg';
 
@@ -102,16 +101,12 @@ const items: SideNavigationProps.Item[] = [
 ];
 
 export default function SideNavigationPage() {
-  const [open, setOpen] = React.useState(true);
-
   return (
     <AppLayout
-      navigationOpen={open}
-      onNavigationChange={({ detail }) => {
-        setOpen(detail.open);
-      }}
+      toolsHide={true}
+      navigationOpen={true}
       contentType="form"
-      ariaLabels={labels}
+      ariaLabels={{ navigationClose: 'Close' }}
       navigation={
         <SideNavigation
           activeHref="#/"
diff --git a/src/app-layout/__tests__/common.test.tsx b/src/app-layout/__tests__/common.test.tsx
index 257b413008..9d38156c59 100644
--- a/src/app-layout/__tests__/common.test.tsx
+++ b/src/app-layout/__tests__/common.test.tsx
@@ -18,7 +18,7 @@ jest.mock('@cloudscape-design/component-toolkit', () => ({
   useContainerQuery: () => [100, () => {}],
 }));
 
-describeEachAppLayout(size => {
+describeEachAppLayout(() => {
   test('Default state', () => {
     const { wrapper } = renderComponent(<AppLayout />);
 
@@ -50,7 +50,6 @@ describeEachAppLayout(size => {
       openProp: 'navigationOpen',
       hideProp: 'navigationHide',
       handler: 'onNavigationChange',
-      expectedCallsOnMobileToggle: 2,
       findLandmarks: (wrapper: AppLayoutWrapper) => wrapper.findAll('nav'),
       findElement: (wrapper: AppLayoutWrapper) => wrapper.findNavigation(),
       findToggle: (wrapper: AppLayoutWrapper) => wrapper.findNavigationToggle(),
@@ -60,182 +59,170 @@ describeEachAppLayout(size => {
       openProp: 'toolsOpen',
       hideProp: 'toolsHide',
       handler: 'onToolsChange',
-      expectedCallsOnMobileToggle: 1,
       findLandmarks: (wrapper: AppLayoutWrapper) => wrapper.findAll('aside'),
       findElement: (wrapper: AppLayoutWrapper) => wrapper.findTools(),
       findToggle: (wrapper: AppLayoutWrapper) => wrapper.findToolsToggle(),
       findClose: (wrapper: AppLayoutWrapper) => wrapper.findToolsClose(),
     },
-  ].forEach(
-    ({
-      openProp,
-      hideProp,
-      handler,
-      expectedCallsOnMobileToggle,
-      findElement,
-      findLandmarks,
-      findToggle,
-      findClose,
-    }) => {
-      describe(`${openProp} prop`, () => {
-        test(`Should call handler once on open when toggle is clicked`, () => {
-          const onToggle = jest.fn();
-          const props = {
-            [openProp]: false,
-            [handler]: onToggle,
-          };
-          const { wrapper } = renderComponent(<AppLayout {...props} />);
-
-          findToggle(wrapper).click();
-          expect(onToggle).toHaveBeenCalledTimes(size === 'mobile' ? expectedCallsOnMobileToggle : 1);
-          expect(onToggle).toHaveBeenLastCalledWith(expect.objectContaining({ detail: { open: true } }));
-        });
-
-        test(`Should call handler once on open when span inside toggle is clicked`, () => {
-          const onToggle = jest.fn();
-          const props = {
-            [openProp]: false,
-            [handler]: onToggle,
-          };
-          const { wrapper } = renderComponent(<AppLayout {...props} />);
-
-          // Chrome bubbles up events from specific elements inside <button>s.
-          findToggle(wrapper).find('span')!.click();
-          expect(onToggle).toHaveBeenCalledTimes(size === 'mobile' ? expectedCallsOnMobileToggle : 1);
-          expect(onToggle).toHaveBeenLastCalledWith(expect.objectContaining({ detail: { open: true } }));
-        });
-
-        test(`Should call handler once on close`, () => {
-          const onToggle = jest.fn();
-          const props = {
-            [openProp]: true,
-            [handler]: onToggle,
-          };
-          const { wrapper } = renderComponent(<AppLayout {...props} />);
-
-          findClose(wrapper).click();
-          expect(onToggle).toHaveBeenCalledTimes(size === 'mobile' ? expectedCallsOnMobileToggle : 1);
-          expect(onToggle).toHaveBeenLastCalledWith(expect.objectContaining({ detail: { open: false } }));
-        });
-
-        test('Renders two landmarks in closed state', () => {
-          const props = {
-            [openProp]: false,
-            [handler]: () => {},
-          };
-          const { wrapper } = renderComponent(<AppLayout {...props} />);
-          const landmarks = findLandmarks(wrapper);
-          expect(landmarks).toHaveLength(2);
-
-          const toggleElement = findToggle(wrapper).getElement();
-
-          if (landmarks[0].getElement().contains(toggleElement)) {
-            expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'false');
-            expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'true');
-          } else {
-            expect(landmarks[1].getElement()).toContainElement(toggleElement);
-            expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'false');
-            expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'true');
-          }
-        });
-
-        test('Renders two landmarks in open state', () => {
-          const props = {
-            [openProp]: true,
-            [handler]: () => {},
-          };
-          const { wrapper } = renderComponent(<AppLayout {...props} />);
-          const landmarks = findLandmarks(wrapper);
-          expect(landmarks).toHaveLength(2);
-          const toggleElement = findToggle(wrapper).getElement();
-
-          if (landmarks[0].getElement().contains(toggleElement)) {
-            expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'true');
-            expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'false');
-          } else {
-            expect(landmarks[1].getElement()).toContainElement(toggleElement);
-            expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'true');
-            expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'false');
-          }
-        });
-
-        test('Renders aria-expanded only on toggle', () => {
-          const { wrapper } = renderComponent(<AppLayout />);
-          expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-expanded', 'false');
-          expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-haspopup', 'true');
-          expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-expanded');
-          expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-haspopup');
-        });
-
-        test('Does not add a label to the toggle and landmark when they are not defined', () => {
-          const { wrapper } = renderComponent(<AppLayout />);
-          expect(findToggle(wrapper).getElement()).not.toHaveAttribute('aria-label');
-          expect(findLandmarks(wrapper)[0].getElement()).not.toHaveAttribute('aria-label');
-        });
-
-        test('Adds labels to toggle button and landmark when defined', () => {
-          const labels = {
-            navigationToggle: 'toggle',
-            toolsToggle: 'toggle',
-            navigation: 'landmark',
-            tools: 'landmark',
-          };
-
-          const { wrapper } = renderComponent(<AppLayout ariaLabels={labels} />);
-          expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-label', 'toggle');
-          expect(findLandmarks(wrapper)[0].getElement()).toHaveAttribute('aria-label', 'landmark');
-        });
-
-        test('Close button does have a label if it is defined', () => {
-          const props = { [openProp]: true, [handler]: () => {} };
-          const labels = {
-            navigationClose: 'close label',
-            toolsClose: 'close label',
-          };
-          const { wrapper } = renderComponent(<AppLayout {...props} ariaLabels={labels} />);
-
-          expect(findClose(wrapper).getElement()).toHaveAttribute('aria-label', 'close label');
-        });
-
-        test('Close button does not render a label if is not defined', () => {
-          const props = { [openProp]: true, [handler]: () => {} };
-          const { wrapper } = renderComponent(<AppLayout {...props} />);
-
-          expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-label');
-        });
-
-        test('Opens and closes drawer in uncontrolled mode', () => {
-          // use content type with initial closed state for all drawers
-          const { wrapper } = renderComponent(<AppLayout contentType="form" />);
-          expect(isDrawerClosed(findElement(wrapper))).toBe(true);
-
-          act(() => findToggle(wrapper).click());
-          expect(isDrawerClosed(findElement(wrapper))).toBe(false);
-
-          act(() => findClose(wrapper).click());
-          expect(isDrawerClosed(findElement(wrapper))).toBe(true);
-        });
-
-        test('Moves focus between open and close buttons', () => {
-          // use content type with initial closed state for all drawers
-          const { wrapper } = renderComponent(<AppLayout contentType="form" />);
-
-          act(() => findToggle(wrapper).click());
-          expect(findClose(wrapper).getElement()).toBe(document.activeElement);
-
-          act(() => findClose(wrapper).click());
-          expect(findToggle(wrapper).getElement()).toBe(document.activeElement);
-        });
-
-        test(`Should not render the drawer if ${hideProp} is set to true`, () => {
-          const props = { [hideProp]: true };
-          const { wrapper } = renderComponent(<AppLayout {...props} />);
-          expect(findElement(wrapper)).toBeFalsy();
-          expect(findLandmarks(wrapper)).toHaveLength(0);
-        });
+  ].forEach(({ openProp, hideProp, handler, findElement, findLandmarks, findToggle, findClose }) => {
+    describe(`${openProp} prop`, () => {
+      test(`Should call handler once on open when toggle is clicked`, () => {
+        const onToggle = jest.fn();
+        const props = {
+          [openProp]: false,
+          [handler]: onToggle,
+        };
+        const { wrapper } = renderComponent(<AppLayout {...props} />);
+
+        findToggle(wrapper).click();
+        expect(onToggle).toHaveBeenCalledTimes(1);
+        expect(onToggle).toHaveBeenCalledWith(expect.objectContaining({ detail: { open: true } }));
       });
-    }
-  );
+
+      test(`Should call handler once on open when span inside toggle is clicked`, () => {
+        const onToggle = jest.fn();
+        const props = {
+          [openProp]: false,
+          [handler]: onToggle,
+        };
+        const { wrapper } = renderComponent(<AppLayout {...props} />);
+
+        // Chrome bubbles up events from specific elements inside <button>s.
+        findToggle(wrapper).find('span')!.click();
+        expect(onToggle).toHaveBeenCalledTimes(1);
+        expect(onToggle).toHaveBeenCalledWith(expect.objectContaining({ detail: { open: true } }));
+      });
+
+      test(`Should call handler once on close`, () => {
+        const onToggle = jest.fn();
+        const props = {
+          [openProp]: true,
+          [handler]: onToggle,
+        };
+        const { wrapper } = renderComponent(<AppLayout {...props} />);
+
+        findClose(wrapper).click();
+        expect(onToggle).toHaveBeenCalledTimes(1);
+        expect(onToggle).toHaveBeenCalledWith(expect.objectContaining({ detail: { open: false } }));
+      });
+
+      test('Renders two landmarks in closed state', () => {
+        const props = {
+          [openProp]: false,
+          [handler]: () => {},
+        };
+        const { wrapper } = renderComponent(<AppLayout {...props} />);
+        const landmarks = findLandmarks(wrapper);
+        expect(landmarks).toHaveLength(2);
+
+        const toggleElement = findToggle(wrapper).getElement();
+
+        if (landmarks[0].getElement().contains(toggleElement)) {
+          expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'false');
+          expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'true');
+        } else {
+          expect(landmarks[1].getElement()).toContainElement(toggleElement);
+          expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'false');
+          expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'true');
+        }
+      });
+
+      test('Renders two landmarks in open state', () => {
+        const props = {
+          [openProp]: true,
+          [handler]: () => {},
+        };
+        const { wrapper } = renderComponent(<AppLayout {...props} />);
+        const landmarks = findLandmarks(wrapper);
+        expect(landmarks).toHaveLength(2);
+        const toggleElement = findToggle(wrapper).getElement();
+
+        if (landmarks[0].getElement().contains(toggleElement)) {
+          expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'true');
+          expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'false');
+        } else {
+          expect(landmarks[1].getElement()).toContainElement(toggleElement);
+          expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'true');
+          expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'false');
+        }
+      });
+
+      test('Renders aria-expanded only on toggle', () => {
+        const { wrapper } = renderComponent(<AppLayout />);
+        expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-expanded', 'false');
+        expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-haspopup', 'true');
+        expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-expanded');
+        expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-haspopup');
+      });
+
+      test('Does not add a label to the toggle and landmark when they are not defined', () => {
+        const { wrapper } = renderComponent(<AppLayout />);
+        expect(findToggle(wrapper).getElement()).not.toHaveAttribute('aria-label');
+        expect(findLandmarks(wrapper)[0].getElement()).not.toHaveAttribute('aria-label');
+      });
+
+      test('Adds labels to toggle button and landmark when defined', () => {
+        const labels = {
+          navigationToggle: 'toggle',
+          toolsToggle: 'toggle',
+          navigation: 'landmark',
+          tools: 'landmark',
+        };
+
+        const { wrapper } = renderComponent(<AppLayout ariaLabels={labels} />);
+        expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-label', 'toggle');
+        expect(findLandmarks(wrapper)[0].getElement()).toHaveAttribute('aria-label', 'landmark');
+      });
+
+      test('Close button does have a label if it is defined', () => {
+        const props = { [openProp]: true, [handler]: () => {} };
+        const labels = {
+          navigationClose: 'close label',
+          toolsClose: 'close label',
+        };
+        const { wrapper } = renderComponent(<AppLayout {...props} ariaLabels={labels} />);
+
+        expect(findClose(wrapper).getElement()).toHaveAttribute('aria-label', 'close label');
+      });
+
+      test('Close button does not render a label if is not defined', () => {
+        const props = { [openProp]: true, [handler]: () => {} };
+        const { wrapper } = renderComponent(<AppLayout {...props} />);
+
+        expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-label');
+      });
+
+      test('Opens and closes drawer in uncontrolled mode', () => {
+        // use content type with initial closed state for all drawers
+        const { wrapper } = renderComponent(<AppLayout contentType="form" />);
+        expect(isDrawerClosed(findElement(wrapper))).toBe(true);
+
+        act(() => findToggle(wrapper).click());
+        expect(isDrawerClosed(findElement(wrapper))).toBe(false);
+
+        act(() => findClose(wrapper).click());
+        expect(isDrawerClosed(findElement(wrapper))).toBe(true);
+      });
+
+      test('Moves focus between open and close buttons', () => {
+        // use content type with initial closed state for all drawers
+        const { wrapper } = renderComponent(<AppLayout contentType="form" />);
+
+        act(() => findToggle(wrapper).click());
+        expect(findClose(wrapper).getElement()).toBe(document.activeElement);
+
+        act(() => findClose(wrapper).click());
+        expect(findToggle(wrapper).getElement()).toBe(document.activeElement);
+      });
+
+      test(`Should not render the drawer if ${hideProp} is set to true`, () => {
+        const props = { [hideProp]: true };
+        const { wrapper } = renderComponent(<AppLayout {...props} />);
+        expect(findElement(wrapper)).toBeFalsy();
+        expect(findLandmarks(wrapper)).toHaveLength(0);
+      });
+    });
+  });
 
   // Drawers tests
 
@@ -251,7 +238,7 @@ describeEachAppLayout(size => {
       const { wrapper } = renderComponent(<AppLayout contentType="form" {...drawersClosed} />);
 
       wrapper.findDrawersTriggers()![0].click();
-
+      expect(onChange).toHaveBeenCalledTimes(1);
       expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ detail: 'security' }));
     });
 
diff --git a/src/app-layout/__tests__/mobile.test.tsx b/src/app-layout/__tests__/mobile.test.tsx
index 4748d86fc4..d142b3249d 100644
--- a/src/app-layout/__tests__/mobile.test.tsx
+++ b/src/app-layout/__tests__/mobile.test.tsx
@@ -1,6 +1,6 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-import React, { useState } from 'react';
+import React from 'react';
 import { act } from 'react-dom/test-utils';
 import {
   describeEachThemeAppLayout,
@@ -22,33 +22,12 @@ import testUtilsStyles from '../../../lib/components/app-layout/test-classes/sty
 
 import visualRefreshRefactoredStyles from '../../../lib/components/app-layout/visual-refresh/styles.css.js';
 import { findUpUntil } from '../../../lib/components/internal/utils/dom';
-import SideNavigation from '../../../lib/components/side-navigation';
 
 jest.mock('@cloudscape-design/component-toolkit/internal', () => ({
   ...jest.requireActual('@cloudscape-design/component-toolkit/internal'),
   isMotionDisabled: jest.fn().mockReturnValue(true),
 }));
 
-function AppLayoutWithControlledNavigation({
-  initialNavigationOpen,
-  navigation,
-}: {
-  initialNavigationOpen: boolean;
-  navigation: React.ReactNode;
-}) {
-  const [navigationOpen, setNavigationOpen] = useState(initialNavigationOpen);
-
-  return (
-    <AppLayout
-      navigationOpen={navigationOpen}
-      onNavigationChange={({ detail }) => {
-        setNavigationOpen(detail.open);
-      }}
-      navigation={navigation}
-    />
-  );
-}
-
 describeEachThemeAppLayout(true, theme => {
   // In refactored Visual Refresh different styles are used compared to Classic
   const mobileBarClassName = theme === 'refresh' ? testUtilsStyles['mobile-bar'] : toolbarStyles['mobile-bar'];
@@ -67,37 +46,6 @@ describeEachThemeAppLayout(true, theme => {
     expect(wrapper.findToolsToggle().getElement()).toBeEnabled();
   });
 
-  test('AppLayout with controlled navigation has navigation forcely closed on initial load', () => {
-    const { wrapper } = renderComponent(
-      <AppLayoutWithControlledNavigation
-        initialNavigationOpen={true}
-        navigation={
-          <>
-            <h1>Navigation</h1>
-            <a href="test">Link</a>
-          </>
-        }
-      />
-    );
-    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
-  });
-
-  test('AppLayout with uncontrolled navigation has navigation forcely closed on initial load', () => {
-    const { wrapper } = renderComponent(
-      <AppLayout
-        navigation={
-          <>
-            <h1>Navigation</h1>
-            <a href="test">Link</a>
-          </>
-        }
-      />
-    );
-    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
-  });
-
   test('renders open navigation state', () => {
     const { wrapper } = renderComponent(<AppLayout navigationOpen={true} onNavigationChange={() => {}} />);
     expect(wrapper.findNavigation()).toBeTruthy();
@@ -138,58 +86,30 @@ describeEachThemeAppLayout(true, theme => {
   });
 
   test('closes navigation when clicking on links', () => {
+    const onNavigationChange = jest.fn();
     const { wrapper } = renderComponent(
-      <AppLayoutWithControlledNavigation
-        initialNavigationOpen={true}
+      <AppLayout
+        navigationOpen={true}
+        onNavigationChange={onNavigationChange}
         navigation={
           <>
             <h1>Navigation</h1>
-            <a href="test">Link</a>
+            <a href="#">Link</a>
           </>
         }
       />
     );
-    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
-
-    wrapper.findNavigationToggle().click();
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
-
     wrapper.findNavigation().find('a')!.click();
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
-  });
-
-  test('closes navigation when clicking on a link in the Side Navigation component', () => {
-    const { wrapper } = renderComponent(
-      <AppLayoutWithControlledNavigation
-        initialNavigationOpen={true}
-        navigation={
-          <SideNavigation
-            items={[
-              {
-                type: 'link',
-                text: 'Page 1',
-                href: '#/page1',
-              },
-            ]}
-          />
-        }
-      />
-    );
-    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
-
-    wrapper.findNavigationToggle().click();
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
 
-    wrapper.findNavigation().find('a')!.click();
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
+    expect(onNavigationChange).toHaveBeenCalledWith(expect.objectContaining({ detail: { open: false } }));
   });
 
   test('does not close navigation when anchor without href was clicked', () => {
+    const onNavigationChange = jest.fn();
     const { wrapper } = renderComponent(
-      <AppLayoutWithControlledNavigation
-        initialNavigationOpen={true}
+      <AppLayout
+        navigationOpen={true}
+        onNavigationChange={onNavigationChange}
         navigation={
           <>
             <h1>Navigation</h1>
@@ -198,36 +118,28 @@ describeEachThemeAppLayout(true, theme => {
         }
       />
     );
-    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
-
-    wrapper.findNavigationToggle().click();
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
-
     wrapper.findNavigation().find('a')!.click();
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
+
+    expect(onNavigationChange).not.toHaveBeenCalled();
   });
 
   test('does not close navigation when other elements were clicked', () => {
+    const onNavigationChange = jest.fn();
     const { wrapper } = renderComponent(
-      <AppLayoutWithControlledNavigation
-        initialNavigationOpen={true}
+      <AppLayout
+        navigationOpen={true}
+        onNavigationChange={onNavigationChange}
         navigation={
           <>
             <h1>Navigation</h1>
-            <a>Link</a>
+            <a href="#">Link</a>
           </>
         }
       />
     );
-    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
-
-    wrapper.findNavigationToggle().click();
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
-
     wrapper.findNavigation().find('h1')!.click();
-    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
+
+    expect(onNavigationChange).not.toHaveBeenCalled();
   });
 
   test('does not close tools when clicking on any element', () => {
diff --git a/src/app-layout/__tests__/utils.tsx b/src/app-layout/__tests__/utils.tsx
index 4f06a052b6..b01ed83590 100644
--- a/src/app-layout/__tests__/utils.tsx
+++ b/src/app-layout/__tests__/utils.tsx
@@ -76,9 +76,9 @@ export function describeEachThemeAppLayout(isMobile: boolean, callback: (theme:
   }
 }
 
-export function describeEachAppLayout(callback: (size: 'desktop' | 'mobile') => void) {
+export function describeEachAppLayout(callback: () => void) {
   for (const theme of ['refresh', 'classic']) {
-    for (const size of ['desktop', 'mobile'] as const) {
+    for (const size of ['desktop', 'mobile']) {
       describe(`Theme=${theme}, Size=${size}`, () => {
         beforeEach(() => {
           (useMobile as jest.Mock).mockReturnValue(size === 'mobile');
@@ -88,7 +88,7 @@ export function describeEachAppLayout(callback: (size: 'desktop' | 'mobile') =>
           (useMobile as jest.Mock).mockReset();
           (useVisualRefresh as jest.Mock).mockReset();
         });
-        callback(size);
+        callback();
       });
     }
   }
diff --git a/src/app-layout/index.tsx b/src/app-layout/index.tsx
index 62604687b0..3440267a8b 100644
--- a/src/app-layout/index.tsx
+++ b/src/app-layout/index.tsx
@@ -170,11 +170,14 @@ const OldAppLayout = React.forwardRef(
       setLastInteraction: setDrawerLastInteraction,
     } = useDrawerFocusControl([activeDrawer?.resizable], toolsOpen || activeDrawer !== undefined, true);
 
-    const onNavigationToggle = useStableCallback((open: boolean) => {
-      setNavigationOpen(open);
-      focusNavButtons();
-      fireNonCancelableEvent(onNavigationChange, { open });
-    });
+    const onNavigationToggle = useCallback(
+      (open: boolean) => {
+        setNavigationOpen(open);
+        focusNavButtons();
+        fireNonCancelableEvent(onNavigationChange, { open });
+      },
+      [setNavigationOpen, onNavigationChange, focusNavButtons]
+    );
     const onToolsToggle = useCallback(
       (open: boolean) => {
         setToolsOpen(open);
@@ -194,13 +197,6 @@ const OldAppLayout = React.forwardRef(
       }
     };
 
-    useEffect(() => {
-      // Close navigation drawer on mobile so that the main content is visible
-      if (isMobile) {
-        onNavigationToggle(false);
-      }
-    }, [isMobile, onNavigationToggle]);
-
     const navigationVisible = !navigationHide && navigationOpen;
     const toolsVisible = !toolsHide && toolsOpen;
 
diff --git a/src/app-layout/visual-refresh/context.tsx b/src/app-layout/visual-refresh/context.tsx
index 8dd30ab048..761ce64a02 100644
--- a/src/app-layout/visual-refresh/context.tsx
+++ b/src/app-layout/visual-refresh/context.tsx
@@ -26,7 +26,7 @@ import { SplitPanelSideToggleProps } from '../../internal/context/split-panel-co
 import { useObservedElement } from '../utils/use-observed-element';
 import { useMobile } from '../../internal/hooks/use-mobile';
 import { DrawerItem, InternalDrawerProps } from '../drawer/interfaces';
-import { useStableCallback, warnOnce } from '@cloudscape-design/component-toolkit/internal';
+import { warnOnce } from '@cloudscape-design/component-toolkit/internal';
 import useResize from '../utils/use-resize';
 import styles from './styles.css.js';
 import { useContainerQuery } from '@cloudscape-design/component-toolkit';
@@ -179,18 +179,14 @@ export const AppLayoutInternalsProvider = React.forwardRef(
 
     const { refs: navigationRefs, setFocus: focusNavButtons } = useFocusControl(isNavigationOpen);
 
-    const handleNavigationClick = useStableCallback(function handleNavigationChange(isOpen: boolean) {
-      setIsNavigationOpen(isOpen);
-      focusNavButtons();
-      fireNonCancelableEvent(props.onNavigationChange, { open: isOpen });
-    });
-
-    useEffect(() => {
-      // Close navigation drawer on mobile so that the main content is visible
-      if (isMobile) {
-        handleNavigationClick(false);
-      }
-    }, [isMobile, handleNavigationClick]);
+    const handleNavigationClick = useCallback(
+      function handleNavigationChange(isOpen: boolean) {
+        setIsNavigationOpen(isOpen);
+        focusNavButtons();
+        fireNonCancelableEvent(props.onNavigationChange, { open: isOpen });
+      },
+      [props.onNavigationChange, setIsNavigationOpen, focusNavButtons]
+    );
 
     /**
      * The useControllable hook will set the default value and manage either
diff --git a/src/expandable-section/expandable-section-header.tsx b/src/expandable-section/expandable-section-header.tsx
index 1429fb6b46..26a7f1a9da 100644
--- a/src/expandable-section/expandable-section-header.tsx
+++ b/src/expandable-section/expandable-section-header.tsx
@@ -95,7 +95,7 @@ const ExpandableNavigationHeader = ({
   icon,
 }: ExpandableNavigationHeaderProps) => {
   return (
-    <div id={id} className={clsx(className, styles['click-target'])}>
+    <div id={id} className={clsx(className, styles['click-target'])} onClick={onClick}>
       <button
         className={clsx(styles['icon-container'], styles['expand-button'])}
         aria-labelledby={ariaLabelledBy}
@@ -103,7 +103,6 @@ const ExpandableNavigationHeader = ({
         aria-controls={ariaControls}
         aria-expanded={expanded}
         type="button"
-        onClick={onClick}
       >
         {icon}
       </button>
diff --git a/src/side-navigation/internal.tsx b/src/side-navigation/internal.tsx
index 601a9e54b2..66048ce75c 100644
--- a/src/side-navigation/internal.tsx
+++ b/src/side-navigation/internal.tsx
@@ -227,6 +227,9 @@ function Link({ definition, expanded, activeHref, fireFollow }: LinkProps) {
 
   const onClick = useCallback(
     (event: React.MouseEvent) => {
+      // Prevent the click event from toggling outer expandable sections.
+      event.stopPropagation();
+
       if (isPlainLeftClick(event)) {
         fireFollow(definition, event);
       }

From ea396d805c91e6b422323e011e4c0a93066ffa86 Mon Sep 17 00:00:00 2001
From: Katie George <katiegeorge94@gmail.com>
Date: Fri, 25 Aug 2023 09:01:26 -0700
Subject: [PATCH 2/8] fix: Fixes aria expanded and aria controls in AppLayout
 (#1479)

Co-authored-by: Katie George <katiegeo@amazon.com>
---
 src/app-layout/__tests__/common.test.tsx      |  9 +++++++-
 src/app-layout/__tests__/desktop.test.tsx     |  7 ++++++
 src/app-layout/drawer/index.tsx               | 22 ++++++++++++++++---
 src/app-layout/mobile-toolbar/index.tsx       |  4 ++--
 src/app-layout/toggles/index.tsx              | 16 ++++++++++++--
 src/app-layout/toggles/interfaces.ts          |  1 +
 src/app-layout/visual-refresh/context.tsx     |  7 ++++++
 src/app-layout/visual-refresh/drawers.tsx     | 11 +++++++++-
 src/app-layout/visual-refresh/navigation.tsx  |  1 +
 src/app-layout/visual-refresh/split-panel.tsx |  2 ++
 src/app-layout/visual-refresh/tools.tsx       |  7 ++++++
 .../visual-refresh/trigger-button.tsx         | 18 +++++++++++++--
 src/button-dropdown/interfaces.ts             |  1 +
 src/button-dropdown/internal.tsx              |  1 +
 14 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/src/app-layout/__tests__/common.test.tsx b/src/app-layout/__tests__/common.test.tsx
index 9d38156c59..8b5710e8bf 100644
--- a/src/app-layout/__tests__/common.test.tsx
+++ b/src/app-layout/__tests__/common.test.tsx
@@ -148,7 +148,13 @@ describeEachAppLayout(() => {
       });
 
       test('Renders aria-expanded only on toggle', () => {
-        const { wrapper } = renderComponent(<AppLayout />);
+        const props = {
+          [openProp]: false,
+          [handler]: () => {},
+        };
+
+        const { wrapper } = renderComponent(<AppLayout {...props} />);
+
         expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-expanded', 'false');
         expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-haspopup', 'true');
         expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-expanded');
@@ -281,6 +287,7 @@ describeEachAppLayout(() => {
       expect(wrapper.findDrawersTriggers()![0].getElement()).toHaveAttribute('aria-expanded', 'false');
       expect(wrapper.findDrawersTriggers()![0].getElement()).toHaveAttribute('aria-haspopup', 'true');
       wrapper.findDrawersTriggers()![0].click();
+      expect(wrapper.findDrawersTriggers()![0].getElement()).toHaveAttribute('aria-expanded', 'true');
       expect(wrapper.findActiveDrawerCloseButton()!.getElement()).not.toHaveAttribute('aria-expanded');
       expect(wrapper.findActiveDrawerCloseButton()!.getElement()).not.toHaveAttribute('aria-haspopup');
     });
diff --git a/src/app-layout/__tests__/desktop.test.tsx b/src/app-layout/__tests__/desktop.test.tsx
index 1f7509f07e..78dff81cf2 100644
--- a/src/app-layout/__tests__/desktop.test.tsx
+++ b/src/app-layout/__tests__/desktop.test.tsx
@@ -240,6 +240,13 @@ describeEachThemeAppLayout(false, () => {
 
     expect(wrapper.findDrawersTriggers()!.length).toBeLessThan(100);
   });
+
+  test('Renders aria-controls on toggle only when active', () => {
+    const { wrapper } = renderComponent(<AppLayout contentType="form" {...singleDrawer} />);
+    expect(wrapper.findDrawersTriggers()![0].getElement()).not.toHaveAttribute('aria-controls');
+    act(() => wrapper.findDrawersTriggers()![0].click());
+    expect(wrapper.findDrawersTriggers()![0].getElement()).toHaveAttribute('aria-controls', 'security');
+  });
 });
 
 // In VR we use a custom CSS property so we cannot test the style declaration.
diff --git a/src/app-layout/drawer/index.tsx b/src/app-layout/drawer/index.tsx
index 164f9daf78..6ae0e0b79f 100644
--- a/src/app-layout/drawer/index.tsx
+++ b/src/app-layout/drawer/index.tsx
@@ -66,13 +66,14 @@ export const Drawer = React.forwardRef(
           iconName={iconName}
           ariaLabel={openLabel}
           onClick={() => onToggle(true)}
-          ariaExpanded={false}
+          ariaExpanded={isOpen ? undefined : false}
         />
       </TagName>
     );
 
     return (
       <div
+        id={drawers?.activeDrawerId}
         ref={ref}
         className={clsx(styles.drawer, {
           [styles['drawer-closed']]: !isOpen,
@@ -132,6 +133,7 @@ interface DrawerTriggerProps {
   testUtilsClassName?: string;
   ariaLabel: string | undefined;
   ariaExpanded: boolean;
+  ariaControls?: string;
   badge: boolean | undefined;
   itemId?: string;
   isActive: boolean;
@@ -141,7 +143,17 @@ interface DrawerTriggerProps {
 
 const DrawerTrigger = React.forwardRef(
   (
-    { testUtilsClassName, ariaLabel, ariaExpanded, badge, itemId, isActive, trigger, onClick }: DrawerTriggerProps,
+    {
+      testUtilsClassName,
+      ariaLabel,
+      ariaExpanded,
+      ariaControls,
+      badge,
+      itemId,
+      isActive,
+      trigger,
+      onClick,
+    }: DrawerTriggerProps,
     ref: React.Ref<{ focus: () => void }>
   ) => (
     <div className={clsx(styles['drawer-trigger'], isActive && styles['drawer-trigger-active'])} onClick={onClick}>
@@ -152,6 +164,7 @@ const DrawerTrigger = React.forwardRef(
         iconSvg={trigger.iconSvg}
         ariaLabel={ariaLabel}
         ariaExpanded={ariaExpanded}
+        ariaControls={ariaControls}
         badge={badge}
         testId={itemId && `awsui-app-layout-trigger-${itemId}`}
       />
@@ -189,6 +202,8 @@ export const DrawerTriggersBar = ({ isMobile, topOffset, bottomOffset, drawers }
         ref={triggersContainerRef}
         style={{ top: topOffset, bottom: bottomOffset }}
         className={clsx(styles['drawer-content'])}
+        role="toolbar"
+        aria-orientation="vertical"
       >
         {!isMobile && (
           <aside
@@ -201,8 +216,9 @@ export const DrawerTriggersBar = ({ isMobile, topOffset, bottomOffset, drawers }
                   <DrawerTrigger
                     key={index}
                     testUtilsClassName={testutilStyles['drawers-trigger']}
-                    ariaExpanded={drawers?.activeDrawerId !== undefined}
+                    ariaExpanded={drawers?.activeDrawerId === item.id}
                     ariaLabel={item.ariaLabels?.triggerButton}
+                    ariaControls={drawers?.activeDrawerId === item.id ? item.id : undefined}
                     trigger={item.trigger}
                     badge={item.badge}
                     itemId={item.id}
diff --git a/src/app-layout/mobile-toolbar/index.tsx b/src/app-layout/mobile-toolbar/index.tsx
index 45f756cee4..3cad1116e3 100644
--- a/src/app-layout/mobile-toolbar/index.tsx
+++ b/src/app-layout/mobile-toolbar/index.tsx
@@ -129,7 +129,7 @@ export function MobileToolbar({
           aria-label={drawers.ariaLabel}
           className={clsx(styles['drawers-container'], testutilStyles['drawers-mobile-triggers-container'])}
         >
-          {visibleItems.map((item: DrawerItem, index: number) => (
+          {visibleItems.map((item, index) => (
             <div
               className={clsx(styles['mobile-toggle'], styles['mobile-toggle-type-drawer'])}
               key={index}
@@ -141,7 +141,7 @@ export function MobileToolbar({
                 iconSvg={item.trigger.iconSvg}
                 badge={item.badge}
                 ariaLabel={item.ariaLabels?.triggerButton}
-                ariaExpanded={drawers.activeDrawerId !== undefined}
+                ariaExpanded={drawers.activeDrawerId === item.id}
                 testId={`awsui-app-layout-trigger-${item.id}`}
               />
             </div>
diff --git a/src/app-layout/toggles/index.tsx b/src/app-layout/toggles/index.tsx
index 60f392cdfe..1e72bbef69 100644
--- a/src/app-layout/toggles/index.tsx
+++ b/src/app-layout/toggles/index.tsx
@@ -32,7 +32,18 @@ export const togglesConfig = {
 
 export const ToggleButton = React.forwardRef(
   (
-    { className, ariaLabel, ariaExpanded, iconName, iconSvg, disabled, testId, onClick, badge }: AppLayoutButtonProps,
+    {
+      className,
+      ariaLabel,
+      ariaExpanded,
+      ariaControls,
+      iconName,
+      iconSvg,
+      disabled,
+      testId,
+      onClick,
+      badge,
+    }: AppLayoutButtonProps,
     ref: React.Ref<{ focus(): void }>
   ) => {
     return (
@@ -43,8 +54,9 @@ export const ToggleButton = React.forwardRef(
         type="button"
         onClick={onClick}
         disabled={disabled}
-        aria-expanded={ariaExpanded ? undefined : false}
+        aria-expanded={ariaExpanded}
         aria-haspopup={ariaExpanded ? undefined : true}
+        aria-controls={ariaControls}
         data-testid={testId}
       >
         <InternalIcon svg={iconSvg} name={iconName} badge={badge} />
diff --git a/src/app-layout/toggles/interfaces.ts b/src/app-layout/toggles/interfaces.ts
index 3d1d2e62a9..b0b9e5378f 100644
--- a/src/app-layout/toggles/interfaces.ts
+++ b/src/app-layout/toggles/interfaces.ts
@@ -7,6 +7,7 @@ export interface AppLayoutButtonProps {
   className?: string;
   ariaLabel: string | undefined;
   ariaExpanded?: boolean;
+  ariaControls?: string;
   iconName?: IconProps.Name;
   iconSvg?: React.ReactNode;
   onClick?: () => void;
diff --git a/src/app-layout/visual-refresh/context.tsx b/src/app-layout/visual-refresh/context.tsx
index 761ce64a02..efada3de4b 100644
--- a/src/app-layout/visual-refresh/context.tsx
+++ b/src/app-layout/visual-refresh/context.tsx
@@ -32,6 +32,7 @@ import styles from './styles.css.js';
 import { useContainerQuery } from '@cloudscape-design/component-toolkit';
 import useBackgroundOverlap from './use-background-overlap';
 import { useDrawers } from '../utils/use-drawers';
+import { useUniqueId } from '../../internal/hooks/use-unique-id';
 
 interface AppLayoutInternals extends AppLayoutProps {
   activeDrawerId: string | undefined;
@@ -77,6 +78,7 @@ interface AppLayoutInternals extends AppLayoutProps {
   setSplitPanelReportedHeaderHeight: (value: number) => void;
   headerHeight: number;
   footerHeight: number;
+  splitPanelControlId: string;
   splitPanelMaxWidth: number;
   splitPanelMinWidth: number;
   splitPanelPosition: AppLayoutProps.SplitPanelPosition;
@@ -86,6 +88,7 @@ interface AppLayoutInternals extends AppLayoutProps {
   setSplitPanelToggle: (toggle: SplitPanelSideToggleProps) => void;
   splitPanelDisplayed: boolean;
   splitPanelRefs: SplitPanelFocusControlRefs;
+  toolsControlId: string;
   toolsRefs: FocusControlRefs;
 }
 
@@ -372,6 +375,8 @@ export const AppLayoutInternalsProvider = React.forwardRef(
       ariaLabel: undefined,
     });
     const splitPanelDisplayed = !!(splitPanelToggle.displayed || isSplitPanelOpen);
+    const splitPanelControlId = useUniqueId('split-panel-');
+    const toolsControlId = useUniqueId('tools-');
 
     const [splitPanelSize, setSplitPanelSize] = useControllable(
       props.splitPanelSize,
@@ -633,6 +638,7 @@ export const AppLayoutInternalsProvider = React.forwardRef(
           setSplitPanelReportedSize,
           setSplitPanelReportedHeaderHeight,
           splitPanel,
+          splitPanelControlId,
           splitPanelDisplayed,
           splitPanelMaxWidth,
           splitPanelMinWidth,
@@ -644,6 +650,7 @@ export const AppLayoutInternalsProvider = React.forwardRef(
           splitPanelToggle,
           setSplitPanelToggle,
           splitPanelRefs,
+          toolsControlId,
           toolsHide,
           toolsOpen: isToolsOpen,
           toolsWidth,
diff --git a/src/app-layout/visual-refresh/drawers.tsx b/src/app-layout/visual-refresh/drawers.tsx
index 9bbe3724aa..72f297443b 100644
--- a/src/app-layout/visual-refresh/drawers.tsx
+++ b/src/app-layout/visual-refresh/drawers.tsx
@@ -95,6 +95,7 @@ function ActiveDrawer() {
 
   return (
     <aside
+      id={activeDrawerId}
       aria-hidden={isHidden}
       aria-label={computedAriaLabels.content}
       className={clsx(styles.drawer, sharedStyles['with-motion'], {
@@ -158,6 +159,7 @@ function DesktopTriggers() {
     isSplitPanelOpen,
     isToolsOpen,
     splitPanel,
+    splitPanelControlId,
     splitPanelDisplayed,
     splitPanelPosition,
     splitPanelRefs,
@@ -220,11 +222,15 @@ function DesktopTriggers() {
           [styles['has-multiple-triggers']]: hasMultipleTriggers,
           [styles['has-open-drawer']]: hasOpenDrawer,
         })}
+        role="toolbar"
+        aria-orientation="vertical"
       >
         {visibleItems.map(item => {
           return (
             <TriggerButton
               ariaLabel={item.ariaLabels?.triggerButton}
+              ariaExpanded={item.id === activeDrawerId}
+              ariaControls={activeDrawerId === item.id ? item.id : undefined}
               className={clsx(styles['drawers-trigger'], testutilStyles['drawers-trigger'])}
               iconName={item.trigger.iconName}
               iconSvg={item.trigger.iconSvg}
@@ -245,11 +251,12 @@ function DesktopTriggers() {
           <OverflowMenu
             items={overflowItems}
             ariaLabel={drawersOverflowAriaLabel}
-            customTriggerBuilder={({ onClick, triggerRef, ariaLabel, testUtilsClass }) => (
+            customTriggerBuilder={({ onClick, triggerRef, ariaLabel, testUtilsClass, ariaExpanded }) => (
               <div className={clsx(styles['trigger-wrapper'])}>
                 <TriggerButton
                   ref={triggerRef}
                   ariaLabel={ariaLabel}
+                  ariaExpanded={ariaExpanded}
                   className={clsx(
                     styles['drawers-trigger'],
                     {
@@ -272,6 +279,8 @@ function DesktopTriggers() {
         {hasSplitPanel && splitPanelToggle.displayed && (
           <TriggerButton
             ariaLabel={splitPanelToggle.ariaLabel}
+            ariaControls={splitPanelControlId}
+            ariaExpanded={!!isSplitPanelOpen}
             className={clsx(styles['drawers-trigger'], splitPanelStyles['open-button'])}
             iconName="view-vertical"
             onClick={() => handleSplitPanelClick()}
diff --git a/src/app-layout/visual-refresh/navigation.tsx b/src/app-layout/visual-refresh/navigation.tsx
index 0bcaefa849..4da679cc8c 100644
--- a/src/app-layout/visual-refresh/navigation.tsx
+++ b/src/app-layout/visual-refresh/navigation.tsx
@@ -75,6 +75,7 @@ export default function Navigation() {
             >
               <TriggerButton
                 ariaLabel={ariaLabels?.navigationToggle}
+                ariaExpanded={isNavigationOpen ? undefined : false}
                 iconName="menu"
                 className={testutilStyles['navigation-toggle']}
                 onClick={() => handleNavigationClick(true)}
diff --git a/src/app-layout/visual-refresh/split-panel.tsx b/src/app-layout/visual-refresh/split-panel.tsx
index 2a2f56af43..61149646a3 100644
--- a/src/app-layout/visual-refresh/split-panel.tsx
+++ b/src/app-layout/visual-refresh/split-panel.tsx
@@ -120,6 +120,7 @@ function SplitPanelSide() {
     splitPanelMaxWidth,
     splitPanelMinWidth,
     splitPanelReportedSize,
+    splitPanelControlId,
   } = useAppLayoutInternals();
 
   if (!splitPanel) {
@@ -130,6 +131,7 @@ function SplitPanelSide() {
     <Transition in={isSplitPanelOpen ?? false} exit={false}>
       {(state, transitionEventsRef) => (
         <section
+          id={splitPanelControlId}
           aria-hidden={!isSplitPanelOpen || splitPanelPosition === 'bottom' ? true : false}
           className={clsx(styles['split-panel-side'], styles[`position-${splitPanelPosition}`], {
             [styles.animating]: state === 'entering',
diff --git a/src/app-layout/visual-refresh/tools.tsx b/src/app-layout/visual-refresh/tools.tsx
index ff98a85cbc..3e000fdaaf 100644
--- a/src/app-layout/visual-refresh/tools.tsx
+++ b/src/app-layout/visual-refresh/tools.tsx
@@ -37,11 +37,13 @@ export default function Tools({ children }: ToolsProps) {
     isToolsOpen,
     loseToolsFocus,
     splitPanel,
+    splitPanelControlId,
     splitPanelDisplayed,
     splitPanelPosition,
     splitPanelRefs,
     splitPanelToggle,
     tools,
+    toolsControlId,
     toolsHide,
     toolsRefs,
     toolsWidth,
@@ -84,6 +86,7 @@ export default function Tools({ children }: ToolsProps) {
 
           {!toolsHide && (
             <aside
+              id={toolsControlId}
               aria-hidden={!isToolsOpen ? true : false}
               aria-label={ariaLabels?.tools ?? undefined}
               className={clsx(
@@ -130,6 +133,8 @@ export default function Tools({ children }: ToolsProps) {
               {!toolsHide && (
                 <TriggerButton
                   ariaLabel={ariaLabels?.toolsToggle}
+                  ariaControls={toolsControlId}
+                  ariaExpanded={isToolsOpen}
                   iconName="status-info"
                   onClick={() => handleToolsClick(!isToolsOpen)}
                   selected={hasSplitPanel && isToolsOpen}
@@ -141,6 +146,8 @@ export default function Tools({ children }: ToolsProps) {
               {hasSplitPanel && splitPanelToggle.displayed && (
                 <TriggerButton
                   ariaLabel={splitPanelToggle.ariaLabel}
+                  ariaControls={splitPanelControlId}
+                  ariaExpanded={!!isSplitPanelOpen}
                   iconName="view-vertical"
                   onClick={() => handleSplitPanelClick()}
                   selected={hasSplitPanel && isSplitPanelOpen}
diff --git a/src/app-layout/visual-refresh/trigger-button.tsx b/src/app-layout/visual-refresh/trigger-button.tsx
index 86f4daddf1..e575a83014 100644
--- a/src/app-layout/visual-refresh/trigger-button.tsx
+++ b/src/app-layout/visual-refresh/trigger-button.tsx
@@ -12,6 +12,8 @@ export interface TriggerButtonProps {
   className?: string;
   iconName?: IconProps.Name;
   iconSvg?: React.ReactNode;
+  ariaExpanded: boolean | undefined;
+  ariaControls?: string;
   testId?: string;
   onClick: React.MouseEventHandler<HTMLButtonElement>;
   selected?: boolean;
@@ -19,13 +21,25 @@ export interface TriggerButtonProps {
 }
 
 function TriggerButton(
-  { ariaLabel, className, iconName, iconSvg, onClick, testId, badge, selected = false }: TriggerButtonProps,
+  {
+    ariaLabel,
+    className,
+    iconName,
+    iconSvg,
+    ariaExpanded,
+    ariaControls,
+    onClick,
+    testId,
+    badge,
+    selected = false,
+  }: TriggerButtonProps,
   ref: React.Ref<ButtonProps.Ref>
 ) {
   return (
     <div className={clsx(styles['trigger-wrapper'])}>
       <button
-        aria-expanded={false}
+        aria-expanded={ariaExpanded}
+        aria-controls={ariaControls}
         aria-haspopup={true}
         aria-label={ariaLabel}
         className={clsx(
diff --git a/src/button-dropdown/interfaces.ts b/src/button-dropdown/interfaces.ts
index fd104c466a..c48180a7bb 100644
--- a/src/button-dropdown/interfaces.ts
+++ b/src/button-dropdown/interfaces.ts
@@ -259,4 +259,5 @@ export interface CustomTriggerProps {
   disabled: boolean;
   isOpen: boolean;
   onClick: () => void;
+  ariaExpanded: boolean;
 }
diff --git a/src/button-dropdown/internal.tsx b/src/button-dropdown/internal.tsx
index b87c9def46..0072240b18 100644
--- a/src/button-dropdown/internal.tsx
+++ b/src/button-dropdown/internal.tsx
@@ -163,6 +163,7 @@ const InternalButtonDropdown = React.forwardRef(
         <div className={styles['dropdown-trigger']}>
           {customTriggerBuilder({
             testUtilsClass: styles['test-utils-button-trigger'],
+            ariaExpanded: canBeOpened && isOpen,
             onClick: clickHandler,
             triggerRef,
             ariaLabel,

From 45dae1a6b271f88e7fc5674497384960a4e43bac Mon Sep 17 00:00:00 2001
From: Avinash Dwarapu <dwaraa@amazon.com>
Date: Fri, 25 Aug 2023 22:00:56 +0200
Subject: [PATCH 3/8] fix: Always offset sticky scrollbar if sticky columns are
 present (#1488)

---
 src/table/internal.tsx                                   | 2 ++
 .../sticky-scrollbar/__tests__/sticky-scrollbar.test.tsx | 9 +++++++++
 src/table/sticky-scrollbar/sticky-scrollbar.tsx          | 8 ++++++--
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/table/internal.tsx b/src/table/internal.tsx
index ca644c02e7..6539fa8d92 100644
--- a/src/table/internal.tsx
+++ b/src/table/internal.tsx
@@ -184,6 +184,7 @@ const InternalTable = React.forwardRef(
       stickyColumnsLast: stickyColumns?.last || 0,
     });
 
+    const hasStickyColumns = !!((stickyColumns?.first ?? 0) + (stickyColumns?.last ?? 0) > 0);
     const hasEditableCells = !!columnDefinitions.find(col => col.editConfig);
     const tableRole = hasEditableCells ? 'grid-no-navigation' : 'table';
 
@@ -476,6 +477,7 @@ const InternalTable = React.forwardRef(
               wrapperRef={wrapperRefObject}
               tableRef={tableRefObject}
               onScroll={handleScroll}
+              hasStickyColumns={hasStickyColumns}
             />
           </InternalContainer>
         </ColumnWidthsProvider>
diff --git a/src/table/sticky-scrollbar/__tests__/sticky-scrollbar.test.tsx b/src/table/sticky-scrollbar/__tests__/sticky-scrollbar.test.tsx
index e3e750cc10..37bdcef3a5 100644
--- a/src/table/sticky-scrollbar/__tests__/sticky-scrollbar.test.tsx
+++ b/src/table/sticky-scrollbar/__tests__/sticky-scrollbar.test.tsx
@@ -19,6 +19,15 @@ it('adds an offset if the scrollbar is an overlay', () => {
   expect(container.querySelector(`.${styles['sticky-scrollbar-offset']}`)).not.toBeNull();
 });
 
+it('adds an offset if the scrollbar occupies space but the table has sticky columns', () => {
+  const tableRef = { current: document.createElement('table') };
+  const wrapperRef = { current: document.createElement('div') };
+
+  browserScrollbarSizeMock.mockImplementationOnce(() => ({ height: 20, width: 100 }));
+  const { container } = render(<StickyScrollbar tableRef={tableRef} wrapperRef={wrapperRef} hasStickyColumns={true} />);
+  expect(container.querySelector(`.${styles['sticky-scrollbar-offset']}`)).not.toBeNull();
+});
+
 it('does not add an offset if the scrollbar occupies space', () => {
   const tableRef = { current: document.createElement('table') };
   const wrapperRef = { current: document.createElement('div') };
diff --git a/src/table/sticky-scrollbar/sticky-scrollbar.tsx b/src/table/sticky-scrollbar/sticky-scrollbar.tsx
index e9cffef945..3696bd9b34 100644
--- a/src/table/sticky-scrollbar/sticky-scrollbar.tsx
+++ b/src/table/sticky-scrollbar/sticky-scrollbar.tsx
@@ -13,11 +13,15 @@ interface StickyScrollbarProps {
   wrapperRef: React.RefObject<HTMLDivElement>;
   tableRef: React.RefObject<HTMLTableElement>;
   onScroll?: React.UIEventHandler<HTMLDivElement>;
+  hasStickyColumns?: boolean;
 }
 
 export default forwardRef(StickyScrollbar);
 
-function StickyScrollbar({ wrapperRef, tableRef, onScroll }: StickyScrollbarProps, ref: React.Ref<HTMLDivElement>) {
+function StickyScrollbar(
+  { wrapperRef, tableRef, onScroll, hasStickyColumns }: StickyScrollbarProps,
+  ref: React.Ref<HTMLDivElement>
+) {
   const isVisualRefresh = useVisualRefresh();
   const scrollbarRef = React.useRef<HTMLDivElement>(null);
   const scrollbarContentRef = React.useRef<HTMLDivElement>(null);
@@ -27,7 +31,7 @@ function StickyScrollbar({ wrapperRef, tableRef, onScroll }: StickyScrollbarProp
    * If the height of the scrollbar is 0, we're likely on a platform that uses
    * overlay scrollbars (e.g. Mac).
    */
-  const offsetScrollbar = browserScrollbarSize().height === 0;
+  const offsetScrollbar = hasStickyColumns || browserScrollbarSize().height === 0;
 
   /**
    * Use the appropriate AppLayout context (Classic or Visual Refresh) to determine

From 3507cce7ddf60dcfdc96f8e91ca0916c917827c4 Mon Sep 17 00:00:00 2001
From: Katie George <katiegeorge94@gmail.com>
Date: Sun, 27 Aug 2023 22:06:03 -0700
Subject: [PATCH 4/8] fix: Adds box shadow back to trigger buttons in drawers
 (#1477)

Co-authored-by: Katie George <katiegeo@amazon.com>
---
 src/app-layout/visual-refresh/drawers.tsx     | 29 +++----
 .../visual-refresh/trigger-button.scss        | 77 +++++++++++++------
 .../visual-refresh/trigger-button.tsx         |  4 +-
 3 files changed, 66 insertions(+), 44 deletions(-)

diff --git a/src/app-layout/visual-refresh/drawers.tsx b/src/app-layout/visual-refresh/drawers.tsx
index 72f297443b..abb4e42021 100644
--- a/src/app-layout/visual-refresh/drawers.tsx
+++ b/src/app-layout/visual-refresh/drawers.tsx
@@ -251,25 +251,16 @@ function DesktopTriggers() {
           <OverflowMenu
             items={overflowItems}
             ariaLabel={drawersOverflowAriaLabel}
-            customTriggerBuilder={({ onClick, triggerRef, ariaLabel, testUtilsClass, ariaExpanded }) => (
-              <div className={clsx(styles['trigger-wrapper'])}>
-                <TriggerButton
-                  ref={triggerRef}
-                  ariaLabel={ariaLabel}
-                  ariaExpanded={ariaExpanded}
-                  className={clsx(
-                    styles['drawers-trigger'],
-                    {
-                      [styles.badge]: overflowMenuHasBadge,
-                    },
-                    testutilStyles['drawers-trigger'],
-                    testUtilsClass
-                  )}
-                  iconName="ellipsis"
-                  onClick={onClick}
-                />
-                {overflowMenuHasBadge && <div className={clsx(styles.dot)} />}
-              </div>
+            customTriggerBuilder={({ onClick, triggerRef, ariaLabel, ariaExpanded, testUtilsClass }) => (
+              <TriggerButton
+                ref={triggerRef}
+                ariaLabel={ariaLabel}
+                ariaExpanded={ariaExpanded}
+                badge={overflowMenuHasBadge}
+                className={clsx(styles['drawers-trigger'], testutilStyles['drawers-trigger'], testUtilsClass)}
+                iconName="ellipsis"
+                onClick={onClick}
+              />
             )}
             onItemClick={({ detail }) => {
               handleDrawersClick(detail.id);
diff --git a/src/app-layout/visual-refresh/trigger-button.scss b/src/app-layout/visual-refresh/trigger-button.scss
index 577dd6b1ed..27fb76087d 100644
--- a/src/app-layout/visual-refresh/trigger-button.scss
+++ b/src/app-layout/visual-refresh/trigger-button.scss
@@ -7,22 +7,61 @@
 @use '../../internal/styles/tokens' as awsui;
 @use '../../internal/hooks/focus-visible' as focus-visible;
 
+@mixin trigger-selected-styles {
+  background: awsui.$color-background-layout-toggle-selected-default;
+  color: awsui.$color-text-layout-toggle-selected;
+
+  &:hover {
+    background: awsui.$color-background-layout-toggle-selected-hover;
+  }
+  &:active {
+    background: awsui.$color-background-layout-toggle-selected-active;
+  }
+}
+
+@mixin trigger-button-styles {
+  background: awsui.$color-background-layout-toggle-default;
+  border-radius: 50%;
+  height: awsui.$space-layout-toggle-diameter;
+  width: awsui.$space-layout-toggle-diameter;
+
+  &:hover {
+    background: awsui.$color-background-layout-toggle-hover;
+  }
+
+  &:active {
+    background: awsui.$color-background-layout-toggle-active;
+  }
+}
+
+.trigger-badge-wrapper {
+  @include trigger-button-styles();
+
+  position: absolute;
+  top: 0;
+  left: 0;
+  display: flex;
+  justify-content: center;
+  align-items: center;
+
+  clip-path: path(
+    'M38.0215 11.3161C37.1068 11.7545 36.0821 12 35 12C31.134 12 28 8.86599 28 5C28 3.91794 28.2455 2.89322 28.6839 1.97851C26.0575 0.710652 23.1118 0 20 0C8.9543 0 0 8.9543 0 20C0 31.0457 8.9543 40 20 40C31.0457 40 40 31.0457 40 20C40 16.8882 39.2893 13.9425 38.0215 11.3161Z'
+  );
+}
+
 /*
 Warning! If these design tokens for width change it will adversely impact 
 the calculation used to determine the Split Panel maximum width in the 
 handleSplitPanelMaxWidth function in the context.
 */
 .trigger {
-  background: awsui.$color-background-layout-toggle-default;
+  @include trigger-button-styles();
+
   border: none;
-  border-radius: 50%;
-  box-shadow: awsui.$shadow-panel-toggle;
   color: awsui.$color-text-layout-toggle;
   cursor: pointer;
-  height: awsui.$space-layout-toggle-diameter;
   padding: 0 awsui.$space-s;
   pointer-events: auto;
-  width: awsui.$space-layout-toggle-diameter;
   position: relative;
 
   @include focus-visible.when-visible {
@@ -33,35 +72,25 @@ handleSplitPanelMaxWidth function in the context.
     outline: none;
   }
 
-  &:hover {
-    background: awsui.$color-background-layout-toggle-hover;
-  }
-
-  &:active {
-    background: awsui.$color-background-layout-toggle-active;
-  }
-
   &.selected {
-    background: awsui.$color-background-layout-toggle-selected-default;
-    color: awsui.$color-text-layout-toggle-selected;
+    @include trigger-selected-styles();
 
-    &:hover {
-      background: awsui.$color-background-layout-toggle-selected-hover;
-    }
-    &:active {
-      background: awsui.$color-background-layout-toggle-selected-active;
+    > .trigger-badge-wrapper {
+      @include trigger-selected-styles();
     }
   }
 
-  &.badge {
-    clip-path: path(
-      'M38.0215 11.3161C37.1068 11.7545 36.0821 12 35 12C31.134 12 28 8.86599 28 5C28 3.91794 28.2455 2.89322 28.6839 1.97851C26.0575 0.710652 23.1118 0 20 0C8.9543 0 0 8.9543 0 20C0 31.0457 8.9543 40 20 40C31.0457 40 40 31.0457 40 20C40 16.8882 39.2893 13.9425 38.0215 11.3161Z'
-    );
+  &.badge,
+  &.badge:hover,
+  &.badge:active {
+    background: transparent;
   }
 }
 
 .trigger-wrapper {
   position: relative;
+  box-shadow: awsui.$shadow-panel-toggle;
+  border-radius: 50%;
 }
 
 .dot {
diff --git a/src/app-layout/visual-refresh/trigger-button.tsx b/src/app-layout/visual-refresh/trigger-button.tsx
index e575a83014..be6d0848bc 100644
--- a/src/app-layout/visual-refresh/trigger-button.tsx
+++ b/src/app-layout/visual-refresh/trigger-button.tsx
@@ -55,7 +55,9 @@ function TriggerButton(
         type="button"
         data-testid={testId}
       >
-        <Icon name={iconName} svg={iconSvg} />
+        <span className={clsx(badge && styles['trigger-badge-wrapper'])}>
+          <Icon name={iconName} svg={iconSvg} />
+        </span>
       </button>
       {badge && <div className={clsx(styles.dot)} />}
     </div>

From 334a015ccba651d71488b0b06b0856ad70427f48 Mon Sep 17 00:00:00 2001
From: Johannes Weber <jowejowe@amazon.com>
Date: Tue, 22 Aug 2023 23:19:01 +0200
Subject: [PATCH 5/8] fix: Click outside focus handling on table inline edit

When editing a table cell and clicking somewhere outside, the edit mode ended and the edited cell kept focus by purpose. Now, if a focusable element got clicked outside, it will receive the focus - the focus handling does not get intercepted anymore.
---
 src/table/__integ__/inline-editing.test.ts | 13 +++++++++++++
 src/table/body-cell/index.tsx              |  6 +++---
 src/table/body-cell/inline-editor.tsx      | 21 +++++++++++++--------
 3 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/src/table/__integ__/inline-editing.test.ts b/src/table/__integ__/inline-editing.test.ts
index aa4270d912..04b42a23a3 100644
--- a/src/table/__integ__/inline-editing.test.ts
+++ b/src/table/__integ__/inline-editing.test.ts
@@ -137,3 +137,16 @@ test(
     await page.waitForAssertion(() => expect(page.isFocused(cellInputField$)).resolves.toBe(true));
   })
 );
+
+test(
+  'click focusable element outside when editing cancels editing and focuses clicked element',
+  setupTest(async page => {
+    // Edit a cell
+    await page.click(cellEditButton$);
+    await expect(page.isFocused(cellInputField$)).resolves.toBe(true);
+
+    // Click on the input element outside, it should get focused.
+    await page.click('[data-testid="focus"]');
+    await expect(page.isFocused('[data-testid="focus"]')).resolves.toBe(true);
+  })
+);
diff --git a/src/table/body-cell/index.tsx b/src/table/body-cell/index.tsx
index 981dcbf55b..ca11eea2a9 100644
--- a/src/table/body-cell/index.tsx
+++ b/src/table/body-cell/index.tsx
@@ -92,10 +92,10 @@ function TableCellEditable<ItemType>({
           ariaLabels={ariaLabels}
           column={column}
           item={item}
-          onEditEnd={e => {
+          onEditEnd={options => {
             setShowSuccessIcon(false);
-            isFocusMoveNeededRef.current = true;
-            onEditEnd(e);
+            isFocusMoveNeededRef.current = options.refocusCell;
+            onEditEnd(options.cancelled);
           }}
           submitEdit={submitEdit ?? submitHandlerFallback}
         />
diff --git a/src/table/body-cell/inline-editor.tsx b/src/table/body-cell/inline-editor.tsx
index 32092f54a2..4acdb462f7 100644
--- a/src/table/body-cell/inline-editor.tsx
+++ b/src/table/body-cell/inline-editor.tsx
@@ -15,11 +15,16 @@ import { useInternalI18n } from '../../i18n/context';
 // A function that does nothing
 const noop = () => undefined;
 
+interface OnEditEndOptions {
+  cancelled: boolean;
+  refocusCell: boolean;
+}
+
 interface InlineEditorProps<ItemType> {
   ariaLabels: TableProps['ariaLabels'];
   column: TableProps.ColumnDefinition<ItemType>;
   item: ItemType;
-  onEditEnd: (cancelled: boolean) => void;
+  onEditEnd: (options: OnEditEndOptions) => void;
   submitEdit: TableProps.SubmitEditFunction<ItemType>;
   __onRender?: () => void;
 }
@@ -43,11 +48,11 @@ export function InlineEditor<ItemType>({
     setValue: setCurrentEditValue,
   };
 
-  function finishEdit(cancel = false) {
-    if (!cancel) {
+  function finishEdit({ cancelled = false, refocusCell = true }: Partial<OnEditEndOptions> = {}) {
+    if (!cancelled) {
       setCurrentEditValue(undefined);
     }
-    onEditEnd(cancel);
+    onEditEnd({ cancelled, refocusCell: refocusCell });
   }
 
   async function onSubmitClick(evt: React.FormEvent) {
@@ -68,11 +73,11 @@ export function InlineEditor<ItemType>({
     }
   }
 
-  function onCancel() {
+  function onCancel({ reFocusEditedCell = true } = {}) {
     if (currentEditLoading) {
       return;
     }
-    finishEdit(true);
+    finishEdit({ cancelled: true, refocusCell: reFocusEditedCell });
   }
 
   function handleEscape(event: React.KeyboardEvent): void {
@@ -81,7 +86,7 @@ export function InlineEditor<ItemType>({
     }
   }
 
-  const clickAwayRef = useClickAway(onCancel);
+  const clickAwayRef = useClickAway(() => onCancel({ reFocusEditedCell: false }));
 
   useEffect(() => {
     if (__onRender) {
@@ -127,7 +132,7 @@ export function InlineEditor<ItemType>({
                       formAction="none"
                       iconName="close"
                       variant="inline-icon"
-                      onClick={onCancel}
+                      onClick={() => onCancel()}
                     />
                   ) : null}
                   <Button

From 37db07542afb3188dac95e16798b4380178dd6a5 Mon Sep 17 00:00:00 2001
From: Andrei Zhaleznichenka <zhalea@amazon.de>
Date: Mon, 28 Aug 2023 11:20:21 +0200
Subject: [PATCH 6/8] refactor: Introduce table th element (#1486)

---
 src/table/__tests__/header-cell.test.tsx |  7 +--
 src/table/header-cell/index.tsx          | 61 +++++++-------------
 src/table/header-cell/th-element.tsx     | 71 ++++++++++++++++++++++++
 src/table/header-cell/utils.ts           |  2 +-
 src/table/internal.tsx                   |  2 +-
 src/table/resizer/index.tsx              |  4 +-
 src/table/selection-control/index.tsx    | 14 ++---
 src/table/sticky-header.tsx              |  6 +-
 src/table/thead.tsx                      | 58 +++++++++----------
 9 files changed, 133 insertions(+), 92 deletions(-)
 create mode 100644 src/table/header-cell/th-element.tsx

diff --git a/src/table/__tests__/header-cell.test.tsx b/src/table/__tests__/header-cell.test.tsx
index 9ab9caa0da..949bbf39b0 100644
--- a/src/table/__tests__/header-cell.test.tsx
+++ b/src/table/__tests__/header-cell.test.tsx
@@ -49,11 +49,10 @@ it('renders a fake focus outline on the sort control', () => {
   const { container } = render(
     <TableWrapper>
       <TableHeaderCell<typeof testItem>
-        focusedComponent={{ type: 'column', col: 0 }}
+        focusedComponent="sorting-control-id"
         column={column}
         colIndex={0}
         tabIndex={0}
-        onFocusedComponentChange={() => {}}
         updateColumn={() => {}}
         onClick={() => {}}
         onResizeFinish={() => {}}
@@ -73,12 +72,11 @@ it('renders a fake focus outline on the resize control', () => {
   const { container } = render(
     <TableWrapper>
       <TableHeaderCell<typeof testItem>
-        focusedComponent={{ type: 'resizer', col: 0 }}
+        focusedComponent="resize-control-id"
         column={column}
         colIndex={0}
         tabIndex={0}
         resizableColumns={true}
-        onFocusedComponentChange={() => {}}
         updateColumn={() => {}}
         onClick={() => {}}
         onResizeFinish={() => {}}
@@ -104,7 +102,6 @@ describe('i18n', () => {
             colIndex={0}
             tabIndex={0}
             resizableColumns={true}
-            onFocusedComponentChange={() => {}}
             updateColumn={() => {}}
             onClick={() => {}}
             onResizeFinish={() => {}}
diff --git a/src/table/header-cell/index.tsx b/src/table/header-cell/index.tsx
index e9d3f8bea0..ce45566126 100644
--- a/src/table/header-cell/index.tsx
+++ b/src/table/header-cell/index.tsx
@@ -9,12 +9,10 @@ import { getSortingIconName, getSortingStatus, isSorted } from './utils';
 import styles from './styles.css.js';
 import { Resizer } from '../resizer';
 import { useUniqueId } from '../../internal/hooks/use-unique-id';
-import { InteractiveComponent } from '../thead';
-import { getStickyClassNames } from '../utils';
 import { useInternalI18n } from '../../i18n/context';
-import { StickyColumnsModel, useStickyCellStyles } from '../sticky-columns';
-import { useMergeRefs } from '../../internal/hooks/use-merge-refs';
-import { TableRole, getTableColHeaderRoleProps } from '../table-role';
+import { StickyColumnsModel } from '../sticky-columns';
+import { TableRole } from '../table-role';
+import { TableThElement } from './th-element';
 
 interface TableHeaderCellProps<ItemType> {
   className?: string;
@@ -37,8 +35,7 @@ interface TableHeaderCellProps<ItemType> {
   columnId: PropertyKey;
   stickyState: StickyColumnsModel;
   cellRef: React.RefCallback<HTMLElement>;
-  focusedComponent?: InteractiveComponent | null;
-  onFocusedComponentChange?: (element: InteractiveComponent | null) => void;
+  focusedComponent?: null | string;
   tableRole: TableRole;
 }
 
@@ -52,7 +49,6 @@ export function TableHeaderCell<ItemType>({
   sortingDisabled,
   wrapLines,
   focusedComponent,
-  onFocusedComponentChange,
   hidden,
   onClick,
   colIndex,
@@ -88,36 +84,24 @@ export function TableHeaderCell<ItemType>({
 
   const headerId = useUniqueId('table-header-');
 
-  const stickyStyles = useStickyCellStyles({
-    stickyColumns: stickyState,
-    columnId,
-    getClassName: props => getStickyClassNames(styles, props),
-  });
-
-  const mergedRef = useMergeRefs(stickyStyles.ref, cellRef);
-
   return (
-    <th
-      className={clsx(
-        className,
-        {
-          [styles['header-cell-resizable']]: !!resizableColumns,
-          [styles['header-cell-sortable']]: sortingStatus,
-          [styles['header-cell-sorted']]: sortingStatus === 'ascending' || sortingStatus === 'descending',
-          [styles['header-cell-disabled']]: sortingDisabled,
-          [styles['header-cell-ascending']]: sortingStatus === 'ascending',
-          [styles['header-cell-descending']]: sortingStatus === 'descending',
-          [styles['header-cell-hidden']]: hidden,
-        },
-        stickyStyles.className
-      )}
-      style={{ ...style, ...stickyStyles.style }}
-      ref={mergedRef}
-      {...getTableColHeaderRoleProps({ tableRole, sortingStatus, colIndex })}
+    <TableThElement
+      className={className}
+      style={style}
+      cellRef={cellRef}
+      sortingStatus={sortingStatus}
+      sortingDisabled={sortingDisabled}
+      hidden={hidden}
+      colIndex={colIndex}
+      resizableColumns={resizableColumns}
+      columnId={columnId}
+      stickyState={stickyState}
+      tableRole={tableRole}
     >
       <div
+        data-focus-id={`sorting-control-${String(columnId)}`}
         className={clsx(styles['header-cell-content'], {
-          [styles['header-cell-fake-focus']]: focusedComponent?.type === 'column' && focusedComponent.col === colIndex,
+          [styles['header-cell-fake-focus']]: focusedComponent === `sorting-control-${String(columnId)}`,
         })}
         aria-label={
           column.ariaLabel
@@ -134,8 +118,6 @@ export function TableHeaderCell<ItemType>({
               tabIndex: tabIndex,
               role: 'button',
               onClick: handleClick,
-              onFocus: () => onFocusedComponentChange?.({ type: 'column', col: colIndex }),
-              onBlur: () => onFocusedComponentChange?.(null),
             }
           : {})}
       >
@@ -161,16 +143,15 @@ export function TableHeaderCell<ItemType>({
         <>
           <Resizer
             tabIndex={tabIndex}
-            showFocusRing={focusedComponent?.type === 'resizer' && focusedComponent.col === colIndex}
+            focusId={`resize-control-${String(columnId)}`}
+            showFocusRing={focusedComponent === `resize-control-${String(columnId)}`}
             onDragMove={newWidth => updateColumn(columnId, newWidth)}
             onFinish={onResizeFinish}
             ariaLabelledby={headerId}
-            onFocus={() => onFocusedComponentChange?.({ type: 'resizer', col: colIndex })}
-            onBlur={() => onFocusedComponentChange?.(null)}
             minWidth={typeof column.minWidth === 'string' ? parseInt(column.minWidth) : column.minWidth}
           />
         </>
       )}
-    </th>
+    </TableThElement>
   );
 }
diff --git a/src/table/header-cell/th-element.tsx b/src/table/header-cell/th-element.tsx
new file mode 100644
index 0000000000..a7264f842e
--- /dev/null
+++ b/src/table/header-cell/th-element.tsx
@@ -0,0 +1,71 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+import clsx from 'clsx';
+import React from 'react';
+import { SortingStatus } from './utils';
+import styles from './styles.css.js';
+import { getStickyClassNames } from '../utils';
+import { StickyColumnsModel, useStickyCellStyles } from '../sticky-columns';
+import { useMergeRefs } from '../../internal/hooks/use-merge-refs';
+import { TableRole, getTableColHeaderRoleProps } from '../table-role';
+
+interface TableThElementProps {
+  className?: string;
+  style?: React.CSSProperties;
+  sortingStatus?: SortingStatus;
+  sortingDisabled?: boolean;
+  hidden?: boolean;
+  colIndex: number;
+  resizableColumns?: boolean;
+  columnId: PropertyKey;
+  stickyState: StickyColumnsModel;
+  cellRef?: React.RefCallback<HTMLElement>;
+  tableRole: TableRole;
+  children: React.ReactNode;
+}
+
+export function TableThElement({
+  className,
+  style,
+  sortingStatus,
+  sortingDisabled,
+  hidden,
+  colIndex,
+  resizableColumns,
+  columnId,
+  stickyState,
+  cellRef,
+  tableRole,
+  children,
+}: TableThElementProps) {
+  const stickyStyles = useStickyCellStyles({
+    stickyColumns: stickyState,
+    columnId,
+    getClassName: props => getStickyClassNames(styles, props),
+  });
+
+  const mergedRef = useMergeRefs(stickyStyles.ref, cellRef);
+
+  return (
+    <th
+      className={clsx(
+        className,
+        {
+          [styles['header-cell-resizable']]: !!resizableColumns,
+          [styles['header-cell-sortable']]: sortingStatus,
+          [styles['header-cell-sorted']]: sortingStatus === 'ascending' || sortingStatus === 'descending',
+          [styles['header-cell-disabled']]: sortingDisabled,
+          [styles['header-cell-ascending']]: sortingStatus === 'ascending',
+          [styles['header-cell-descending']]: sortingStatus === 'descending',
+          [styles['header-cell-hidden']]: hidden,
+        },
+        stickyStyles.className
+      )}
+      style={{ ...style, ...stickyStyles.style }}
+      ref={mergedRef}
+      {...getTableColHeaderRoleProps({ tableRole, sortingStatus, colIndex })}
+    >
+      {children}
+    </th>
+  );
+}
diff --git a/src/table/header-cell/utils.ts b/src/table/header-cell/utils.ts
index 3fe61b5096..9df9fa78b3 100644
--- a/src/table/header-cell/utils.ts
+++ b/src/table/header-cell/utils.ts
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 import { TableProps } from '../interfaces';
 
-type SortingStatus = 'sortable' | 'ascending' | 'descending';
+export type SortingStatus = 'sortable' | 'ascending' | 'descending';
 const stateToIcon = {
   sortable: 'caret-down',
   ascending: 'caret-up-filled',
diff --git a/src/table/internal.tsx b/src/table/internal.tsx
index 6539fa8d92..c0f83876bf 100644
--- a/src/table/internal.tsx
+++ b/src/table/internal.tsx
@@ -340,7 +340,7 @@ const InternalTable = React.forwardRef(
                 <Thead
                   ref={theadRef}
                   hidden={stickyHeader}
-                  onFocusedComponentChange={component => stickyHeaderRef.current?.setFocus(component)}
+                  onFocusedComponentChange={focusId => stickyHeaderRef.current?.setFocus(focusId)}
                   {...theadProps}
                 />
                 <tbody>
diff --git a/src/table/resizer/index.tsx b/src/table/resizer/index.tsx
index b77c52e5c7..2d9233399a 100644
--- a/src/table/resizer/index.tsx
+++ b/src/table/resizer/index.tsx
@@ -16,7 +16,7 @@ interface ResizerProps {
   ariaLabelledby?: string;
   minWidth?: number;
   tabIndex?: number;
-
+  focusId?: string;
   showFocusRing?: boolean;
   onFocus?: () => void;
   onBlur?: () => void;
@@ -33,6 +33,7 @@ export function Resizer({
   minWidth = DEFAULT_COLUMN_WIDTH,
   tabIndex,
   showFocusRing,
+  focusId,
   onFocus,
   onBlur,
 }: ResizerProps) {
@@ -171,6 +172,7 @@ export function Resizer({
       aria-valuetext={headerCellWidth.toString()}
       aria-valuemin={minWidth}
       tabIndex={tabIndex}
+      data-focus-id={focusId}
     />
   );
 }
diff --git a/src/table/selection-control/index.tsx b/src/table/selection-control/index.tsx
index 80ed6e3d7d..6041beb78d 100644
--- a/src/table/selection-control/index.tsx
+++ b/src/table/selection-control/index.tsx
@@ -8,7 +8,6 @@ import InternalCheckbox from '../../checkbox/internal';
 import RadioButton from '../../radio-group/radio-button';
 
 import styles from './styles.css.js';
-import { InteractiveComponent } from '../thead';
 import { SelectionProps } from '../use-selection';
 
 export interface SelectionControlProps extends SelectionProps {
@@ -17,9 +16,7 @@ export interface SelectionControlProps extends SelectionProps {
   onFocusDown?: KeyboardEventHandler;
   ariaLabel?: string;
   tabIndex?: -1;
-
-  focusedComponent?: InteractiveComponent | null;
-  onFocusedComponentChange?: (element: InteractiveComponent | null) => void;
+  focusedComponent?: null | string;
 }
 
 export default function SelectionControl({
@@ -30,9 +27,7 @@ export default function SelectionControl({
   onFocusDown,
   name,
   ariaLabel,
-
   focusedComponent,
-  onFocusedComponentChange,
   ...sharedProps
 }: SelectionControlProps) {
   const controlId = useUniqueId();
@@ -79,10 +74,9 @@ export default function SelectionControl({
   const selector = isMultiSelection ? (
     <InternalCheckbox
       {...sharedProps}
-      showOutline={focusedComponent?.type === 'selection'}
-      onFocus={() => onFocusedComponentChange?.({ type: 'selection' })}
-      onBlur={() => onFocusedComponentChange?.(null)}
+      showOutline={focusedComponent === 'selection-control'}
       controlId={controlId}
+      data-focus-id="selection-control"
       indeterminate={indeterminate}
     />
   ) : (
@@ -104,7 +98,7 @@ export default function SelectionControl({
       >
         {selector}
       </label>
-      {/* HACK: IE11 collapses td's height to 0, if it contains only an absouletely positioned label */}
+      {/* HACK: IE11 collapses td's height to 0, if it contains only an absolutely positioned label */}
       <span className={clsx(styles.stud)} aria-hidden={true}>
         &nbsp;
       </span>
diff --git a/src/table/sticky-header.tsx b/src/table/sticky-header.tsx
index 34d47e6272..e931370ce5 100644
--- a/src/table/sticky-header.tsx
+++ b/src/table/sticky-header.tsx
@@ -4,7 +4,7 @@ import clsx from 'clsx';
 import React, { forwardRef, useContext, useImperativeHandle, useRef, useState } from 'react';
 import { StickyHeaderContext } from '../container/use-sticky-header';
 import { TableProps } from './interfaces';
-import Thead, { InteractiveComponent, TheadProps } from './thead';
+import Thead, { TheadProps } from './thead';
 import { useStickyHeader } from './use-sticky-header';
 import styles from './styles.css.js';
 import { getVisualContextClassname } from '../internal/components/visual-context';
@@ -13,7 +13,7 @@ import { TableRole, getTableRoleProps } from './table-role';
 export interface StickyHeaderRef {
   scrollToTop(): void;
   scrollToRow(node: null | HTMLElement): void;
-  setFocus(element: InteractiveComponent | null): void;
+  setFocus(focusId: null | string): void;
 }
 
 interface StickyHeaderProps {
@@ -50,7 +50,7 @@ function StickyHeader(
   const secondaryTableRef = useRef<HTMLTableElement>(null);
   const { isStuck } = useContext(StickyHeaderContext);
 
-  const [focusedComponent, setFocusedComponent] = useState<InteractiveComponent | null>(null);
+  const [focusedComponent, setFocusedComponent] = useState<null | string>(null);
   const { scrollToRow, scrollToTop } = useStickyHeader(
     tableRef,
     theadRef,
diff --git a/src/table/thead.tsx b/src/table/thead.tsx
index 41fba63349..4be7f2c1ae 100644
--- a/src/table/thead.tsx
+++ b/src/table/thead.tsx
@@ -6,21 +6,17 @@ import { TableProps } from './interfaces';
 import SelectionControl from './selection-control';
 import { focusMarkers, SelectionProps } from './use-selection';
 import { fireNonCancelableEvent, NonCancelableEventHandler } from '../internal/events';
-import { getColumnKey, getStickyClassNames } from './utils';
+import { getColumnKey } from './utils';
 import { TableHeaderCell } from './header-cell';
 import { useColumnWidths } from './use-column-widths';
 import { useVisualRefresh } from '../internal/hooks/use-visual-mode';
 import styles from './styles.css.js';
-import cellStyles from './header-cell/styles.css.js';
 import headerCellStyles from './header-cell/styles.css.js';
 import ScreenreaderOnly from '../internal/components/screenreader-only';
-import { StickyColumnsModel, useStickyCellStyles } from './sticky-columns';
-import { getTableColHeaderRoleProps, getTableHeaderRowRoleProps, TableRole } from './table-role';
-
-export type InteractiveComponent =
-  | { type: 'selection' }
-  | { type: 'column'; col: number }
-  | { type: 'resizer'; col: number };
+import { StickyColumnsModel } from './sticky-columns';
+import { getTableHeaderRowRoleProps, TableRole } from './table-role';
+import { TableThElement } from './header-cell/th-element';
+import { findUpUntil } from '@cloudscape-design/component-toolkit/dom';
 
 export interface TheadProps {
   containerWidth: number | null;
@@ -43,8 +39,8 @@ export interface TheadProps {
   stripedRows?: boolean;
   stickyState: StickyColumnsModel;
   selectionColumnId: PropertyKey;
-  focusedComponent?: InteractiveComponent | null;
-  onFocusedComponentChange?: (element: InteractiveComponent | null) => void;
+  focusedComponent?: null | string;
+  onFocusedComponentChange?: (focusId: null | string) => void;
   tableRole: TableRole;
 }
 
@@ -96,26 +92,28 @@ const Thead = React.forwardRef(
 
     const { columnWidths, totalWidth, updateColumn, setCell } = useColumnWidths();
 
-    const stickyStyles = useStickyCellStyles({
-      stickyColumns: stickyState,
-      columnId: selectionColumnId,
-      getClassName: props => getStickyClassNames(cellStyles, props),
-    });
     return (
       <thead className={clsx(!hidden && styles['thead-active'])}>
-        <tr {...focusMarkers.all} ref={outerRef} aria-rowindex={1} {...getTableHeaderRowRoleProps({ tableRole })}>
+        <tr
+          {...focusMarkers.all}
+          ref={outerRef}
+          aria-rowindex={1}
+          {...getTableHeaderRowRoleProps({ tableRole })}
+          onFocus={event => {
+            const focusControlElement = findUpUntil(event.target, element => !!element.getAttribute('data-focus-id'));
+            const focusId = focusControlElement?.getAttribute('data-focus-id') ?? null;
+            onFocusedComponentChange?.(focusId);
+          }}
+          onBlur={() => onFocusedComponentChange?.(null)}
+        >
           {selectionType ? (
-            <th
-              className={clsx(
-                headerCellClass,
-                selectionCellClass,
-                hidden && headerCellStyles['header-cell-hidden'],
-                stickyStyles.className
-              )}
-              style={stickyStyles.style}
-              ref={stickyStyles.ref}
-              scope="col"
-              {...getTableColHeaderRoleProps({ tableRole, colIndex: 0 })}
+            <TableThElement
+              className={clsx(headerCellClass, selectionCellClass, hidden && headerCellStyles['header-cell-hidden'])}
+              hidden={hidden}
+              tableRole={tableRole}
+              colIndex={0}
+              columnId={selectionColumnId}
+              stickyState={stickyState}
             >
               {selectionType === 'multi' ? (
                 <SelectionControl
@@ -123,14 +121,13 @@ const Thead = React.forwardRef(
                     onFocusMove!(event.target as HTMLElement, -1, +1);
                   }}
                   focusedComponent={focusedComponent}
-                  onFocusedComponentChange={onFocusedComponentChange}
                   {...getSelectAllProps()}
                   {...(sticky ? { tabIndex: -1 } : {})}
                 />
               ) : (
                 <ScreenreaderOnly>{singleSelectionHeaderAriaLabel}</ScreenreaderOnly>
               )}
-            </th>
+            </TableThElement>
           ) : null}
 
           {columnDefinitions.map((column, colIndex) => {
@@ -158,7 +155,6 @@ const Thead = React.forwardRef(
                 }}
                 tabIndex={sticky ? -1 : 0}
                 focusedComponent={focusedComponent}
-                onFocusedComponentChange={onFocusedComponentChange}
                 column={column}
                 activeSortingColumn={sortingColumn}
                 sortingDescending={sortingDescending}

From 9cd41aa0deab4c0759c613da1dbe8c9791ce2804 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=BAben=20Carvalho?= <rucarva@amazon.de>
Date: Mon, 28 Aug 2023 14:03:04 +0200
Subject: [PATCH 7/8] fix: Refactor and fix AppLayout and side navigation panel
 state and behaviour (#1490)

---
 pages/side-navigation/app-layout.page.tsx     |  11 +-
 src/app-layout/__tests__/common.test.tsx      | 344 +++++++++---------
 src/app-layout/__tests__/mobile.test.tsx      | 128 ++++++-
 src/app-layout/__tests__/utils.tsx            |   6 +-
 src/app-layout/index.tsx                      |  20 +-
 src/app-layout/visual-refresh/context.tsx     |  22 +-
 .../expandable-section-header.tsx             |   3 +-
 src/side-navigation/internal.tsx              |   3 -
 8 files changed, 324 insertions(+), 213 deletions(-)

diff --git a/pages/side-navigation/app-layout.page.tsx b/pages/side-navigation/app-layout.page.tsx
index e202c6e7f8..cc20c50f7f 100644
--- a/pages/side-navigation/app-layout.page.tsx
+++ b/pages/side-navigation/app-layout.page.tsx
@@ -4,6 +4,7 @@ import React from 'react';
 import SideNavigation, { SideNavigationProps } from '~components/side-navigation';
 import AppLayout from '~components/app-layout';
 import Badge from '~components/badge';
+import labels from '../app-layout/utils/labels';
 
 import logoSmall from './logos/logo-small.svg';
 
@@ -101,12 +102,16 @@ const items: SideNavigationProps.Item[] = [
 ];
 
 export default function SideNavigationPage() {
+  const [open, setOpen] = React.useState(true);
+
   return (
     <AppLayout
-      toolsHide={true}
-      navigationOpen={true}
+      navigationOpen={open}
+      onNavigationChange={({ detail }) => {
+        setOpen(detail.open);
+      }}
       contentType="form"
-      ariaLabels={{ navigationClose: 'Close' }}
+      ariaLabels={labels}
       navigation={
         <SideNavigation
           activeHref="#/"
diff --git a/src/app-layout/__tests__/common.test.tsx b/src/app-layout/__tests__/common.test.tsx
index 8b5710e8bf..eab7c76e36 100644
--- a/src/app-layout/__tests__/common.test.tsx
+++ b/src/app-layout/__tests__/common.test.tsx
@@ -18,7 +18,7 @@ jest.mock('@cloudscape-design/component-toolkit', () => ({
   useContainerQuery: () => [100, () => {}],
 }));
 
-describeEachAppLayout(() => {
+describeEachAppLayout(size => {
   test('Default state', () => {
     const { wrapper } = renderComponent(<AppLayout />);
 
@@ -50,6 +50,7 @@ describeEachAppLayout(() => {
       openProp: 'navigationOpen',
       hideProp: 'navigationHide',
       handler: 'onNavigationChange',
+      expectedCallsOnMobileToggle: 2,
       findLandmarks: (wrapper: AppLayoutWrapper) => wrapper.findAll('nav'),
       findElement: (wrapper: AppLayoutWrapper) => wrapper.findNavigation(),
       findToggle: (wrapper: AppLayoutWrapper) => wrapper.findNavigationToggle(),
@@ -59,176 +60,187 @@ describeEachAppLayout(() => {
       openProp: 'toolsOpen',
       hideProp: 'toolsHide',
       handler: 'onToolsChange',
+      expectedCallsOnMobileToggle: 1,
       findLandmarks: (wrapper: AppLayoutWrapper) => wrapper.findAll('aside'),
       findElement: (wrapper: AppLayoutWrapper) => wrapper.findTools(),
       findToggle: (wrapper: AppLayoutWrapper) => wrapper.findToolsToggle(),
       findClose: (wrapper: AppLayoutWrapper) => wrapper.findToolsClose(),
     },
-  ].forEach(({ openProp, hideProp, handler, findElement, findLandmarks, findToggle, findClose }) => {
-    describe(`${openProp} prop`, () => {
-      test(`Should call handler once on open when toggle is clicked`, () => {
-        const onToggle = jest.fn();
-        const props = {
-          [openProp]: false,
-          [handler]: onToggle,
-        };
-        const { wrapper } = renderComponent(<AppLayout {...props} />);
-
-        findToggle(wrapper).click();
-        expect(onToggle).toHaveBeenCalledTimes(1);
-        expect(onToggle).toHaveBeenCalledWith(expect.objectContaining({ detail: { open: true } }));
+  ].forEach(
+    ({
+      openProp,
+      hideProp,
+      handler,
+      expectedCallsOnMobileToggle,
+      findElement,
+      findLandmarks,
+      findToggle,
+      findClose,
+    }) => {
+      describe(`${openProp} prop`, () => {
+        test(`Should call handler once on open when toggle is clicked`, () => {
+          const onToggle = jest.fn();
+          const props = {
+            [openProp]: false,
+            [handler]: onToggle,
+          };
+          const { wrapper } = renderComponent(<AppLayout {...props} />);
+
+          findToggle(wrapper).click();
+          expect(onToggle).toHaveBeenCalledTimes(size === 'mobile' ? expectedCallsOnMobileToggle : 1);
+          expect(onToggle).toHaveBeenLastCalledWith(expect.objectContaining({ detail: { open: true } }));
+        });
+
+        test(`Should call handler once on open when span inside toggle is clicked`, () => {
+          const onToggle = jest.fn();
+          const props = {
+            [openProp]: false,
+            [handler]: onToggle,
+          };
+          const { wrapper } = renderComponent(<AppLayout {...props} />);
+
+          // Chrome bubbles up events from specific elements inside <button>s.
+          findToggle(wrapper).find('span')!.click();
+          expect(onToggle).toHaveBeenCalledTimes(size === 'mobile' ? expectedCallsOnMobileToggle : 1);
+          expect(onToggle).toHaveBeenLastCalledWith(expect.objectContaining({ detail: { open: true } }));
+        });
+
+        test(`Should call handler once on close`, () => {
+          const onToggle = jest.fn();
+          const props = {
+            [openProp]: true,
+            [handler]: onToggle,
+          };
+          const { wrapper } = renderComponent(<AppLayout {...props} />);
+
+          findClose(wrapper).click();
+          expect(onToggle).toHaveBeenCalledTimes(size === 'mobile' ? expectedCallsOnMobileToggle : 1);
+          expect(onToggle).toHaveBeenLastCalledWith(expect.objectContaining({ detail: { open: false } }));
+        });
+
+        test('Renders two landmarks in closed state', () => {
+          const props = {
+            [openProp]: false,
+            [handler]: () => {},
+          };
+          const { wrapper } = renderComponent(<AppLayout {...props} />);
+          const landmarks = findLandmarks(wrapper);
+          expect(landmarks).toHaveLength(2);
+
+          const toggleElement = findToggle(wrapper).getElement();
+
+          if (landmarks[0].getElement().contains(toggleElement)) {
+            expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'false');
+            expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'true');
+          } else {
+            expect(landmarks[1].getElement()).toContainElement(toggleElement);
+            expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'false');
+            expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'true');
+          }
+        });
+
+        test('Renders two landmarks in open state', () => {
+          const props = {
+            [openProp]: true,
+            [handler]: () => {},
+          };
+          const { wrapper } = renderComponent(<AppLayout {...props} />);
+          const landmarks = findLandmarks(wrapper);
+          expect(landmarks).toHaveLength(2);
+          const toggleElement = findToggle(wrapper).getElement();
+
+          if (landmarks[0].getElement().contains(toggleElement)) {
+            expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'true');
+            expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'false');
+          } else {
+            expect(landmarks[1].getElement()).toContainElement(toggleElement);
+            expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'true');
+            expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'false');
+          }
+        });
+
+        test('Renders aria-expanded only on toggle', () => {
+          const props = {
+            [openProp]: false,
+            [handler]: () => {},
+          };
+
+          const { wrapper } = renderComponent(<AppLayout {...props} />);
+          expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-expanded', 'false');
+          expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-haspopup', 'true');
+          expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-expanded');
+          expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-haspopup');
+        });
+
+        test('Does not add a label to the toggle and landmark when they are not defined', () => {
+          const { wrapper } = renderComponent(<AppLayout />);
+          expect(findToggle(wrapper).getElement()).not.toHaveAttribute('aria-label');
+          expect(findLandmarks(wrapper)[0].getElement()).not.toHaveAttribute('aria-label');
+        });
+
+        test('Adds labels to toggle button and landmark when defined', () => {
+          const labels = {
+            navigationToggle: 'toggle',
+            toolsToggle: 'toggle',
+            navigation: 'landmark',
+            tools: 'landmark',
+          };
+
+          const { wrapper } = renderComponent(<AppLayout ariaLabels={labels} />);
+          expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-label', 'toggle');
+          expect(findLandmarks(wrapper)[0].getElement()).toHaveAttribute('aria-label', 'landmark');
+        });
+
+        test('Close button does have a label if it is defined', () => {
+          const props = { [openProp]: true, [handler]: () => {} };
+          const labels = {
+            navigationClose: 'close label',
+            toolsClose: 'close label',
+          };
+          const { wrapper } = renderComponent(<AppLayout {...props} ariaLabels={labels} />);
+
+          expect(findClose(wrapper).getElement()).toHaveAttribute('aria-label', 'close label');
+        });
+
+        test('Close button does not render a label if is not defined', () => {
+          const props = { [openProp]: true, [handler]: () => {} };
+          const { wrapper } = renderComponent(<AppLayout {...props} />);
+
+          expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-label');
+        });
+
+        test('Opens and closes drawer in uncontrolled mode', () => {
+          // use content type with initial closed state for all drawers
+          const { wrapper } = renderComponent(<AppLayout contentType="form" />);
+          expect(isDrawerClosed(findElement(wrapper))).toBe(true);
+
+          act(() => findToggle(wrapper).click());
+          expect(isDrawerClosed(findElement(wrapper))).toBe(false);
+
+          act(() => findClose(wrapper).click());
+          expect(isDrawerClosed(findElement(wrapper))).toBe(true);
+        });
+
+        test('Moves focus between open and close buttons', () => {
+          // use content type with initial closed state for all drawers
+          const { wrapper } = renderComponent(<AppLayout contentType="form" />);
+
+          act(() => findToggle(wrapper).click());
+          expect(findClose(wrapper).getElement()).toBe(document.activeElement);
+
+          act(() => findClose(wrapper).click());
+          expect(findToggle(wrapper).getElement()).toBe(document.activeElement);
+        });
+
+        test(`Should not render the drawer if ${hideProp} is set to true`, () => {
+          const props = { [hideProp]: true };
+          const { wrapper } = renderComponent(<AppLayout {...props} />);
+          expect(findElement(wrapper)).toBeFalsy();
+          expect(findLandmarks(wrapper)).toHaveLength(0);
+        });
       });
-
-      test(`Should call handler once on open when span inside toggle is clicked`, () => {
-        const onToggle = jest.fn();
-        const props = {
-          [openProp]: false,
-          [handler]: onToggle,
-        };
-        const { wrapper } = renderComponent(<AppLayout {...props} />);
-
-        // Chrome bubbles up events from specific elements inside <button>s.
-        findToggle(wrapper).find('span')!.click();
-        expect(onToggle).toHaveBeenCalledTimes(1);
-        expect(onToggle).toHaveBeenCalledWith(expect.objectContaining({ detail: { open: true } }));
-      });
-
-      test(`Should call handler once on close`, () => {
-        const onToggle = jest.fn();
-        const props = {
-          [openProp]: true,
-          [handler]: onToggle,
-        };
-        const { wrapper } = renderComponent(<AppLayout {...props} />);
-
-        findClose(wrapper).click();
-        expect(onToggle).toHaveBeenCalledTimes(1);
-        expect(onToggle).toHaveBeenCalledWith(expect.objectContaining({ detail: { open: false } }));
-      });
-
-      test('Renders two landmarks in closed state', () => {
-        const props = {
-          [openProp]: false,
-          [handler]: () => {},
-        };
-        const { wrapper } = renderComponent(<AppLayout {...props} />);
-        const landmarks = findLandmarks(wrapper);
-        expect(landmarks).toHaveLength(2);
-
-        const toggleElement = findToggle(wrapper).getElement();
-
-        if (landmarks[0].getElement().contains(toggleElement)) {
-          expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'false');
-          expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'true');
-        } else {
-          expect(landmarks[1].getElement()).toContainElement(toggleElement);
-          expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'false');
-          expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'true');
-        }
-      });
-
-      test('Renders two landmarks in open state', () => {
-        const props = {
-          [openProp]: true,
-          [handler]: () => {},
-        };
-        const { wrapper } = renderComponent(<AppLayout {...props} />);
-        const landmarks = findLandmarks(wrapper);
-        expect(landmarks).toHaveLength(2);
-        const toggleElement = findToggle(wrapper).getElement();
-
-        if (landmarks[0].getElement().contains(toggleElement)) {
-          expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'true');
-          expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'false');
-        } else {
-          expect(landmarks[1].getElement()).toContainElement(toggleElement);
-          expect(landmarks[1].getElement()).toHaveAttribute('aria-hidden', 'true');
-          expect(landmarks[0].getElement()).toHaveAttribute('aria-hidden', 'false');
-        }
-      });
-
-      test('Renders aria-expanded only on toggle', () => {
-        const props = {
-          [openProp]: false,
-          [handler]: () => {},
-        };
-
-        const { wrapper } = renderComponent(<AppLayout {...props} />);
-
-        expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-expanded', 'false');
-        expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-haspopup', 'true');
-        expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-expanded');
-        expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-haspopup');
-      });
-
-      test('Does not add a label to the toggle and landmark when they are not defined', () => {
-        const { wrapper } = renderComponent(<AppLayout />);
-        expect(findToggle(wrapper).getElement()).not.toHaveAttribute('aria-label');
-        expect(findLandmarks(wrapper)[0].getElement()).not.toHaveAttribute('aria-label');
-      });
-
-      test('Adds labels to toggle button and landmark when defined', () => {
-        const labels = {
-          navigationToggle: 'toggle',
-          toolsToggle: 'toggle',
-          navigation: 'landmark',
-          tools: 'landmark',
-        };
-
-        const { wrapper } = renderComponent(<AppLayout ariaLabels={labels} />);
-        expect(findToggle(wrapper).getElement()).toHaveAttribute('aria-label', 'toggle');
-        expect(findLandmarks(wrapper)[0].getElement()).toHaveAttribute('aria-label', 'landmark');
-      });
-
-      test('Close button does have a label if it is defined', () => {
-        const props = { [openProp]: true, [handler]: () => {} };
-        const labels = {
-          navigationClose: 'close label',
-          toolsClose: 'close label',
-        };
-        const { wrapper } = renderComponent(<AppLayout {...props} ariaLabels={labels} />);
-
-        expect(findClose(wrapper).getElement()).toHaveAttribute('aria-label', 'close label');
-      });
-
-      test('Close button does not render a label if is not defined', () => {
-        const props = { [openProp]: true, [handler]: () => {} };
-        const { wrapper } = renderComponent(<AppLayout {...props} />);
-
-        expect(findClose(wrapper).getElement()).not.toHaveAttribute('aria-label');
-      });
-
-      test('Opens and closes drawer in uncontrolled mode', () => {
-        // use content type with initial closed state for all drawers
-        const { wrapper } = renderComponent(<AppLayout contentType="form" />);
-        expect(isDrawerClosed(findElement(wrapper))).toBe(true);
-
-        act(() => findToggle(wrapper).click());
-        expect(isDrawerClosed(findElement(wrapper))).toBe(false);
-
-        act(() => findClose(wrapper).click());
-        expect(isDrawerClosed(findElement(wrapper))).toBe(true);
-      });
-
-      test('Moves focus between open and close buttons', () => {
-        // use content type with initial closed state for all drawers
-        const { wrapper } = renderComponent(<AppLayout contentType="form" />);
-
-        act(() => findToggle(wrapper).click());
-        expect(findClose(wrapper).getElement()).toBe(document.activeElement);
-
-        act(() => findClose(wrapper).click());
-        expect(findToggle(wrapper).getElement()).toBe(document.activeElement);
-      });
-
-      test(`Should not render the drawer if ${hideProp} is set to true`, () => {
-        const props = { [hideProp]: true };
-        const { wrapper } = renderComponent(<AppLayout {...props} />);
-        expect(findElement(wrapper)).toBeFalsy();
-        expect(findLandmarks(wrapper)).toHaveLength(0);
-      });
-    });
-  });
+    }
+  );
 
   // Drawers tests
 
@@ -244,7 +256,7 @@ describeEachAppLayout(() => {
       const { wrapper } = renderComponent(<AppLayout contentType="form" {...drawersClosed} />);
 
       wrapper.findDrawersTriggers()![0].click();
-      expect(onChange).toHaveBeenCalledTimes(1);
+
       expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ detail: 'security' }));
     });
 
diff --git a/src/app-layout/__tests__/mobile.test.tsx b/src/app-layout/__tests__/mobile.test.tsx
index d142b3249d..4748d86fc4 100644
--- a/src/app-layout/__tests__/mobile.test.tsx
+++ b/src/app-layout/__tests__/mobile.test.tsx
@@ -1,6 +1,6 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-import React from 'react';
+import React, { useState } from 'react';
 import { act } from 'react-dom/test-utils';
 import {
   describeEachThemeAppLayout,
@@ -22,12 +22,33 @@ import testUtilsStyles from '../../../lib/components/app-layout/test-classes/sty
 
 import visualRefreshRefactoredStyles from '../../../lib/components/app-layout/visual-refresh/styles.css.js';
 import { findUpUntil } from '../../../lib/components/internal/utils/dom';
+import SideNavigation from '../../../lib/components/side-navigation';
 
 jest.mock('@cloudscape-design/component-toolkit/internal', () => ({
   ...jest.requireActual('@cloudscape-design/component-toolkit/internal'),
   isMotionDisabled: jest.fn().mockReturnValue(true),
 }));
 
+function AppLayoutWithControlledNavigation({
+  initialNavigationOpen,
+  navigation,
+}: {
+  initialNavigationOpen: boolean;
+  navigation: React.ReactNode;
+}) {
+  const [navigationOpen, setNavigationOpen] = useState(initialNavigationOpen);
+
+  return (
+    <AppLayout
+      navigationOpen={navigationOpen}
+      onNavigationChange={({ detail }) => {
+        setNavigationOpen(detail.open);
+      }}
+      navigation={navigation}
+    />
+  );
+}
+
 describeEachThemeAppLayout(true, theme => {
   // In refactored Visual Refresh different styles are used compared to Classic
   const mobileBarClassName = theme === 'refresh' ? testUtilsStyles['mobile-bar'] : toolbarStyles['mobile-bar'];
@@ -46,6 +67,37 @@ describeEachThemeAppLayout(true, theme => {
     expect(wrapper.findToolsToggle().getElement()).toBeEnabled();
   });
 
+  test('AppLayout with controlled navigation has navigation forcely closed on initial load', () => {
+    const { wrapper } = renderComponent(
+      <AppLayoutWithControlledNavigation
+        initialNavigationOpen={true}
+        navigation={
+          <>
+            <h1>Navigation</h1>
+            <a href="test">Link</a>
+          </>
+        }
+      />
+    );
+    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
+  });
+
+  test('AppLayout with uncontrolled navigation has navigation forcely closed on initial load', () => {
+    const { wrapper } = renderComponent(
+      <AppLayout
+        navigation={
+          <>
+            <h1>Navigation</h1>
+            <a href="test">Link</a>
+          </>
+        }
+      />
+    );
+    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
+  });
+
   test('renders open navigation state', () => {
     const { wrapper } = renderComponent(<AppLayout navigationOpen={true} onNavigationChange={() => {}} />);
     expect(wrapper.findNavigation()).toBeTruthy();
@@ -86,30 +138,58 @@ describeEachThemeAppLayout(true, theme => {
   });
 
   test('closes navigation when clicking on links', () => {
-    const onNavigationChange = jest.fn();
     const { wrapper } = renderComponent(
-      <AppLayout
-        navigationOpen={true}
-        onNavigationChange={onNavigationChange}
+      <AppLayoutWithControlledNavigation
+        initialNavigationOpen={true}
         navigation={
           <>
             <h1>Navigation</h1>
-            <a href="#">Link</a>
+            <a href="test">Link</a>
           </>
         }
       />
     );
+    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
+
+    wrapper.findNavigationToggle().click();
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
+
     wrapper.findNavigation().find('a')!.click();
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
+  });
+
+  test('closes navigation when clicking on a link in the Side Navigation component', () => {
+    const { wrapper } = renderComponent(
+      <AppLayoutWithControlledNavigation
+        initialNavigationOpen={true}
+        navigation={
+          <SideNavigation
+            items={[
+              {
+                type: 'link',
+                text: 'Page 1',
+                href: '#/page1',
+              },
+            ]}
+          />
+        }
+      />
+    );
+    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
+
+    wrapper.findNavigationToggle().click();
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
 
-    expect(onNavigationChange).toHaveBeenCalledWith(expect.objectContaining({ detail: { open: false } }));
+    wrapper.findNavigation().find('a')!.click();
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
   });
 
   test('does not close navigation when anchor without href was clicked', () => {
-    const onNavigationChange = jest.fn();
     const { wrapper } = renderComponent(
-      <AppLayout
-        navigationOpen={true}
-        onNavigationChange={onNavigationChange}
+      <AppLayoutWithControlledNavigation
+        initialNavigationOpen={true}
         navigation={
           <>
             <h1>Navigation</h1>
@@ -118,28 +198,36 @@ describeEachThemeAppLayout(true, theme => {
         }
       />
     );
-    wrapper.findNavigation().find('a')!.click();
+    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
+
+    wrapper.findNavigationToggle().click();
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
 
-    expect(onNavigationChange).not.toHaveBeenCalled();
+    wrapper.findNavigation().find('a')!.click();
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
   });
 
   test('does not close navigation when other elements were clicked', () => {
-    const onNavigationChange = jest.fn();
     const { wrapper } = renderComponent(
-      <AppLayout
-        navigationOpen={true}
-        onNavigationChange={onNavigationChange}
+      <AppLayoutWithControlledNavigation
+        initialNavigationOpen={true}
         navigation={
           <>
             <h1>Navigation</h1>
-            <a href="#">Link</a>
+            <a>Link</a>
           </>
         }
       />
     );
-    wrapper.findNavigation().find('h1')!.click();
+    // AppLayout forcely closes the navigation on the first load on mobile, so the main content is visible
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(true);
 
-    expect(onNavigationChange).not.toHaveBeenCalled();
+    wrapper.findNavigationToggle().click();
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
+
+    wrapper.findNavigation().find('h1')!.click();
+    expect(isDrawerClosed(wrapper.findNavigation())).toBe(false);
   });
 
   test('does not close tools when clicking on any element', () => {
diff --git a/src/app-layout/__tests__/utils.tsx b/src/app-layout/__tests__/utils.tsx
index b01ed83590..4f06a052b6 100644
--- a/src/app-layout/__tests__/utils.tsx
+++ b/src/app-layout/__tests__/utils.tsx
@@ -76,9 +76,9 @@ export function describeEachThemeAppLayout(isMobile: boolean, callback: (theme:
   }
 }
 
-export function describeEachAppLayout(callback: () => void) {
+export function describeEachAppLayout(callback: (size: 'desktop' | 'mobile') => void) {
   for (const theme of ['refresh', 'classic']) {
-    for (const size of ['desktop', 'mobile']) {
+    for (const size of ['desktop', 'mobile'] as const) {
       describe(`Theme=${theme}, Size=${size}`, () => {
         beforeEach(() => {
           (useMobile as jest.Mock).mockReturnValue(size === 'mobile');
@@ -88,7 +88,7 @@ export function describeEachAppLayout(callback: () => void) {
           (useMobile as jest.Mock).mockReset();
           (useVisualRefresh as jest.Mock).mockReset();
         });
-        callback();
+        callback(size);
       });
     }
   }
diff --git a/src/app-layout/index.tsx b/src/app-layout/index.tsx
index 3440267a8b..62604687b0 100644
--- a/src/app-layout/index.tsx
+++ b/src/app-layout/index.tsx
@@ -170,14 +170,11 @@ const OldAppLayout = React.forwardRef(
       setLastInteraction: setDrawerLastInteraction,
     } = useDrawerFocusControl([activeDrawer?.resizable], toolsOpen || activeDrawer !== undefined, true);
 
-    const onNavigationToggle = useCallback(
-      (open: boolean) => {
-        setNavigationOpen(open);
-        focusNavButtons();
-        fireNonCancelableEvent(onNavigationChange, { open });
-      },
-      [setNavigationOpen, onNavigationChange, focusNavButtons]
-    );
+    const onNavigationToggle = useStableCallback((open: boolean) => {
+      setNavigationOpen(open);
+      focusNavButtons();
+      fireNonCancelableEvent(onNavigationChange, { open });
+    });
     const onToolsToggle = useCallback(
       (open: boolean) => {
         setToolsOpen(open);
@@ -197,6 +194,13 @@ const OldAppLayout = React.forwardRef(
       }
     };
 
+    useEffect(() => {
+      // Close navigation drawer on mobile so that the main content is visible
+      if (isMobile) {
+        onNavigationToggle(false);
+      }
+    }, [isMobile, onNavigationToggle]);
+
     const navigationVisible = !navigationHide && navigationOpen;
     const toolsVisible = !toolsHide && toolsOpen;
 
diff --git a/src/app-layout/visual-refresh/context.tsx b/src/app-layout/visual-refresh/context.tsx
index efada3de4b..76dbfe179a 100644
--- a/src/app-layout/visual-refresh/context.tsx
+++ b/src/app-layout/visual-refresh/context.tsx
@@ -26,7 +26,7 @@ import { SplitPanelSideToggleProps } from '../../internal/context/split-panel-co
 import { useObservedElement } from '../utils/use-observed-element';
 import { useMobile } from '../../internal/hooks/use-mobile';
 import { DrawerItem, InternalDrawerProps } from '../drawer/interfaces';
-import { warnOnce } from '@cloudscape-design/component-toolkit/internal';
+import { useStableCallback, warnOnce } from '@cloudscape-design/component-toolkit/internal';
 import useResize from '../utils/use-resize';
 import styles from './styles.css.js';
 import { useContainerQuery } from '@cloudscape-design/component-toolkit';
@@ -182,14 +182,18 @@ export const AppLayoutInternalsProvider = React.forwardRef(
 
     const { refs: navigationRefs, setFocus: focusNavButtons } = useFocusControl(isNavigationOpen);
 
-    const handleNavigationClick = useCallback(
-      function handleNavigationChange(isOpen: boolean) {
-        setIsNavigationOpen(isOpen);
-        focusNavButtons();
-        fireNonCancelableEvent(props.onNavigationChange, { open: isOpen });
-      },
-      [props.onNavigationChange, setIsNavigationOpen, focusNavButtons]
-    );
+    const handleNavigationClick = useStableCallback(function handleNavigationChange(isOpen: boolean) {
+      setIsNavigationOpen(isOpen);
+      focusNavButtons();
+      fireNonCancelableEvent(props.onNavigationChange, { open: isOpen });
+    });
+
+    useEffect(() => {
+      // Close navigation drawer on mobile so that the main content is visible
+      if (isMobile) {
+        handleNavigationClick(false);
+      }
+    }, [isMobile, handleNavigationClick]);
 
     /**
      * The useControllable hook will set the default value and manage either
diff --git a/src/expandable-section/expandable-section-header.tsx b/src/expandable-section/expandable-section-header.tsx
index 26a7f1a9da..1429fb6b46 100644
--- a/src/expandable-section/expandable-section-header.tsx
+++ b/src/expandable-section/expandable-section-header.tsx
@@ -95,7 +95,7 @@ const ExpandableNavigationHeader = ({
   icon,
 }: ExpandableNavigationHeaderProps) => {
   return (
-    <div id={id} className={clsx(className, styles['click-target'])} onClick={onClick}>
+    <div id={id} className={clsx(className, styles['click-target'])}>
       <button
         className={clsx(styles['icon-container'], styles['expand-button'])}
         aria-labelledby={ariaLabelledBy}
@@ -103,6 +103,7 @@ const ExpandableNavigationHeader = ({
         aria-controls={ariaControls}
         aria-expanded={expanded}
         type="button"
+        onClick={onClick}
       >
         {icon}
       </button>
diff --git a/src/side-navigation/internal.tsx b/src/side-navigation/internal.tsx
index 66048ce75c..601a9e54b2 100644
--- a/src/side-navigation/internal.tsx
+++ b/src/side-navigation/internal.tsx
@@ -227,9 +227,6 @@ function Link({ definition, expanded, activeHref, fireFollow }: LinkProps) {
 
   const onClick = useCallback(
     (event: React.MouseEvent) => {
-      // Prevent the click event from toggling outer expandable sections.
-      event.stopPropagation();
-
       if (isPlainLeftClick(event)) {
         fireFollow(definition, event);
       }

From fb4b0275904963325844a9516b5623cd46b6852e Mon Sep 17 00:00:00 2001
From: Andrei Zhaleznichenka <zhalea@amazon.de>
Date: Mon, 28 Aug 2023 14:53:32 +0200
Subject: [PATCH 8/8] chore: Table grid nav updates (#1478)

---
 .../grid-navigation-custom.page.tsx           | 20 ++++-
 src/table/__tests__/a11y.test.tsx             |  2 +-
 src/table/internal.tsx                        |  2 +-
 .../__integ__/grid-navigation.test.ts         |  6 +-
 .../__tests__/use-grid-navigation.test.tsx    | 83 +++++++++++++++++
 src/table/table-role/grid-navigation.md       |  4 +-
 src/table/table-role/interfaces.ts            |  2 +-
 src/table/table-role/table-role-helper.ts     | 32 +++++--
 src/table/table-role/use-grid-navigation.ts   | 15 ++--
 src/table/table-role/utils.ts                 | 88 +++++++++++++------
 10 files changed, 201 insertions(+), 53 deletions(-)

diff --git a/pages/table-fragments/grid-navigation-custom.page.tsx b/pages/table-fragments/grid-navigation-custom.page.tsx
index d15501d5d2..817cbadc72 100644
--- a/pages/table-fragments/grid-navigation-custom.page.tsx
+++ b/pages/table-fragments/grid-navigation-custom.page.tsx
@@ -15,11 +15,13 @@ import {
   Input,
   Link,
   SegmentedControl,
+  Select,
 } from '~components';
 import styles from './styles.scss';
 import { id as generateId, generateItems, Instance } from '../table/generate-data';
 import AppContext, { AppContextType } from '../app/app-context';
 import {
+  TableRole,
   getTableCellRoleProps,
   getTableColHeaderRoleProps,
   getTableHeaderRowRoleProps,
@@ -34,6 +36,7 @@ import appLayoutLabels from '../app-layout/utils/labels';
 type PageContext = React.Context<
   AppContextType<{
     pageSize: number;
+    tableRole: TableRole;
   }>
 >;
 
@@ -94,10 +97,13 @@ const createColumnDefinitions = ({
   { key: 'type', label: 'Type', render: (item: Instance) => item.type },
 ];
 
+const tableRoleOptions = [{ value: 'table' }, { value: 'grid' }, { value: 'grid-default' }];
+
 export default function Page() {
   const [toolsOpen, setToolsOpen] = useState(false);
   const { urlParams, setUrlParams } = useContext(AppContext as PageContext);
   const pageSize = urlParams.pageSize ?? 10;
+  const tableRole = urlParams.tableRole ?? 'grid';
 
   const [items, setItems] = useState(generateItems(25));
   const columnDefinitions = useMemo(
@@ -117,7 +123,6 @@ export default function Page() {
 
   const tableRef = useRef<HTMLTableElement>(null);
 
-  const tableRole = 'grid';
   useGridNavigation({ tableRole, pageSize, getTable: () => tableRef.current });
 
   const sortedItems = useMemo(() => {
@@ -149,6 +154,14 @@ export default function Page() {
                       onChange={event => setUrlParams({ pageSize: parseInt(event.detail.value) })}
                     />
                   </FormField>
+
+                  <FormField label="Table role">
+                    <Select
+                      options={tableRoleOptions}
+                      selectedOption={tableRoleOptions.find(option => option.value === tableRole) ?? null}
+                      onChange={event => setUrlParams({ tableRole: event.detail.selectedOption.value as TableRole })}
+                    />
+                  </FormField>
                 </ColumnLayout>
 
                 <Link onFollow={() => setToolsOpen(true)} data-testid="link-before">
@@ -178,7 +191,7 @@ export default function Page() {
                       <th
                         key={column.key}
                         className={styles['custom-table-cell']}
-                        {...getTableColHeaderRoleProps({ tableRole, colIndex })}
+                        {...getTableColHeaderRoleProps({ tableRole, colIndex, isWidget: true })}
                       >
                         <div style={{ display: 'flex', gap: '8px', flexWrap: 'nowrap' }}>
                           <button
@@ -208,8 +221,7 @@ export default function Page() {
                         <td
                           key={column.key}
                           className={styles['custom-table-cell']}
-                          {...getTableCellRoleProps({ tableRole, colIndex })}
-                          data-widget-cell={column.isWidget}
+                          {...getTableCellRoleProps({ tableRole, colIndex, isWidget: column.isWidget })}
                         >
                           {column.render(item)}
                         </td>
diff --git a/src/table/__tests__/a11y.test.tsx b/src/table/__tests__/a11y.test.tsx
index 3773847f5c..17a39d8011 100644
--- a/src/table/__tests__/a11y.test.tsx
+++ b/src/table/__tests__/a11y.test.tsx
@@ -102,7 +102,7 @@ describe('labels', () => {
       const [headerRow, firstRowInTable] = wrapper.findAll('tr');
       // header row is always index 1
       expect(headerRow!.getElement().getAttribute('aria-rowindex')).toEqual('1');
-      // rows in a table are index + 1  as header row is index 1
+      // rows in a table are index + 1 as header row is index 1
       expect(firstRowInTable!.getElement().getAttribute('aria-rowindex')).toEqual('22');
     });
   });
diff --git a/src/table/internal.tsx b/src/table/internal.tsx
index c0f83876bf..0b56f5707f 100644
--- a/src/table/internal.tsx
+++ b/src/table/internal.tsx
@@ -186,7 +186,7 @@ const InternalTable = React.forwardRef(
 
     const hasStickyColumns = !!((stickyColumns?.first ?? 0) + (stickyColumns?.last ?? 0) > 0);
     const hasEditableCells = !!columnDefinitions.find(col => col.editConfig);
-    const tableRole = hasEditableCells ? 'grid-no-navigation' : 'table';
+    const tableRole = hasEditableCells ? 'grid-default' : 'table';
 
     const theadProps: TheadProps = {
       containerWidth,
diff --git a/src/table/table-role/__integ__/grid-navigation.test.ts b/src/table/table-role/__integ__/grid-navigation.test.ts
index 4957885cfb..8b25dc5920 100644
--- a/src/table/table-role/__integ__/grid-navigation.test.ts
+++ b/src/table/table-role/__integ__/grid-navigation.test.ts
@@ -37,15 +37,15 @@ test(
 
     await page.click('[data-testid="link-before"]');
     await page.keys('Tab');
-    await expect(page.isFocused('tr[aria-rowindex="1"] > th[aria-colindex="1"] button')).resolves.toBe(true);
+    await expect(page.isFocused('tr[aria-rowindex="1"] > th[aria-colindex="1"]')).resolves.toBe(true);
 
     await page.keys('Tab');
     await expect(page.isFocused('[data-testid="link-after"]')).resolves.toBe(true);
 
     await page.keys(['Shift', 'Tab', 'Null']);
-    await expect(page.isFocused('tr[aria-rowindex="1"] > th[aria-colindex="1"] button')).resolves.toBe(true);
+    await expect(page.isFocused('tr[aria-rowindex="1"] > th[aria-colindex="1"]')).resolves.toBe(true);
 
-    await page.keys(['ArrowRight', 'ArrowDown', 'ArrowRight']);
+    await page.keys(['ArrowRight', 'ArrowDown', 'Enter', 'ArrowRight']);
     await expect(page.isFocused('tr[aria-rowindex="2"] [aria-label="Duplicate item"]')).resolves.toBe(true);
 
     await page.keys('Tab');
diff --git a/src/table/table-role/__tests__/use-grid-navigation.test.tsx b/src/table/table-role/__tests__/use-grid-navigation.test.tsx
index 9bef7b7972..424b229fe8 100644
--- a/src/table/table-role/__tests__/use-grid-navigation.test.tsx
+++ b/src/table/table-role/__tests__/use-grid-navigation.test.tsx
@@ -135,6 +135,40 @@ test('supports arrow keys navigation', () => {
   expect(getActiveElement()).toEqual(['button', 'desc']);
 });
 
+test('up/down navigation works when first data row index is not 2', () => {
+  function TestComponent() {
+    const tableRef = useRef<HTMLTableElement>(null);
+    useGridNavigation({ tableRole: 'grid', pageSize: 2, getTable: () => tableRef.current });
+    return (
+      <table role="grid" ref={tableRef}>
+        <thead>
+          <tr aria-rowindex={1}>
+            <th aria-colindex={1}>header-1</th>
+            <th aria-colindex={2}>header-2</th>
+          </tr>
+        </thead>
+        <tbody>
+          <tr aria-rowindex={5}>
+            <td aria-colindex={1}>cell-1-1</td>
+            <td aria-colindex={2}>cell-1-2</td>
+          </tr>
+        </tbody>
+      </table>
+    );
+  }
+  const { container } = render(<TestComponent />);
+  const table = container.querySelector('table')!;
+
+  container.querySelector('th')?.focus();
+  expect(getActiveElement()).toEqual(['th', 'header-1']);
+
+  fireEvent.keyDown(table, { keyCode: KeyCode.down });
+  expect(getActiveElement()).toEqual(['td', 'cell-1-1']);
+
+  fireEvent.keyDown(table, { keyCode: KeyCode.up });
+  expect(getActiveElement()).toEqual(['th', 'header-1']);
+});
+
 test('supports key combination navigation', () => {
   const { container } = render(<InteractiveTable />);
   const table = container.querySelector('table')!;
@@ -441,3 +475,52 @@ test('throws no error when focusing on incorrect target', () => {
   g.focus();
   expect(g).toHaveFocus();
 });
+
+test('elements focus is restored if table changes role after being rendered as grid', () => {
+  function TestComponent({ tableRole }: { tableRole: 'table' | 'grid' }) {
+    const tableRef = useRef<HTMLTableElement>(null);
+    useGridNavigation({ tableRole, pageSize: 2, getTable: () => tableRef.current });
+    return (
+      <table role={tableRole} ref={tableRef}>
+        <tbody>
+          <tr aria-rowindex={1}>
+            <td aria-colindex={1}>
+              <button>action 1</button>
+            </td>
+            <td aria-colindex={2}>
+              <button>action 2</button>
+            </td>
+            <td aria-colindex={3}>
+              <button>action 3</button>
+            </td>
+          </tr>
+        </tbody>
+      </table>
+    );
+  }
+
+  const { container, rerender } = render(<TestComponent tableRole="grid" />);
+
+  expect(Array.from(container.querySelectorAll('button')).map(button => button.tabIndex)).toEqual([0, -1, -1]);
+
+  rerender(<TestComponent tableRole="table" />);
+
+  expect(Array.from(container.querySelectorAll('button')).map(button => button.tabIndex)).toEqual([0, 0, 0]);
+});
+
+test('when focus is inside widget cell the cell and the cell next to it are focusable', () => {
+  const { container } = render(<InteractiveTable actionsWidget={true} />);
+  const table = container.querySelector('table')!;
+  const widgetCell = container.querySelector('[aria-rowindex="2"]')!.querySelector('[aria-colindex="3"]')!;
+  const nextCell = container.querySelector('[aria-rowindex="3"]')!.querySelector('[aria-colindex="1"]')!;
+
+  widgetCell.querySelector('button')?.focus();
+
+  expect(widgetCell).toHaveAttribute('tabIndex', '0');
+  expect(nextCell).toHaveAttribute('tabIndex', '0');
+
+  fireEvent.keyDown(table, { keyCode: KeyCode.escape });
+
+  expect(widgetCell).toHaveAttribute('tabIndex', '0');
+  expect(nextCell).toHaveAttribute('tabIndex', '-1');
+});
diff --git a/src/table/table-role/grid-navigation.md b/src/table/table-role/grid-navigation.md
index ffbc6dcf40..bfd971148c 100644
--- a/src/table/table-role/grid-navigation.md
+++ b/src/table/table-role/grid-navigation.md
@@ -47,4 +47,6 @@ See: https://www.w3.org/WAI/ARIA/apg/patterns/grid/#gridNav_focus
 
 Widget cells are those including one or multiple elements that utilize arrow keys interaction model such is segmented control, radio group, slider, etc. The widget cells are not determined automatically and must be explicitly specified.
 
-Same as for the multi-element cells the focus is not automatically moved inside when a cell is navigated to. The `Enter`, `F2` and `Escape` commands work the same way. The difference is that when the focus is inside a widget cell the navigation commands are not intercepted. Besides, the default focusing behavior is restored so that the elements inside can be navigated with `Tab` and `Shift + Tab` commands. 
+Same as for the multi-element cells the focus is not automatically moved inside when a cell is navigated to. The `Enter`, `F2` and `Escape` commands work the same way. The difference is that when the focus is inside a widget cell the navigation commands are not intercepted. Besides, the default focusing behavior is restored so that the elements inside can be navigated with `Tab` and `Shift + Tab` commands.
+
+When the focus is within a widget cell all table cells become focusable so that pressing `Tab` and `Shift + Tab` also restores table navigation by moving the focus to the cell itself or the cell next to it (if available).
diff --git a/src/table/table-role/interfaces.ts b/src/table/table-role/interfaces.ts
index af6d7382d5..e73a55e38f 100644
--- a/src/table/table-role/interfaces.ts
+++ b/src/table/table-role/interfaces.ts
@@ -1,7 +1,7 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-export type TableRole = 'table' | 'grid' | 'grid-no-navigation';
+export type TableRole = 'table' | 'grid' | 'grid-default';
 
 export interface GridNavigationProps {
   tableRole: TableRole;
diff --git a/src/table/table-role/table-role-helper.ts b/src/table/table-role/table-role-helper.ts
index b09169b9e7..ecd1f709ae 100644
--- a/src/table/table-role/table-role-helper.ts
+++ b/src/table/table-role/table-role-helper.ts
@@ -27,7 +27,7 @@ export function getTableRoleProps(options: {
 
   // Browsers have weird mechanism to guess whether it's a data table or a layout table.
   // If we state explicitly, they get it always correctly even with low number of rows.
-  nativeProps.role = options.tableRole === 'grid-no-navigation' ? 'grid' : options.tableRole;
+  nativeProps.role = options.tableRole === 'grid-default' ? 'grid' : options.tableRole;
 
   nativeProps['aria-label'] = options.ariaLabel;
   nativeProps['aria-labelledby'] = options.ariaLabelledBy;
@@ -64,7 +64,7 @@ export function getTableHeaderRowRoleProps(options: { tableRole: TableRole }) {
   const nativeProps: React.HTMLAttributes<HTMLTableRowElement> = {};
 
   // For grids headers are treated similar to data rows and are indexed accordingly.
-  if (options.tableRole === 'grid' || options.tableRole === 'grid-no-navigation') {
+  if (options.tableRole === 'grid' || options.tableRole === 'grid-default') {
     nativeProps['aria-rowindex'] = 1;
   }
 
@@ -74,13 +74,13 @@ export function getTableHeaderRowRoleProps(options: { tableRole: TableRole }) {
 export function getTableRowRoleProps(options: { tableRole: TableRole; rowIndex: number; firstIndex?: number }) {
   const nativeProps: React.HTMLAttributes<HTMLTableRowElement> = {};
 
-  // For grids data cell indices are incremented by 2 to account for the header cells.
-  if (options.tableRole === 'grid' || options.tableRole === 'grid-no-navigation') {
-    nativeProps['aria-rowindex'] = (options.firstIndex ?? 0) + options.rowIndex + 2;
+  // The data cell indices are incremented by 1 to account for the header cells.
+  if (options.tableRole === 'grid') {
+    nativeProps['aria-rowindex'] = (options.firstIndex || 1) + options.rowIndex + 1;
   }
   // For tables indices are only added when the first index is not 0 (not the first page/frame).
   else if (options.firstIndex !== undefined) {
-    nativeProps['aria-rowindex'] = (options.firstIndex ?? 0) + options.rowIndex + 1;
+    nativeProps['aria-rowindex'] = options.firstIndex + options.rowIndex + 1;
   }
 
   return nativeProps;
@@ -90,8 +90,9 @@ export function getTableColHeaderRoleProps(options: {
   tableRole: TableRole;
   colIndex: number;
   sortingStatus?: SortingStatus;
+  isWidget?: boolean;
 }) {
-  const nativeProps: React.ThHTMLAttributes<HTMLTableCellElement> = {};
+  const nativeProps: React.ThHTMLAttributes<HTMLTableCellElement> & { 'data-widget-cell'?: boolean } = {};
 
   nativeProps.scope = 'col';
 
@@ -103,11 +104,20 @@ export function getTableColHeaderRoleProps(options: {
     nativeProps['aria-sort'] = getAriaSort(options.sortingStatus);
   }
 
+  if (options.tableRole === 'grid' && options.isWidget) {
+    nativeProps['data-widget-cell'] = true;
+  }
+
   return nativeProps;
 }
 
-export function getTableCellRoleProps(options: { tableRole: TableRole; colIndex: number; isRowHeader?: boolean }) {
-  const nativeProps: React.TdHTMLAttributes<HTMLTableCellElement> = {};
+export function getTableCellRoleProps(options: {
+  tableRole: TableRole;
+  colIndex: number;
+  isRowHeader?: boolean;
+  isWidget?: boolean;
+}) {
+  const nativeProps: React.TdHTMLAttributes<HTMLTableCellElement> & { 'data-widget-cell'?: boolean } = {};
 
   if (options.tableRole === 'grid') {
     nativeProps['aria-colindex'] = options.colIndex + 1;
@@ -117,5 +127,9 @@ export function getTableCellRoleProps(options: { tableRole: TableRole; colIndex:
     nativeProps.scope = 'row';
   }
 
+  if (options.tableRole === 'grid' && options.isWidget) {
+    nativeProps['data-widget-cell'] = true;
+  }
+
   return nativeProps;
 }
diff --git a/src/table/table-role/use-grid-navigation.ts b/src/table/table-role/use-grid-navigation.ts
index 80a77702d4..739d33666a 100644
--- a/src/table/table-role/use-grid-navigation.ts
+++ b/src/table/table-role/use-grid-navigation.ts
@@ -2,10 +2,10 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { useEffect, useMemo } from 'react';
-import { findFocusinCell, moveFocusBy, moveFocusIn, updateTableIndices } from './utils';
+import { findFocusinCell, moveFocusBy, moveFocusIn, restoreTableFocusables, updateTableFocusables } from './utils';
 import { FocusedCell, GridNavigationAPI, GridNavigationProps } from './interfaces';
 import { KeyCode } from '../../internal/keycode';
-import { containsOrEqual } from '../../internal/utils/dom';
+import { nodeContains } from '@cloudscape-design/component-toolkit/dom';
 
 /**
  * Makes table with role="grid" navigable with keyboard commands.
@@ -56,8 +56,7 @@ class GridNavigationModel {
     const mutationObserver = new MutationObserver(this.onMutation);
     mutationObserver.observe(table, { childList: true, subtree: true });
 
-    // No need to clean this up as no resources are allocated.
-    updateTableIndices(this.table, this.focusedCell ?? this.prevFocusedCell);
+    updateTableFocusables(this.table, this.focusedCell ?? this.prevFocusedCell);
 
     this.cleanup = () => {
       this.table.removeEventListener('focusin', this.onFocusin);
@@ -65,6 +64,8 @@ class GridNavigationModel {
       this.table.removeEventListener('keydown', this.onKeydown);
 
       mutationObserver.disconnect();
+
+      restoreTableFocusables(this.table);
     };
   }
 
@@ -96,7 +97,7 @@ class GridNavigationModel {
     this.prevFocusedCell = cell;
     this.focusedCell = cell;
 
-    updateTableIndices(this.table, cell);
+    updateTableFocusables(this.table, cell);
   };
 
   private onFocusout = () => {
@@ -212,13 +213,13 @@ class GridNavigationModel {
     for (const record of mutationRecords) {
       if (record.type === 'childList') {
         for (const removedNode of Array.from(record.removedNodes)) {
-          if (containsOrEqual(removedNode, this.prevFocusedCell.element)) {
+          if (removedNode === this.prevFocusedCell.element || nodeContains(removedNode, this.prevFocusedCell.element)) {
             moveFocusBy(this.table, this.prevFocusedCell, { y: 0, x: 0 });
           }
         }
       }
     }
 
-    updateTableIndices(this.table, this.focusedCell ?? this.prevFocusedCell);
+    updateTableFocusables(this.table, this.focusedCell ?? this.prevFocusedCell);
   };
 }
diff --git a/src/table/table-role/utils.ts b/src/table/table-role/utils.ts
index 22af4a59fc..9e43062d8d 100644
--- a/src/table/table-role/utils.ts
+++ b/src/table/table-role/utils.ts
@@ -51,7 +51,7 @@ export function moveFocusBy(table: HTMLTableElement, from: FocusedCell, delta: {
   const cellFocusables = getFocusables(from.cellElement);
   const eligibleForElementFocus =
     delta.x && !isWidgetCell(from.element) && (cellFocusables.length === 1 || from.element !== from.cellElement);
-  const targetElementIndex = from.elementIndex + delta.x;
+  const targetElementIndex = from.elementIndex === -1 ? -1 : from.elementIndex + delta.x;
   if (eligibleForElementFocus && 0 <= targetElementIndex && targetElementIndex < cellFocusables.length) {
     focus(cellFocusables[targetElementIndex]);
     return;
@@ -75,7 +75,8 @@ export function moveFocusBy(table: HTMLTableElement, from: FocusedCell, delta: {
   }
 
   // For non-widget cell focus on the focusable element inside if exactly one is available.
-  const focusIndex = delta.x === 0 ? from.elementIndex : targetCellFocusables.length === 1 ? 0 : -1;
+  const focusIndex =
+    delta.x === 0 && from.elementIndex !== -1 ? from.elementIndex : targetCellFocusables.length === 1 ? 0 : -1;
   const focusTarget = targetCellFocusables[focusIndex] ?? targetCell;
   focus(focusTarget);
 }
@@ -88,12 +89,20 @@ export function moveFocusIn(from: FocusedCell) {
 }
 
 /**
- * Ensures exactly one table element is focusable for the entire table to have a single TAB stop.
+ * Overrides focusability of the table elements to make focus targets controllable with keyboard commands.
  */
-export function updateTableIndices(table: HTMLTableElement, cell: null | FocusedCell) {
-  const tableCells = table.querySelectorAll('td,th') as NodeListOf<HTMLTableCellElement>;
+export function updateTableFocusables(table: HTMLTableElement, cell: null | FocusedCell) {
+  // Restore default focus behavior and make all cells focusable when focus is inside a widget cell.
+  if (cell && cell.widget && cell.element !== cell.cellElement) {
+    for (const focusable of getFocusables(table)) {
+      focusable.tabIndex = 0;
+    }
+    return;
+  }
 
-  for (const cell of Array.from(tableCells)) {
+  const tableCells = Array.from(table.querySelectorAll('td,th') as NodeListOf<HTMLTableCellElement>);
+
+  for (const cell of tableCells) {
     cell.tabIndex = -1;
     cell.setAttribute('data-focusable', 'true');
   }
@@ -102,20 +111,29 @@ export function updateTableIndices(table: HTMLTableElement, cell: null | Focused
     focusable.setAttribute('data-focusable', 'true');
   }
 
-  // Make focused element the only focusable element of the table.
+  // The only focusable element of the table.
+  let focusTarget: undefined | HTMLElement = tableCells[0];
+
   if (cell && table.contains(cell.element)) {
-    cell.element.tabIndex = 0;
+    focusTarget = cell.element;
+  } else if (tableCells.length > 0) {
+    const cellFocusables = getFocusables(tableCells[0]);
+    const eligibleForElementFocus = !isWidgetCell(tableCells[0]) && cellFocusables.length === 1;
+    focusTarget = eligibleForElementFocus ? cellFocusables[0] : focusTarget;
+  }
 
-    // For widget cells also unmute all cell elements to be focusable with Tab/Shift+Tab.
-    if (cell.widget) {
-      getFocusables(cell.cellElement).forEach(element => (element.tabIndex = 0));
-    }
+  if (focusTarget) {
+    focusTarget.tabIndex = 0;
   }
+}
 
-  // Make first table cell the only focusable element of the table.
-  else if (tableCells[0]) {
-    const cellFocusables = getFocusables(tableCells[0]);
-    cellFocusables[0] ? (cellFocusables[0].tabIndex = 0) : (tableCells[0].tabIndex = 0);
+export function restoreTableFocusables(table: HTMLTableElement) {
+  for (const focusable of getFocusables(table)) {
+    if (focusable instanceof HTMLTableCellElement) {
+      focusable.tabIndex = -1;
+    } else {
+      focusable.tabIndex = 0;
+    }
   }
 }
 
@@ -133,12 +151,21 @@ function getFirstFocusable(element: HTMLElement) {
 
 function findTableRowByAriaRowIndex(table: HTMLTableElement, targetAriaRowIndex: number, delta: number) {
   let targetRow: null | HTMLTableRowElement = null;
-  const rowElements = table.querySelectorAll('tr[aria-rowindex]');
-  for (let elementIndex = 0; elementIndex < rowElements.length; elementIndex++) {
-    const rowIndex = parseInt(rowElements[elementIndex].getAttribute('aria-rowindex') ?? '');
-    targetRow = rowElements[elementIndex] as HTMLTableRowElement;
+  const rowElements = Array.from(table.querySelectorAll('tr[aria-rowindex]'));
+  if (delta < 0) {
+    rowElements.reverse();
+  }
+  for (const element of rowElements) {
+    const rowIndex = parseInt(element.getAttribute('aria-rowindex') ?? '');
+    targetRow = element as HTMLTableRowElement;
 
-    if (rowIndex === targetAriaRowIndex || (delta < 0 && rowIndex >= targetAriaRowIndex)) {
+    if (rowIndex === targetAriaRowIndex) {
+      break;
+    }
+    if (delta >= 0 && rowIndex > targetAriaRowIndex) {
+      break;
+    }
+    if (delta < 0 && rowIndex < targetAriaRowIndex) {
       break;
     }
   }
@@ -147,12 +174,21 @@ function findTableRowByAriaRowIndex(table: HTMLTableElement, targetAriaRowIndex:
 
 function findTableRowCellByAriaColIndex(tableRow: HTMLTableRowElement, targetAriaColIndex: number, delta: number) {
   let targetCell: null | HTMLTableCellElement = null;
-  const cellElements = tableRow.querySelectorAll('td[aria-colindex],th[aria-colindex]');
-  for (let elementIndex = 0; elementIndex < cellElements.length; elementIndex++) {
-    const columnIndex = parseInt(cellElements[elementIndex].getAttribute('aria-colindex') ?? '');
-    targetCell = cellElements[elementIndex] as HTMLTableCellElement;
+  const cellElements = Array.from(tableRow.querySelectorAll('td[aria-colindex],th[aria-colindex]'));
+  if (delta < 0) {
+    cellElements.reverse();
+  }
+  for (const element of cellElements) {
+    const columnIndex = parseInt(element.getAttribute('aria-colindex') ?? '');
+    targetCell = element as HTMLTableCellElement;
 
-    if (columnIndex === targetAriaColIndex || (delta < 0 && columnIndex >= targetAriaColIndex)) {
+    if (columnIndex === targetAriaColIndex) {
+      break;
+    }
+    if (delta >= 0 && columnIndex > targetAriaColIndex) {
+      break;
+    }
+    if (delta < 0 && columnIndex < targetAriaColIndex) {
       break;
     }
   }