fix: Fix controlled tools mode when drawers API is used (#1543)

pages/app-layout/runtime-drawers.page.tsx

@@ -6,6 +6,7 @@ import {
   ContentLayout,
   Header,
   HelpPanel,
+  Link,
   NonCancelableCustomEvent,
   SpaceBetween,
   SplitPanel,
@@ -71,7 +72,15 @@ export default function WithDrawers() {
         <ContentLayout
           header={
             <SpaceBetween size="m">
-              <Header variant="h1" description="Sometimes you need custom drawers to get the job done.">
+              <Header
+                variant="h1"
+                description="Sometimes you need custom drawers to get the job done."
+                info={
+                  <Link variant="info" onFollow={() => setIsToolsOpen(true)}>
+                    Info
+                  </Link>
+                }
+              >
                 Testing Custom Drawers!
               </Header>
 

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

@@ -0,0 +1,56 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+import { BasePageObject } from '@cloudscape-design/browser-test-tools/page-objects';
+import useBrowser from '@cloudscape-design/browser-test-tools/use-browser';
+import createWrapper from '../../../lib/components/test-utils/selectors';
+
+const wrapper = createWrapper().findAppLayout();
+
+for (const visualRefresh of [true, false]) {
+  describe(`visualRefresh=${visualRefresh}`, () => {
+    function setupTest(testFn: (page: BasePageObject) => Promise<void>) {
+      return useBrowser(async browser => {
+        const page = new BasePageObject(browser);
+
+        await browser.url(
+          `#/light/app-layout/runtime-drawers?${new URLSearchParams({
+            hasDrawers: 'false',
+            hasTools: 'true',
+            visualRefresh: `${visualRefresh}`,
+          }).toString()}`
+        );
+        await page.waitForVisible(wrapper.findDrawerTriggerById('security').toSelector(), true);
+        await testFn(page);
+      });
+    }
+
+    test(
+      'should switch between tools panel and runtime drawers',
+      setupTest(async page => {
+        await page.click(wrapper.findToolsToggle().toSelector());
+        await expect(page.getText(wrapper.findTools().getElement())).resolves.toContain('Here is some info for you!');
+
+        await page.click(wrapper.findDrawerTriggerById('security').toSelector());
+        await expect(page.getText(wrapper.findActiveDrawer().getElement())).resolves.toContain('I am runtime drawer');
+
+        await page.click(wrapper.findDrawerTriggerById('awsui-internal-tools').toSelector());
+        await expect(page.getText(wrapper.findTools().getElement())).resolves.toContain('Here is some info for you!');
+      })
+    );
+
+    test(
+      'should open and close tools via controlled mode',
+      setupTest(async page => {
+        const toolsContentSelector = wrapper.findTools().getElement();
+        await page.click(createWrapper().findHeader().findInfo().findLink().toSelector());
+        await expect(page.isDisplayed(toolsContentSelector)).resolves.toBe(true);
+        await expect(page.getText(wrapper.findActiveDrawer().getElement())).resolves.toContain(
+          'Here is some info for you!'
+        );
+
+        await page.click(wrapper.findToolsClose().toSelector());
+        await expect(page.isDisplayed(toolsContentSelector)).resolves.toBe(false);
+      })
+    );
+  });
+}

src/app-layout/__tests__/drawers.test.tsx

@@ -17,7 +17,7 @@ jest.mock('@cloudscape-design/component-toolkit', () => ({
   useContainerQuery: () => [100, () => {}],
 }));
 
-describeEachAppLayout(() => {
+describeEachAppLayout(size => {
   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);
@@ -50,14 +50,61 @@ describeEachAppLayout(() => {
     expect(wrapper.findDrawersTriggers()).toHaveLength(2);
   });
 
-  test('should respect toolsOpen property when merging into drawers', () => {
+  // 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} />);
 

src/app-layout/__tests__/runtime-drawers.test.tsx

@@ -8,7 +8,7 @@ import { TOOLS_DRAWER_ID } from '../../../lib/components/app-layout/utils/use-dr
 import { awsuiPlugins, awsuiPluginsInternal } from '../../../lib/components/internal/plugins/api';
 import { DrawerConfig } from '../../../lib/components/internal/plugins/controllers/drawers';
 import createWrapper from '../../../lib/components/test-utils/dom';
-import { singleDrawer } from './utils';
+import { describeEachAppLayout, singleDrawer } from './utils';
 
 beforeEach(() => {
   awsuiPluginsInternal.appLayout.clearRegisteredDrawers();
@@ -34,11 +34,11 @@ const drawerDefaults: DrawerConfig = {
   id: 'test',
   ariaLabels: {},
   trigger: { iconSvg: '' },
-  mountContent: () => {},
+  mountContent: container => (container.textContent = 'runtime drawer content'),
   unmountContent: () => {},
 };
 
-describe('Runtime drawers', () => {
+describeEachAppLayout(() => {
   test('does not render runtime drawers when it is explicitly disabled', async () => {
     awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
     const { wrapper } = await renderComponent(<AppLayout {...({ __disableRuntimeDrawers: true } as any)} />);
@@ -104,6 +104,66 @@ describe('Runtime drawers', () => {
     expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Tools content');
   });
 
+  test('allows controlled toolsOpen when runtime drawers exist', async () => {
+    const onToolsChange = jest.fn();
+    awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
+    const { wrapper, rerender } = await renderComponent(
+      <AppLayout tools="Tools content" toolsOpen={false} onToolsChange={event => onToolsChange(event.detail)} />
+    );
+    expect(wrapper.findDrawersTriggers()).toHaveLength(2);
+    expect(wrapper.findTools()).toBeFalsy();
+    expect(onToolsChange).not.toHaveBeenCalled();
+
+    rerender(<AppLayout tools="Tools content" toolsOpen={true} onToolsChange={event => onToolsChange(event.detail)} />);
+    expect(wrapper.findTools().getElement()).toHaveTextContent('Tools content');
+
+    wrapper.findToolsClose().click();
+    expect(onToolsChange).toHaveBeenCalledWith({ open: false });
+  });
+
+  test('allows closing tools in controlled mode when runtime drawers exist', async () => {
+    const onToolsChange = jest.fn();
+    awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
+    const { wrapper } = await renderComponent(
+      <AppLayout tools="Tools content" toolsOpen={true} onToolsChange={event => onToolsChange(event.detail)} />
+    );
+    wrapper.findToolsClose().click();
+    expect(onToolsChange).toHaveBeenCalledWith({ open: false });
+  });
+
+  test('allows controlled toolsOpen when another drawer is open', async () => {
+    const onToolsChange = jest.fn();
+    awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
+    const { wrapper, rerender } = await renderComponent(
+      <AppLayout tools="Tools content" toolsOpen={false} onToolsChange={event => onToolsChange(event.detail)} />
+    );
+
+    wrapper.findDrawerTriggerById(drawerDefaults.id)!.click();
+    expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('runtime drawer content');
+    expect(onToolsChange).toHaveBeenCalledWith({ open: false });
+
+    onToolsChange.mockClear();
+    rerender(<AppLayout tools="Tools content" toolsOpen={true} onToolsChange={event => onToolsChange(event.detail)} />);
+    expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Tools content');
+    expect(wrapper.findDrawerTriggerById(TOOLS_DRAWER_ID)!.getElement()).toHaveAttribute('aria-expanded', 'true');
+    expect(wrapper.findDrawerTriggerById(drawerDefaults.id)!.getElement()).toHaveAttribute('aria-expanded', 'false');
+    expect(onToolsChange).not.toHaveBeenCalled();
+  });
+
+  test('allows switching drawers when toolsOpen is controlled', async () => {
+    const onToolsChange = jest.fn();
+    awsuiPlugins.appLayout.registerDrawer(drawerDefaults);
+    const { wrapper } = await renderComponent(
+      <AppLayout tools="Tools content" toolsOpen={false} onToolsChange={event => onToolsChange(event.detail)} />
+    );
+    wrapper.findDrawerTriggerById(drawerDefaults.id)!.click();
+    expect(onToolsChange).toHaveBeenCalledWith({ open: false });
+    expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('runtime drawer content');
+
+    wrapper.findToolsToggle().click();
+    expect(onToolsChange).toHaveBeenCalledWith({ open: true });
+  });
+
   test('updates active drawer if multiple are registered', async () => {
     awsuiPlugins.appLayout.registerDrawer({
       ...drawerDefaults,
@@ -158,11 +218,7 @@ describe('Runtime drawers', () => {
   });
 
   test('updates active drawer id in controlled mode', async () => {
-    awsuiPlugins.appLayout.registerDrawer({
-      ...drawerDefaults,
-      mountContent: container => (container.textContent = 'runtime drawer content'),
-      defaultActive: true,
-    });
+    awsuiPlugins.appLayout.registerDrawer({ ...drawerDefaults, defaultActive: true });
     const onChange = jest.fn();
     const drawers: Required<InternalDrawerProps> = {
       drawers: {

src/app-layout/utils/use-drawers.ts

@@ -86,7 +86,7 @@ export function useDrawers(
   const [activeDrawerId, setActiveDrawerId] = useControllable(
     ownDrawers?.activeDrawerId,
     ownDrawers?.onChange,
-    !toolsProps.toolsHide && toolsProps.toolsOpen ? TOOLS_DRAWER_ID : undefined,
+    undefined,
     {
       componentName: 'AppLayout',
       controlledProp: 'activeDrawerId',
@@ -105,8 +105,12 @@ export function useDrawers(
   if (toolsDrawer && combinedDrawers.length > 0) {
     combinedDrawers.unshift(toolsDrawer);
   }
-  const activeDrawer = combinedDrawers.find(drawer => drawer.id === activeDrawerId);
-  const activeDrawerIdResolved = activeDrawer?.id; // only defined when corresponding drawer exists
+  // support toolsOpen in runtime-drawers-only mode
+  let activeDrawerIdResolved =
+    toolsProps.toolsOpen && (ownDrawers?.items ?? []).length === 0 ? TOOLS_DRAWER_ID : activeDrawerId;
+  const activeDrawer = combinedDrawers.find(drawer => drawer.id === activeDrawerIdResolved);
+  // ensure that id is only defined when the drawer exists
+  activeDrawerIdResolved = activeDrawer?.id;
 
   function onActiveDrawerResize({ id, size }: { id: string; size: number }) {
     setDrawerSizes(oldSizes => ({ ...oldSizes, [id]: size }));

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

@@ -118,7 +118,10 @@ function ActiveDrawer() {
           })}
           formAction="none"
           iconName={isMobile ? 'close' : 'angle-right'}
-          onClick={() => (activeDrawerId ? handleDrawersClick(activeDrawerId ?? null) : handleToolsClick(false))}
+          onClick={() => {
+            handleDrawersClick(activeDrawerId ?? undefined);
+            handleToolsClick(false);
+          }}
           ref={isToolsDrawer ? toolsRefs.close : drawersRefs.close}
           variant="icon"
         />
@@ -195,6 +198,15 @@ function DesktopTriggers() {
   const { visibleItems, overflowItems } = splitItems(drawers, getIndexOfOverflowItem(), activeDrawerId);
   const overflowMenuHasBadge = !!overflowItems.find(item => item.badge);
 
+  function handleItemClick(itemId: string | undefined) {
+    if (itemId === TOOLS_DRAWER_ID) {
+      handleToolsClick(!isToolsOpen, true);
+    } else {
+      handleToolsClick(false, true);
+    }
+    handleDrawersClick(itemId);
+  }
+
   return (
     <aside
       className={clsx(
@@ -230,10 +242,7 @@ function DesktopTriggers() {
               iconName={item.trigger.iconName}
               iconSvg={item.trigger.iconSvg}
               key={item.id}
-              onClick={() => {
-                isToolsOpen && handleToolsClick(!isToolsOpen, true);
-                handleDrawersClick(item.id);
-              }}
+              onClick={() => handleItemClick(item.id)}
               ref={item.id === previousActiveDrawerId.current ? drawersRefs.toggle : undefined}
               selected={item.id === activeDrawerId}
               badge={item.badge}
@@ -258,7 +267,7 @@ function DesktopTriggers() {
               />
             )}
             onItemClick={({ detail }) => {
-              handleDrawersClick(detail.id);
+              handleItemClick(detail.id);
             }}
           />
         )}
@@ -292,6 +301,8 @@ export function MobileTriggers() {
     drawersAriaLabel,
     drawersOverflowAriaLabel,
     drawersRefs,
+    isToolsOpen,
+    handleToolsClick,
     handleDrawersClick,
     hasDrawerViewportOverlay,
     isMobile,
@@ -311,6 +322,15 @@ export function MobileTriggers() {
 
   const { visibleItems, overflowItems } = splitItems(drawers, splitIndex, activeDrawerId, true);
 
+  function handleItemClick(itemId: string | undefined) {
+    if (itemId === TOOLS_DRAWER_ID) {
+      handleToolsClick(!isToolsOpen, true);
+    } else {
+      handleToolsClick(false, true);
+    }
+    handleDrawersClick(itemId);
+  }
+
   return (
     <aside
       aria-hidden={hasDrawerViewportOverlay}
@@ -339,7 +359,7 @@ export function MobileTriggers() {
           iconSvg={item.trigger.iconSvg}
           badge={item.badge}
           key={item.id}
-          onClick={() => handleDrawersClick(item.id)}
+          onClick={() => handleItemClick(item.id)}
           variant="icon"
           __nativeAttributes={{ 'aria-haspopup': true, 'data-testid': `awsui-app-layout-trigger-${item.id}` }}
         />
@@ -349,7 +369,7 @@ export function MobileTriggers() {
           items={overflowItems}
           ariaLabel={drawersOverflowAriaLabel}
           onItemClick={({ detail }) => {
-            handleDrawersClick(detail.id);
+            handleItemClick(detail.id);
           }}
         />
       )}