From ca69b71a7bdc4bdf16071f3479ba1a92a346f648 Mon Sep 17 00:00:00 2001 From: Boris Serdiuk Date: Wed, 13 Sep 2023 13:06:54 +0200 Subject: [PATCH] fix: Fix controlled tools mode when drawers API is used (#1543) --- pages/app-layout/runtime-drawers.page.tsx | 11 ++- .../__integ__/runtime-drawers.test.ts | 56 +++++++++++++++ src/app-layout/__tests__/drawers.test.tsx | 51 ++++++++++++- .../__tests__/runtime-drawers.test.tsx | 72 ++++++++++++++++--- src/app-layout/utils/use-drawers.ts | 10 ++- src/app-layout/visual-refresh/drawers.tsx | 36 +++++++--- 6 files changed, 214 insertions(+), 22 deletions(-) create mode 100644 src/app-layout/__integ__/runtime-drawers.test.ts diff --git a/pages/app-layout/runtime-drawers.page.tsx b/pages/app-layout/runtime-drawers.page.tsx index afe36a29ea..560837de87 100644 --- a/pages/app-layout/runtime-drawers.page.tsx +++ b/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() { -
+
setIsToolsOpen(true)}> + Info + + } + > Testing Custom Drawers!
diff --git a/src/app-layout/__integ__/runtime-drawers.test.ts b/src/app-layout/__integ__/runtime-drawers.test.ts new file mode 100644 index 0000000000..1bf803bb1d --- /dev/null +++ b/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) { + 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); + }) + ); + }); +} diff --git a/src/app-layout/__tests__/drawers.test.tsx b/src/app-layout/__tests__/drawers.test.tsx index 0012bfb828..6fc63f7798 100644 --- a/src/app-layout/__tests__/drawers.test.tsx +++ b/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(); expect(wrapper.findDrawersTriggers()).toHaveLength(1); @@ -50,7 +50,8 @@ 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(); expect(wrapper.findDrawerTriggerById(TOOLS_DRAWER_ID)!.getElement()).toHaveAttribute('aria-expanded', 'true'); @@ -58,6 +59,52 @@ describeEachAppLayout(() => { 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( + 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( + 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( + onToolsChange(event.detail)} + {...singleDrawerOpen} + /> + ); + wrapper.findToolsToggle().click(); + expect(onToolsChange).toHaveBeenCalledWith({ open: true }); + }); + } + test('activeDrawerId has priority over toolsOpen', () => { const { wrapper } = renderComponent(); diff --git a/src/app-layout/__tests__/runtime-drawers.test.tsx b/src/app-layout/__tests__/runtime-drawers.test.tsx index 641e9e5d0f..15c16fe501 100644 --- a/src/app-layout/__tests__/runtime-drawers.test.tsx +++ b/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(); @@ -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( + onToolsChange(event.detail)} /> + ); + expect(wrapper.findDrawersTriggers()).toHaveLength(2); + expect(wrapper.findTools()).toBeFalsy(); + expect(onToolsChange).not.toHaveBeenCalled(); + + rerender( 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( + 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( + onToolsChange(event.detail)} /> + ); + + wrapper.findDrawerTriggerById(drawerDefaults.id)!.click(); + expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('runtime drawer content'); + expect(onToolsChange).toHaveBeenCalledWith({ open: false }); + + onToolsChange.mockClear(); + rerender( 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( + 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 = { drawers: { diff --git a/src/app-layout/utils/use-drawers.ts b/src/app-layout/utils/use-drawers.ts index 8d0b1379f8..d8951a5db0 100644 --- a/src/app-layout/utils/use-drawers.ts +++ b/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 })); diff --git a/src/app-layout/visual-refresh/drawers.tsx b/src/app-layout/visual-refresh/drawers.tsx index c2b59cf752..9fa13924ab 100644 --- a/src/app-layout/visual-refresh/drawers.tsx +++ b/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 (