Merge branch 'main' into fix-app-layout-nav-label

Author: avinashbot

pages/app-layout/utils/content-blocks.tsx

@@ -37,7 +37,6 @@ export function Containers() {
       {range(count).map(i => (
         <Container
           key={i}
-          data-testid={`container-${i + 1}`}
           header={
             <Header variant="h2" actions={<Button onClick={() => setCount(count - 1)}>Remove</Button>}>
               Demo container #{i + 1}

pages/app-layout/with-split-panel.page.tsx

@@ -1,9 +1,13 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
+import React, { useContext, useState } from 'react';
 
-import React, { useContext } from 'react';
-
-import { AppLayout, AppLayoutProps, Box, Header, Popover, SpaceBetween, SplitPanel, Toggle } from '~components';
+import AppLayout, { AppLayoutProps } from '~components/app-layout';
+import Header from '~components/header';
+import Popover from '~components/popover';
+import SpaceBetween from '~components/space-between';
+import SplitPanel from '~components/split-panel';
+import Toggle from '~components/toggle';
 
 import AppContext, { AppContextType } from '../app/app-context';
 import ScreenshotArea from '../utils/screenshot-area';
@@ -57,23 +61,37 @@ const DEMO_CONTENT = (
       pellentesque id. Porta lorem mollis aliquam ut porttitor leo a. Lectus quam id leo in vitae turpis massa sed.
       Pharetra pharetra massa massa ultricies mi.
     </p>
+    <p>
+      Pharetra et ultrices neque ornare. Bibendum neque egestas congue quisque egestas diam in arcu cursus. Porttitor
+      eget dolor morbi non arcu risus quis. Integer quis auctor elit sed vulputate mi sit. Mauris nunc congue nisi vitae
+      suscipit tellus mauris a diam. Diam donec adipiscing tristique risus nec feugiat in. Arcu felis bibendum ut
+      tristique et egestas quis. Nulla porttitor massa id neque aliquam vestibulum morbi blandit. In hac habitasse
+      platea dictumst quisque sagittis. Sollicitudin tempor id eu nisl nunc mi ipsum. Ornare aenean euismod elementum
+      nisi quis. Elementum curabitur vitae nunc sed velit dignissim sodales. Amet tellus cras adipiscing enim eu. Id
+      interdum velit laoreet id donec ultrices tincidunt. Ullamcorper eget nulla facilisi etiam. Sodales neque sodales
+      ut etiam sit amet nisl purus. Auctor urna nunc id cursus metus aliquam eleifend mi in. Urna condimentum mattis
+      pellentesque id. Porta lorem mollis aliquam ut porttitor leo a. Lectus quam id leo in vitae turpis massa sed.
+      Pharetra pharetra massa massa ultricies mi.
+    </p>
   </div>
 );
 
 export default function () {
-  const {
-    urlParams: { splitPanelEnabled = true, toolsEnabled = true, splitPanelPosition },
-    setUrlParams,
-  } = useContext(AppContext as SplitPanelDemoContext);
+  const { urlParams, setUrlParams } = useContext(AppContext as SplitPanelDemoContext);
+  const [splitPanelEnabled, setSplitPanelEnabled] = useState(urlParams.splitPanelEnabled ?? true);
+  const [toolsPanelEnabled, setToolsPanelEnabled] = useState(urlParams.toolsEnabled ?? true);
+
   return (
     <ScreenshotArea gutters={false}>
       <AppLayout
         ariaLabels={labels}
         breadcrumbs={<Breadcrumbs />}
         navigation={<Navigation />}
         tools={<Tools>{toolsContent.long}</Tools>}
-        toolsHide={!toolsEnabled}
-        splitPanelPreferences={{ position: splitPanelPosition }}
+        toolsHide={!toolsPanelEnabled}
+        splitPanelPreferences={{
+          position: urlParams.splitPanelPosition,
+        }}
         onSplitPanelPreferencesChange={event => {
           const { position } = event.detail;
           setUrlParams({ splitPanelPosition: position === 'side' ? position : undefined });
@@ -106,28 +124,28 @@ export default function () {
                 Demo page
               </Header>
             </div>
-
             <SpaceBetween size="l">
-              <SpaceBetween size="s" direction="horizontal">
-                <Toggle
-                  id="enable-split-panel"
-                  checked={splitPanelEnabled}
-                  onChange={e => setUrlParams({ splitPanelEnabled: e.detail.checked })}
-                >
-                  Enable split panel
-                </Toggle>
-                <Toggle
-                  id="enable-tools-panel"
-                  checked={toolsEnabled}
-                  onChange={e => setUrlParams({ toolsEnabled: e.detail.checked })}
-                >
-                  Enable tools panel
-                </Toggle>
-              </SpaceBetween>
-
+              <Toggle
+                id="enable-split-panel"
+                checked={splitPanelEnabled}
+                onChange={e => {
+                  setSplitPanelEnabled(e.detail.checked);
+                  setUrlParams({ splitPanelEnabled: e.detail.checked });
+                }}
+              >
+                Enable split panel
+              </Toggle>
+              <Toggle
+                id="enable-tools-panel"
+                checked={toolsPanelEnabled}
+                onChange={e => {
+                  setToolsPanelEnabled(e.detail.checked);
+                  setUrlParams({ toolsEnabled: e.detail.checked });
+                }}
+              >
+                Enable tools panel
+              </Toggle>
               <Containers />
-
-              <Box>{DEMO_CONTENT}</Box>
             </SpaceBetween>
           </>
         }

src/app-layout/__integ__/app-layout-drawers.test.ts

@@ -209,7 +209,7 @@ describe.each(['classic', 'refresh', 'refresh-toolbar'] as const)('%s', theme =>
       )
     );
 
-    test(
+    test.skip(
       'updates side split panel position when using different width drawers',
       setupTest(
         { theme, splitPanelPosition: 'side', screenSize: { ...viewports.desktop, width: 1450 } },

src/app-layout/__integ__/app-layout-split-panel.test.ts

@@ -82,18 +82,6 @@ class AppLayoutSplitViewPage extends BasePageObject {
         }, contentSelector);
     }
   }
-
-  hasPageScrollbar() {
-    return this.browser.execute(
-      () => window.document.documentElement.scrollHeight > window.document.documentElement.clientHeight
-    );
-  }
-
-  verifySplitPanelPosition(targetPosition: 'side' | 'bottom') {
-    return targetPosition === 'side'
-      ? this.isExisting(wrapper.findSplitPanel().findOpenPanelSide().toSelector())
-      : this.isExisting(wrapper.findSplitPanel().findOpenPanelBottom().toSelector());
-  }
 }
 
 describe.each(['classic', 'refresh', 'refresh-toolbar'] as const)('%s', theme => {
@@ -363,50 +351,4 @@ describe.each(['classic', 'refresh', 'refresh-toolbar'] as const)('%s', theme =>
       }, '#/light/app-layout/with-full-page-table-and-split-panel')
     );
   });
-
-  test(
-    'forces split panel to the bottom or allows on side when page scrollbar appears',
-    useBrowser(async browser => {
-      const page = new AppLayoutSplitViewPage(browser);
-      const dimensions = {
-        classic: { height: 1400, width: 945 },
-        refresh: { height: 1200, width: 1020 },
-        'refresh-toolbar': { height: 1400, width: 875 },
-      }[theme];
-      const content = wrapper.findContentRegion();
-
-      // Split panel is forced to the bottom when not enough horizontal space.
-      await page.setWindowSize(dimensions);
-      const params = new URLSearchParams({
-        splitPanelPosition: 'side',
-        visualRefresh: `${theme.startsWith('refresh')}`,
-        appLayoutToolbar: `${theme === 'refresh-toolbar'}`,
-      });
-      await browser.url(`#/light/app-layout/with-split-panel?${params.toString()}`);
-      await page.waitForVisible(content.toSelector());
-      await page.click(content.findContainer('[data-testid="container-1"]').findHeader().findButton().toSelector());
-      await page.click(content.findContainer('[data-testid="container-1"]').findHeader().findButton().toSelector());
-
-      // Open split panel and ensure it is forced to the bottom.
-      await page.click(wrapper.findSplitPanel().findOpenButton().toSelector());
-      await expect(page.verifySplitPanelPosition('bottom')).resolves.toBe(true);
-      await expect(page.hasPageScrollbar()).resolves.toBe(true);
-
-      // Make split panel smaller to ensure no page-level scrollbar.
-      const { height: screenHeight } = await page.getViewportSize();
-      await page.dragResizerTo({ x: 0, y: screenHeight });
-      await expect(page.verifySplitPanelPosition('bottom')).resolves.toBe(true);
-      await expect(page.hasPageScrollbar()).resolves.toBe(false);
-
-      // Split panel transitions to the side when enough horizontal space.
-      await page.setWindowSize({ ...dimensions, width: dimensions.width + 25 });
-      await expect(page.verifySplitPanelPosition('side')).resolves.toBe(true);
-      await expect(page.hasPageScrollbar()).resolves.toBe(true);
-
-      // Split panel transitions to the bottom when not enough horizontal space.
-      await page.setWindowSize(dimensions);
-      await expect(page.verifySplitPanelPosition('bottom')).resolves.toBe(true);
-      await expect(page.hasPageScrollbar()).resolves.toBe(false);
-    })
-  );
 });

src/app-layout/classic.tsx

@@ -23,8 +23,12 @@ import { ResizableDrawer } from './drawer/resizable-drawer';
 import { AppLayoutProps, AppLayoutPropsWithDefaults } from './interfaces';
 import { MobileToolbar } from './mobile-toolbar';
 import { Notifications } from './notifications';
-import { SideSplitPanelDrawer, SplitPanelProvider, SplitPanelProviderProps } from './split-panel';
-import { checkSplitPanelForcedPosition } from './split-panel/split-panel-utils';
+import {
+  SideSplitPanelDrawer,
+  SPLIT_PANEL_MIN_WIDTH,
+  SplitPanelProvider,
+  SplitPanelProviderProps,
+} from './split-panel';
 import { togglesConfig } from './toggles';
 import { getStickyOffsetVars } from './utils/sticky-offsets';
 import { TOOLS_DRAWER_ID, useDrawers } from './utils/use-drawers';
@@ -294,8 +298,10 @@ const ClassicAppLayout = React.forwardRef(
     };
 
     const effectiveToolsWidth = getEffectiveToolsWidth();
-    const splitPanelMaxWidth = resizableSpaceAvailable - effectiveToolsWidth;
-    const isSplitPanelForcedPosition = checkSplitPanelForcedPosition({ isMobile, splitPanelMaxWidth });
+
+    // if there is no space to display split panel in the side, force to bottom
+    const isSplitPanelForcedPosition =
+      isMobile || resizableSpaceAvailable - effectiveToolsWidth < SPLIT_PANEL_MIN_WIDTH;
     const finalSplitPanePosition = isSplitPanelForcedPosition ? 'bottom' : splitPanelPosition;
 
     const splitPaneAvailableOnTheSide = splitPanelDisplayed && finalSplitPanePosition === 'side';

src/app-layout/split-panel/constants.ts

@@ -2,4 +2,3 @@
 // SPDX-License-Identifier: Apache-2.0
 export const SPLIT_PANEL_MIN_HEIGHT = 160;
 export const SPLIT_PANEL_MIN_WIDTH = 280;
-export const SPLIT_PANEL_SCROLLBAR_MARGIN = 20;

src/app-layout/split-panel/split-panel-utils.ts

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { AppLayoutPropsWithDefaults } from '../interfaces';
-import { checkSplitPanelForcedPosition } from '../split-panel/split-panel-utils';
+import { SPLIT_PANEL_MIN_WIDTH } from '../split-panel';
 
 interface HorizontalLayoutInput {
   navigationOpen: boolean;
@@ -39,8 +39,7 @@ export function computeHorizontalLayout({
   );
   const totalActiveGlobalDrawersSize = Object.values(activeGlobalDrawersSizes).reduce((acc, size) => acc + size, 0);
 
-  const splitPanelMaxWidth = resizableSpaceAvailable - activeDrawerSize;
-  const splitPanelForcedPosition = checkSplitPanelForcedPosition({ isMobile, splitPanelMaxWidth });
+  const splitPanelForcedPosition = resizableSpaceAvailable - activeDrawerSize < SPLIT_PANEL_MIN_WIDTH || isMobile;
   const resolvedSplitPanelPosition = splitPanelForcedPosition ? 'bottom' : splitPanelPosition ?? 'bottom';
   const sideSplitPanelSize = resolvedSplitPanelPosition === 'side' && splitPanelOpen ? splitPanelSize ?? 0 : 0;
   const maxSplitPanelSize = Math.max(resizableSpaceAvailable - totalActiveGlobalDrawersSize - activeDrawerSize, 0);

src/app-layout/visual-refresh-toolbar/compute-layout.ts

@@ -23,7 +23,6 @@ import { useUniqueId } from '../../internal/hooks/use-unique-id';
 import { getSplitPanelDefaultSize } from '../../split-panel/utils/size-utils';
 import { AppLayoutProps, AppLayoutPropsWithDefaults } from '../interfaces';
 import { SPLIT_PANEL_MIN_WIDTH } from '../split-panel';
-import { checkSplitPanelForcedPosition } from '../split-panel/split-panel-utils';
 import { useDrawers } from '../utils/use-drawers';
 import { FocusControlRefs, useFocusControl } from '../utils/use-focus-control';
 import useResize from '../utils/use-resize';
@@ -235,7 +234,14 @@ export const AppLayoutInternalsProvider = React.forwardRef(
       [props.onSplitPanelToggle, isSplitPanelOpen, setIsSplitPanelOpen, setSplitPanelLastInteraction]
     );
 
-    const isSplitPanelForcedPosition = checkSplitPanelForcedPosition({ isMobile, splitPanelMaxWidth });
+    /**
+     * The Split Panel will be in forced (bottom) position if the defined minimum width is
+     * greater than the maximum width. In other words, the maximum width is the currently
+     * available horizontal space based on all other components that are rendered. If the
+     * minimum width exceeds this value then there is not enough horizontal space and we must
+     * force it to the bottom position.
+     */
+    const isSplitPanelForcedPosition = isMobile || SPLIT_PANEL_MIN_WIDTH > splitPanelMaxWidth;
     const splitPanelPosition = getSplitPanelPosition(isSplitPanelForcedPosition, splitPanelPreferences);
 
     /**

src/app-layout/visual-refresh/context.tsx

@@ -167,26 +167,19 @@ describe('Attribute Editor', () => {
     test('enables the add button by default', () => {
       const wrapper = renderAttributeEditor({ ...defaultProps });
       const buttonElement = wrapper.findAddButton().getElement();
-      expect(buttonElement).not.toHaveAttribute('aria-disabled');
+      expect(buttonElement).not.toHaveAttribute('disabled');
     });
 
     test('enables the add button when disableAddButton is false', () => {
       const wrapper = renderAttributeEditor({ ...defaultProps, disableAddButton: false });
       const buttonElement = wrapper.findAddButton().getElement();
-      expect(buttonElement).not.toHaveAttribute('aria-disabled');
+      expect(buttonElement).not.toHaveAttribute('disabled');
     });
 
     test('disables the add button when disableAddButton is true', () => {
       const wrapper = renderAttributeEditor({ ...defaultProps, disableAddButton: true });
       const buttonElement = wrapper.findAddButton().getElement();
-      expect(buttonElement).toHaveAttribute('aria-disabled');
-    });
-
-    test('allows the add button to be focused manually when disableAddButton is true', () => {
-      const ref: React.Ref<AttributeEditorProps.Ref> = React.createRef();
-      const wrapper = renderAttributeEditor({ ...defaultProps, ref, disableAddButton: true });
-      ref.current!.focusAddButton();
-      expect(wrapper.findAddButton().getElement()).toHaveFocus();
+      expect(buttonElement).toHaveAttribute('disabled');
     });
 
     test('has no aria-describedby if there is no additional info', () => {

src/attribute-editor/__tests__/attribute-editor.test.tsx

@@ -103,11 +103,6 @@ const InternalAttributeEditor = React.forwardRef(
         <InternalButton
           className={styles['add-button']}
           disabled={disableAddButton}
-          // Using aria-disabled="true" and tabindex="-1" instead of "disabled"
-          // because focus can be dynamically moved to this button by calling
-          // `focusAddButton()` on the ref.
-          __nativeAttributes={disableAddButton ? { tabIndex: -1 } : {}}
-          __focusable={true}
           onClick={onAddButtonClick}
           formAction="none"
           ref={addButtonRef}

src/attribute-editor/internal.tsx

@@ -283,7 +283,7 @@ describe('Tag Editor component', () => {
     test('is not disabled by default ', () => {
       const { wrapper } = renderTagEditor();
 
-      expect(wrapper.findAddButton().getElement()).not.toHaveAttribute('aria-disabled');
+      expect(wrapper.findAddButton().getElement()).not.toHaveAttribute('disabled');
     });
 
     test('is disabled when tag limit has been reached', () => {
@@ -292,7 +292,7 @@ describe('Tag Editor component', () => {
         tagLimit: 1,
       });
 
-      expect(wrapper.findAddButton().getElement()).toHaveAttribute('aria-disabled');
+      expect(wrapper.findAddButton().getElement()).toHaveAttribute('disabled');
     });
 
     test('is disabled when tag limit has been exceeded', () => {
@@ -304,7 +304,7 @@ describe('Tag Editor component', () => {
         tagLimit: 1,
       });
 
-      expect(wrapper.findAddButton().getElement()).toHaveAttribute('aria-disabled');
+      expect(wrapper.findAddButton().getElement()).toHaveAttribute('disabled');
     });
 
     test('adds an empty tag when there are no tags', () => {