chore: Disable implicit tools integration into drawers list

cloudscape-design · Sep 28, 2023 · d509db0 · d509db0
1 parent b7d291c
commit d509db0
Show file tree

Hide file tree

Showing 12 changed files with 278 additions and 245 deletions.
diff --git a/pages/app-layout/with-drawers-empty.page.tsx b/pages/app-layout/with-drawers-empty.page.tsx
@@ -0,0 +1,58 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+import React from 'react';
+import { AppLayout, ContentLayout, Header, SplitPanel } from '~components';
+import appLayoutLabels from './utils/labels';
+import { Breadcrumbs, Containers } from './utils/content-blocks';
+import ScreenshotArea from '../utils/screenshot-area';
+
+export default function WithDrawers() {
+  const drawers = {
+    drawers: {
+      items: [],
+    },
+  };
+
+  return (
+    <ScreenshotArea gutters={false}>
+      <AppLayout
+        ariaLabels={appLayoutLabels}
+        breadcrumbs={<Breadcrumbs />}
+        content={
+          <ContentLayout
+            header={
+              <Header
+                variant="h1"
+                description="This page renders an empty drawers array, which should not occupy any space"
+              >
+                Page without drawers
+              </Header>
+            }
+          >
+            <Containers />
+          </ContentLayout>
+        }
+        splitPanel={
+          <SplitPanel
+            header="Split panel header"
+            i18nStrings={{
+              preferencesTitle: 'Preferences',
+              preferencesPositionLabel: 'Split panel position',
+              preferencesPositionDescription: 'Choose the default split panel position for the service.',
+              preferencesPositionSide: 'Side',
+              preferencesPositionBottom: 'Bottom',
+              preferencesConfirm: 'Confirm',
+              preferencesCancel: 'Cancel',
+              closeButtonAriaLabel: 'Close panel',
+              openButtonAriaLabel: 'Open panel',
+              resizeHandleAriaLabel: 'Slider',
+            }}
+          >
+            This is the Split Panel!
+          </SplitPanel>
+        }
+        {...drawers}
+      />
+    </ScreenshotArea>
+  );
+}
diff --git a/src/app-layout/__tests__/drawers.test.tsx b/src/app-layout/__tests__/drawers.test.tsx
@@ -1,12 +1,11 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 import React from 'react';
-import { describeEachAppLayout, renderComponent, singleDrawer, manyDrawers, singleDrawerOpen } from './utils';
+import { describeEachAppLayout, renderComponent, singleDrawer, manyDrawers } from './utils';
 import createWrapper from '../../../lib/components/test-utils/dom';
 
 import { render } from '@testing-library/react';
 import AppLayout from '../../../lib/components/app-layout';
-import { TOOLS_DRAWER_ID } from '../../../lib/components/app-layout/utils/use-drawers';
 
 jest.mock('../../../lib/components/internal/hooks/use-mobile', () => ({
   useMobile: jest.fn().mockReturnValue(true),
@@ -17,7 +16,7 @@ jest.mock('@cloudscape-design/component-toolkit', () => ({
   useContainerQuery: () => [100, () => {}],
 }));
 
-describeEachAppLayout(size => {
+describeEachAppLayout(() => {
   test(`should not render drawer when it is not defined`, () => {
     const { wrapper, rerender } = renderComponent(<AppLayout contentType="form" toolsHide={true} {...singleDrawer} />);
     expect(wrapper.findDrawersTriggers()).toHaveLength(1);
@@ -35,82 +34,14 @@ describeEachAppLayout(size => {
     const { wrapper } = renderComponent(<AppLayout contentType="form" {...emptyDrawerItems} />);
 
     expect(wrapper.findDrawersTriggers()).toHaveLength(0);
-    expect(wrapper.findToolsToggle()).toBeTruthy();
+    expect(wrapper.findToolsToggle()).toBeFalsy();
   });
 
-  test('should apply drawers treatment to the tools if at least one other drawer is provided', () => {
-    const { wrapper } = renderComponent(<AppLayout contentType="form" {...singleDrawer} />);
-    expect(wrapper.findDrawersTriggers()).toHaveLength(2);
-    expect(wrapper.findToolsToggle()).toBeTruthy();
-  });
-
-  test('renders drawers with the tools', () => {
+  test('ignores tools when drawers API is used', () => {
     const { wrapper } = renderComponent(<AppLayout tools="Test" {...singleDrawer} />);
 
-    expect(wrapper.findDrawersTriggers()).toHaveLength(2);
-  });
-
-  // this behavior is no longer supported for compatibility with runtime API
-  test.skip('should respect toolsOpen property when merging into drawers', () => {
-    const { wrapper } = renderComponent(<AppLayout tools="Tools content" toolsOpen={true} {...singleDrawer} />);
-
-    expect(wrapper.findDrawerTriggerById(TOOLS_DRAWER_ID)!.getElement()).toHaveAttribute('aria-expanded', 'true');
-    expect(wrapper.findDrawerTriggerById('security')!.getElement()).toHaveAttribute('aria-expanded', 'false');
-    expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Tools content');
-  });
-
-  test('should fire tools change event when closing tools panel while drawers are present', () => {
-    const onToolsChange = jest.fn();
-    const { wrapper } = renderComponent(
-      <AppLayout tools="Tools content" onToolsChange={event => onToolsChange(event.detail)} {...singleDrawer} />
-    );
-
-    wrapper.findToolsToggle().click();
-    expect(onToolsChange).toHaveBeenCalledWith({ open: true });
-
-    onToolsChange.mockClear();
-    wrapper.findToolsClose().click();
-    expect(onToolsChange).toHaveBeenCalledWith({ open: false });
-  });
-
-  // drawers render full screen on mobile sizes, switching open drawers does not work there
-  if (size === 'desktop') {
-    test('should fire tools close event when switching from tools to another drawer', () => {
-      const onToolsChange = jest.fn();
-      const { wrapper } = renderComponent(
-        <AppLayout
-          tools="Tools content"
-          toolsOpen={true}
-          onToolsChange={event => onToolsChange(event.detail)}
-          {...singleDrawer}
-        />
-      );
-
-      wrapper.findDrawerTriggerById('security')!.click();
-      expect(onToolsChange).toHaveBeenCalledWith({ open: false });
-    });
-
-    test('should fire tools open event when switching from another drawer to tools', () => {
-      const onToolsChange = jest.fn();
-      const { wrapper } = renderComponent(
-        <AppLayout
-          tools="Tools content"
-          toolsOpen={false}
-          onToolsChange={event => onToolsChange(event.detail)}
-          {...singleDrawerOpen}
-        />
-      );
-      wrapper.findToolsToggle().click();
-      expect(onToolsChange).toHaveBeenCalledWith({ open: true });
-    });
-  }
-
-  test('activeDrawerId has priority over toolsOpen', () => {
-    const { wrapper } = renderComponent(<AppLayout tools="Tools content" toolsOpen={true} {...singleDrawerOpen} />);
-
-    expect(wrapper.findDrawerTriggerById(TOOLS_DRAWER_ID)!.getElement()).toHaveAttribute('aria-expanded', 'false');
-    expect(wrapper.findDrawerTriggerById('security')!.getElement()).toHaveAttribute('aria-expanded', 'true');
-    expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security');
+    expect(wrapper.findToolsToggle()).toBeFalsy();
+    expect(wrapper.findDrawersTriggers()).toHaveLength(1);
   });
 
   test('should open active drawer on click of overflow item', () => {

diff --git a/src/app-layout/__tests__/runtime-drawers.test.tsx b/src/app-layout/__tests__/runtime-drawers.test.tsx
@@ -180,6 +180,30 @@ describeEachAppLayout(size => {
       expect(onToolsChange).toHaveBeenCalledWith({ open: false });
       expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('runtime drawer content');
 
+      onToolsChange.mockReset();
+      wrapper.findToolsToggle().click();
+      expect(onToolsChange).toHaveBeenCalledWith({ open: true });
+    });
+
+    test('should fire tools close event when switching from tools to another drawer', async () => {
+      awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
+      const onToolsChange = jest.fn();
+      const { wrapper } = await renderComponent(
+        <AppLayout tools="Tools content" toolsOpen={true} onToolsChange={event => onToolsChange(event.detail)} />
+      );
+
+      wrapper.findDrawerTriggerById(drawerDefaults.id)!.click();
+      expect(onToolsChange).toHaveBeenCalledWith({ open: false });
+    });
+
+    test('should fire tools open event when switching from another drawer to tools', async () => {
+      awsuiPlugins.appLayout.registerDrawer({ ...drawerDefaults, defaultActive: true });
+      const onToolsChange = jest.fn();
+      const { wrapper } = await renderComponent(
+        <AppLayout tools="Tools content" toolsOpen={false} onToolsChange={event => onToolsChange(event.detail)} />
+      );
+      expect(wrapper.findActiveDrawer()!.getElement()).toBeInTheDocument();
+
       wrapper.findToolsToggle().click();
       expect(onToolsChange).toHaveBeenCalledWith({ open: true });
     });
@@ -356,7 +380,7 @@ describeEachAppLayout(size => {
       ]);
     });
 
-    test('allows mixing static and runtime drawers', async () => {
+    test('ignores tools when drawers are present', async () => {
       awsuiPlugins.appLayout.registerDrawer({ ...drawerDefaults, id: 'aaa', ariaLabels: { triggerButton: 'aaa' } });
       awsuiPlugins.appLayout.registerDrawer({
         ...drawerDefaults,
@@ -379,12 +403,26 @@ describeEachAppLayout(size => {
         <AppLayout {...(drawers as any)} ariaLabels={{ toolsToggle: 'tools toggle' }} />
       );
       expect(wrapper.findDrawersTriggers().map(trigger => trigger.getElement().getAttribute('aria-label'))).toEqual([
-        'tools toggle',
         'bbb',
         'ddd',
         'aaa',
         'ccc',
       ]);
     });
   });
+
+  test('should fire tools change event when closing tools panel while drawers are present', async () => {
+    const onToolsChange = jest.fn();
+    awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
+    const { wrapper } = await renderComponent(
+      <AppLayout tools="Tools content" onToolsChange={event => onToolsChange(event.detail)} />
+    );
+
+    wrapper.findToolsToggle().click();
+    expect(onToolsChange).toHaveBeenCalledWith({ open: true });
+
+    onToolsChange.mockClear();
+    wrapper.findToolsClose().click();
+    expect(onToolsChange).toHaveBeenCalledWith({ open: false });
+  });
 });
diff --git a/src/app-layout/drawer/index.tsx b/src/app-layout/drawer/index.tsx
@@ -110,7 +110,7 @@ export const Drawer = React.forwardRef(
           style={{ width: drawerContentWidth, top: topOffset, bottom: bottomOffset }}
           className={clsx(styles['drawer-content'], styles['drawer-content-clickable'], contentClassName)}
         >
-          {!isMobile && regularOpenButton}
+          {!isMobile && !drawers && regularOpenButton}
           {resizeHandle}
           <TagName aria-label={mainLabel} aria-hidden={!isOpen}>
             <CloseButton
@@ -225,11 +225,10 @@ export const DrawerTriggersBar = ({ isMobile, topOffset, bottomOffset, drawers }
                 return (
                   <DrawerTrigger
                     key={index}
-                    testUtilsClassName={
-                      item.id === TOOLS_DRAWER_ID
-                        ? clsx(testutilStyles['drawers-trigger'], testutilStyles['tools-toggle'])
-                        : testutilStyles['drawers-trigger']
-                    }
+                    testUtilsClassName={clsx(
+                      testutilStyles['drawers-trigger'],
+                      item.id === TOOLS_DRAWER_ID && testutilStyles['tools-toggle']
+                    )}
                     ariaExpanded={drawers?.activeDrawerId === item.id}
                     ariaLabel={item.ariaLabels?.triggerButton}
                     ariaControls={drawers?.activeDrawerId === item.id ? item.id : undefined}