From eda44495d4d10aa7eabc93d019c104b23b66c1e1 Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Tue, 8 Oct 2024 16:38:20 +0200 Subject: [PATCH 1/6] Revert "fix: Partially controllable tools (#2793)" This reverts commit 227dd63ea3e3706353ff3e00de4e03c254ca47b4. --- src/app-layout/__tests__/tools.test.tsx | 22 +------------------ src/app-layout/utils/use-drawers.ts | 4 +--- .../visual-refresh-toolbar/index.tsx | 2 -- .../visual-refresh-toolbar/interfaces.ts | 2 -- .../toolbar/drawer-triggers.tsx | 11 +++------- .../visual-refresh-toolbar/toolbar/index.tsx | 4 ---- 6 files changed, 5 insertions(+), 40 deletions(-) diff --git a/src/app-layout/__tests__/tools.test.tsx b/src/app-layout/__tests__/tools.test.tsx index 1105ac54db..2a28f2c7f7 100644 --- a/src/app-layout/__tests__/tools.test.tsx +++ b/src/app-layout/__tests__/tools.test.tsx @@ -6,7 +6,7 @@ import { act, waitFor } from '@testing-library/react'; import { describeEachAppLayout, renderComponent, isDrawerClosed } from './utils'; import AppLayout, { AppLayoutProps } from '../../../lib/components/app-layout'; -describeEachAppLayout({ themes: ['classic', 'refresh', 'refresh-toolbar'] }, ({ theme }) => { +describeEachAppLayout({ themes: ['classic', 'refresh', 'refresh-toolbar'] }, () => { test('opens tools drawer', () => { let ref: AppLayoutProps.Ref | null = null; const { wrapper } = renderComponent( (ref = newRef)} />); @@ -56,24 +56,4 @@ describeEachAppLayout({ themes: ['classic', 'refresh', 'refresh-toolbar'] }, ({ expect(wrapper.find('#custom-button')!.getElement()).toEqual(document.activeElement); }); }); - - test('should not open partially controllable tools', () => { - const { wrapper } = renderComponent( - Click me} toolsOpen={false} /> - ); - - wrapper.findToolsToggle()!.click(); - - if (theme === 'refresh-toolbar') { - expect(wrapper.findTools()).toBeFalsy(); - } - - if (theme === 'refresh') { - expect(wrapper.findTools().getElement()).toHaveAttribute('aria-hidden', 'true'); - } - - if (theme === 'classic') { - expect(wrapper.findTools().getElement().style).not.toContain('width'); - } - }); }); diff --git a/src/app-layout/utils/use-drawers.ts b/src/app-layout/utils/use-drawers.ts index df4ae9668b..fdce5fd12e 100644 --- a/src/app-layout/utils/use-drawers.ts +++ b/src/app-layout/utils/use-drawers.ts @@ -198,9 +198,7 @@ export function useDrawers( } function onActiveDrawerChange(newDrawerId: string | null) { - if (newDrawerId !== TOOLS_DRAWER_ID) { - setActiveDrawerId(newDrawerId); - } + setActiveDrawerId(newDrawerId); if (newDrawerId) { onAddNewActiveDrawer?.(newDrawerId); } diff --git a/src/app-layout/visual-refresh-toolbar/index.tsx b/src/app-layout/visual-refresh-toolbar/index.tsx index ceb8832a03..e2c35c56db 100644 --- a/src/app-layout/visual-refresh-toolbar/index.tsx +++ b/src/app-layout/visual-refresh-toolbar/index.tsx @@ -332,8 +332,6 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef void; onActiveDrawerResize: (detail: { id: string; size: number }) => void; onActiveGlobalDrawersChange: (newDrawerId: string) => void; - toolsOpen: boolean; - onToolsToggle: (value: boolean) => void; } diff --git a/src/app-layout/visual-refresh-toolbar/toolbar/drawer-triggers.tsx b/src/app-layout/visual-refresh-toolbar/toolbar/drawer-triggers.tsx index b7e5ec1c39..1c8bf62ffb 100644 --- a/src/app-layout/visual-refresh-toolbar/toolbar/drawer-triggers.tsx +++ b/src/app-layout/visual-refresh-toolbar/toolbar/drawer-triggers.tsx @@ -38,9 +38,6 @@ interface DrawerTriggersProps { globalDrawers: ReadonlyArray; onActiveGlobalDrawersChange?: (newDrawerId: string) => void; - toolsOpen: boolean; - onToolsToggle: (value: boolean) => void; - splitPanelOpen?: boolean; splitPanelPosition?: AppLayoutProps.SplitPanelPreferences['position']; splitPanelToggleProps: SplitPanelToggleProps | undefined; @@ -65,7 +62,6 @@ export function DrawerTriggers({ globalDrawers, globalDrawersFocusControl, onActiveGlobalDrawersChange, - toolsOpen, }: DrawerTriggersProps) { const isMobile = useMobile(); const hasMultipleTriggers = drawers.length > 1; @@ -156,12 +152,11 @@ export function DrawerTriggers({ )} {visibleItems.slice(0, globalDrawersStartIndex).map(item => { const isForPreviousActiveDrawer = previousActiveLocalDrawerId?.current === item.id; - const isActive = item.id === TOOLS_DRAWER_ID ? toolsOpen : item.id === activeDrawerId; return ( onActiveDrawerChange?.(activeDrawerId !== item.id ? item.id : null)} ref={item.id === previousActiveLocalDrawerId.current ? drawersFocusRef : undefined} - selected={isActive} + selected={item.id === activeDrawerId} badge={item.badge} testId={`awsui-app-layout-trigger-${item.id}`} hasTooltip={true} diff --git a/src/app-layout/visual-refresh-toolbar/toolbar/index.tsx b/src/app-layout/visual-refresh-toolbar/toolbar/index.tsx index 8351a00760..9aa7c1732c 100644 --- a/src/app-layout/visual-refresh-toolbar/toolbar/index.tsx +++ b/src/app-layout/visual-refresh-toolbar/toolbar/index.tsx @@ -97,8 +97,6 @@ export function AppLayoutToolbarImplementation({ setToolbarState, setToolbarHeight, globalDrawersFocusControl, - toolsOpen, - onToolsToggle, } = appLayoutInternals; const { ariaLabels, @@ -224,8 +222,6 @@ export function AppLayoutToolbarImplementation({ globalDrawers={globalDrawers?.filter(item => !!item.trigger) ?? []} activeGlobalDrawersIds={activeGlobalDrawersIds ?? []} onActiveGlobalDrawersChange={onActiveGlobalDrawersChange} - toolsOpen={toolsOpen} - onToolsToggle={onToolsToggle} /> )} From b178f6fa9c97a5b1f5a21b8d353332aad7e1a06c Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Tue, 15 Oct 2024 12:06:17 +0200 Subject: [PATCH 2/6] fix: Runtime drawers defaultActive behavior --- .../__tests__/runtime-drawers.test.tsx | 31 +++++++++++++++++++ src/app-layout/utils/use-drawers.ts | 22 +++++++------ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/app-layout/__tests__/runtime-drawers.test.tsx b/src/app-layout/__tests__/runtime-drawers.test.tsx index c8d954e51f..11d5d2fc2e 100644 --- a/src/app-layout/__tests__/runtime-drawers.test.tsx +++ b/src/app-layout/__tests__/runtime-drawers.test.tsx @@ -1042,5 +1042,36 @@ describe('toolbar mode only features', () => { expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('runtime drawer content'); }); + + test('dynamically registered drawers with defaultActive: true should open even when there are already open drawer(s) on the page', async () => { + const { wrapper, globalDrawersWrapper } = await renderComponent(); + + wrapper.findDrawerTriggerById('security')!.click(); + expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); + + awsuiPlugins.appLayout.registerDrawer({ + ...drawerDefaults, + id: 'global1', + type: 'global', + defaultActive: true, + }); + + await delay(); + + expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); + + awsuiPlugins.appLayout.registerDrawer({ + ...drawerDefaults, + id: 'global2', + type: 'global', + defaultActive: true, + }); + + await delay(); + + expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); + expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); + expect(globalDrawersWrapper.findDrawerById('global2')!.isActive()).toBe(true); + }); }); }); diff --git a/src/app-layout/utils/use-drawers.ts b/src/app-layout/utils/use-drawers.ts index fdce5fd12e..116ebe83a2 100644 --- a/src/app-layout/utils/use-drawers.ts +++ b/src/app-layout/utils/use-drawers.ts @@ -63,8 +63,10 @@ function useRuntimeDrawers( const onLocalDrawerChangeStable = useStableCallback(onActiveDrawerChange); const onGlobalDrawersChangeStable = useStableCallback(onActiveGlobalDrawersChange); - const drawersWereOpenRef = useRef(false); - drawersWereOpenRef.current = drawersWereOpenRef.current || !!activeDrawerId || !!activeGlobalDrawersIds.length; + const localDrawerWasOpenRef = useRef(false); + localDrawerWasOpenRef.current = localDrawerWasOpenRef.current || !!activeDrawerId; + const activeGlobalDrawersIdsStable = useRef>([]); + activeGlobalDrawersIdsStable.current = activeGlobalDrawersIds; useEffect(() => { if (disableRuntimeDrawers) { @@ -74,23 +76,25 @@ function useRuntimeDrawers( const localDrawers = drawers.filter(drawer => drawer.type !== 'global'); const globalDrawers = drawers.filter(drawer => drawer.type === 'global'); setRuntimeDrawers(convertRuntimeDrawers(localDrawers, globalDrawers)); - if (!drawersWereOpenRef.current) { + if (!localDrawerWasOpenRef.current) { const defaultActiveLocalDrawer = sortByPriority(localDrawers).find(drawer => drawer.defaultActive); if (defaultActiveLocalDrawer) { onLocalDrawerChangeStable(defaultActiveLocalDrawer.id); } - - const defaultActiveGlobalDrawers = sortByPriority(globalDrawers).filter(drawer => drawer.defaultActive); - defaultActiveGlobalDrawers.forEach(drawer => { - onGlobalDrawersChangeStable(drawer.id); - }); } + + const defaultActiveGlobalDrawers = sortByPriority(globalDrawers).filter( + drawer => !activeGlobalDrawersIdsStable.current.includes(drawer.id) && drawer.defaultActive + ); + defaultActiveGlobalDrawers.forEach(drawer => { + onGlobalDrawersChangeStable(drawer.id); + }); }); return () => { unsubscribe(); setRuntimeDrawers({ localBefore: [], localAfter: [], global: [] }); }; - }, [disableRuntimeDrawers, onGlobalDrawersChangeStable, onLocalDrawerChangeStable]); + }, [disableRuntimeDrawers, onGlobalDrawersChangeStable, onLocalDrawerChangeStable, activeGlobalDrawersIdsStable]); useEffect(() => { const unsubscribe = awsuiPluginsInternal.appLayout.onDrawerOpened(drawerId => { From bdf2181924e7cedece7fba354ae1d0f13bfe698c Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Tue, 15 Oct 2024 17:58:32 +0200 Subject: [PATCH 3/6] fix: Small fixes --- src/app-layout/utils/use-drawers.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app-layout/utils/use-drawers.ts b/src/app-layout/utils/use-drawers.ts index 116ebe83a2..9d02b9432a 100644 --- a/src/app-layout/utils/use-drawers.ts +++ b/src/app-layout/utils/use-drawers.ts @@ -65,8 +65,8 @@ function useRuntimeDrawers( const localDrawerWasOpenRef = useRef(false); localDrawerWasOpenRef.current = localDrawerWasOpenRef.current || !!activeDrawerId; - const activeGlobalDrawersIdsStable = useRef>([]); - activeGlobalDrawersIdsStable.current = activeGlobalDrawersIds; + const activeGlobalDrawersIdsRef = useRef>([]); + activeGlobalDrawersIdsRef.current = activeGlobalDrawersIds; useEffect(() => { if (disableRuntimeDrawers) { @@ -84,7 +84,7 @@ function useRuntimeDrawers( } const defaultActiveGlobalDrawers = sortByPriority(globalDrawers).filter( - drawer => !activeGlobalDrawersIdsStable.current.includes(drawer.id) && drawer.defaultActive + drawer => !activeGlobalDrawersIdsRef.current.includes(drawer.id) && drawer.defaultActive ); defaultActiveGlobalDrawers.forEach(drawer => { onGlobalDrawersChangeStable(drawer.id); @@ -94,7 +94,7 @@ function useRuntimeDrawers( unsubscribe(); setRuntimeDrawers({ localBefore: [], localAfter: [], global: [] }); }; - }, [disableRuntimeDrawers, onGlobalDrawersChangeStable, onLocalDrawerChangeStable, activeGlobalDrawersIdsStable]); + }, [disableRuntimeDrawers, onGlobalDrawersChangeStable, onLocalDrawerChangeStable]); useEffect(() => { const unsubscribe = awsuiPluginsInternal.appLayout.onDrawerOpened(drawerId => { From ec56ee906be5d54428ef7eb509b3de19e0d94e85 Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Wed, 16 Oct 2024 12:26:31 +0200 Subject: [PATCH 4/6] feat: Prevent defaultActive runtime drawers from opening if the maximum limit is reached or if there are already global drawer(s) opened by user action --- .../__tests__/runtime-drawers.test.tsx | 114 +++++++++++++++--- src/app-layout/utils/use-drawers.ts | 8 ++ 2 files changed, 102 insertions(+), 20 deletions(-) diff --git a/src/app-layout/__tests__/runtime-drawers.test.tsx b/src/app-layout/__tests__/runtime-drawers.test.tsx index 11d5d2fc2e..df8a50975c 100644 --- a/src/app-layout/__tests__/runtime-drawers.test.tsx +++ b/src/app-layout/__tests__/runtime-drawers.test.tsx @@ -1043,35 +1043,109 @@ describe('toolbar mode only features', () => { expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('runtime drawer content'); }); - test('dynamically registered drawers with defaultActive: true should open even when there are already open drawer(s) on the page', async () => { - const { wrapper, globalDrawersWrapper } = await renderComponent(); + describe('dynamically registered drawers with defaultActive: true', () => { + test('should open if there are already open local drawer on the page', async () => { + const { wrapper, globalDrawersWrapper } = await renderComponent(); - wrapper.findDrawerTriggerById('security')!.click(); - expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); + wrapper.findDrawerTriggerById('security')!.click(); + expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); - awsuiPlugins.appLayout.registerDrawer({ - ...drawerDefaults, - id: 'global1', - type: 'global', - defaultActive: true, + awsuiPlugins.appLayout.registerDrawer({ + ...drawerDefaults, + id: 'global1', + type: 'global', + defaultActive: true, + }); + + await delay(); + + expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); + + awsuiPlugins.appLayout.registerDrawer({ + ...drawerDefaults, + id: 'global2', + type: 'global', + defaultActive: true, + }); + + await delay(); + + expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); + expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); + expect(globalDrawersWrapper.findDrawerById('global2')!.isActive()).toBe(true); }); - await delay(); + test('should not open if there are already maximum global drawers opened on the page', async () => { + const { wrapper, globalDrawersWrapper } = await renderComponent(); - expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); + wrapper.findDrawerTriggerById('security')!.click(); + expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); - awsuiPlugins.appLayout.registerDrawer({ - ...drawerDefaults, - id: 'global2', - type: 'global', - defaultActive: true, + awsuiPlugins.appLayout.registerDrawer({ + ...drawerDefaults, + id: 'global1', + type: 'global', + defaultActive: true, + }); + + await delay(); + + expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); + + awsuiPlugins.appLayout.registerDrawer({ + ...drawerDefaults, + id: 'global2', + type: 'global', + defaultActive: true, + }); + + await delay(); + + expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); + expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); + expect(globalDrawersWrapper.findDrawerById('global2')!.isActive()).toBe(true); }); - await delay(); + test('should not open if the maximum number (2) of global drawers is already open on the page', async () => { + const { wrapper, globalDrawersWrapper } = await renderComponent(); + + wrapper.findDrawerTriggerById('security')!.click(); + expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); + + awsuiPlugins.appLayout.registerDrawer({ + ...drawerDefaults, + id: 'global1', + type: 'global', + defaultActive: true, + }); - expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); - expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); - expect(globalDrawersWrapper.findDrawerById('global2')!.isActive()).toBe(true); + await delay(); + + expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); + + awsuiPlugins.appLayout.registerDrawer({ + ...drawerDefaults, + id: 'global2', + type: 'global', + defaultActive: true, + }); + + await delay(); + + // this drawer should not open because there are already two global drawers open on the page, which is the maximum limit + awsuiPlugins.appLayout.registerDrawer({ + ...drawerDefaults, + id: 'global3', + type: 'global', + defaultActive: true, + }); + + await delay(); + + expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); + expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); + expect(globalDrawersWrapper.findDrawerById('global2')!.isActive()).toBe(true); + }); }); }); }); diff --git a/src/app-layout/utils/use-drawers.ts b/src/app-layout/utils/use-drawers.ts index 9d02b9432a..19bd196c00 100644 --- a/src/app-layout/utils/use-drawers.ts +++ b/src/app-layout/utils/use-drawers.ts @@ -83,6 +83,14 @@ function useRuntimeDrawers( } } + const drawersNotActiveByDefault = globalDrawers.filter(drawer => !drawer.defaultActive); + const hasDrawersOpenByUserAction = drawersNotActiveByDefault.find(drawer => + activeGlobalDrawersIdsRef.current.includes(drawer.id) + ); + if (hasDrawersOpenByUserAction || activeGlobalDrawersIdsRef.current.length === DRAWERS_LIMIT) { + return; + } + const defaultActiveGlobalDrawers = sortByPriority(globalDrawers).filter( drawer => !activeGlobalDrawersIdsRef.current.includes(drawer.id) && drawer.defaultActive ); From 4adc9df0b78f10b3cfebdbe508e31ed4ee3ab979 Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Wed, 16 Oct 2024 12:42:11 +0200 Subject: [PATCH 5/6] small fix --- src/app-layout/__tests__/runtime-drawers.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app-layout/__tests__/runtime-drawers.test.tsx b/src/app-layout/__tests__/runtime-drawers.test.tsx index df8a50975c..12fc006f38 100644 --- a/src/app-layout/__tests__/runtime-drawers.test.tsx +++ b/src/app-layout/__tests__/runtime-drawers.test.tsx @@ -1075,7 +1075,7 @@ describe('toolbar mode only features', () => { expect(globalDrawersWrapper.findDrawerById('global2')!.isActive()).toBe(true); }); - test('should not open if there are already maximum global drawers opened on the page', async () => { + test('should not open if there are already global drawers opened by user action on the page', async () => { const { wrapper, globalDrawersWrapper } = await renderComponent(); wrapper.findDrawerTriggerById('security')!.click(); @@ -1085,11 +1085,11 @@ describe('toolbar mode only features', () => { ...drawerDefaults, id: 'global1', type: 'global', - defaultActive: true, }); await delay(); + wrapper.findDrawerTriggerById('global1')!.click(); expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); awsuiPlugins.appLayout.registerDrawer({ @@ -1103,7 +1103,7 @@ describe('toolbar mode only features', () => { expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); - expect(globalDrawersWrapper.findDrawerById('global2')!.isActive()).toBe(true); + expect(globalDrawersWrapper.findDrawerById('global2')).toBeFalsy(); }); test('should not open if the maximum number (2) of global drawers is already open on the page', async () => { From db9fb3124192898b939f8080fdd6d56ebaeedfa5 Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Wed, 16 Oct 2024 12:44:26 +0200 Subject: [PATCH 6/6] chore: Additional test assertion --- src/app-layout/__tests__/runtime-drawers.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app-layout/__tests__/runtime-drawers.test.tsx b/src/app-layout/__tests__/runtime-drawers.test.tsx index 12fc006f38..7c30238f25 100644 --- a/src/app-layout/__tests__/runtime-drawers.test.tsx +++ b/src/app-layout/__tests__/runtime-drawers.test.tsx @@ -1145,6 +1145,7 @@ describe('toolbar mode only features', () => { expect(wrapper.findActiveDrawer()!.getElement()).toHaveTextContent('Security'); expect(globalDrawersWrapper.findDrawerById('global1')!.isActive()).toBe(true); expect(globalDrawersWrapper.findDrawerById('global2')!.isActive()).toBe(true); + expect(globalDrawersWrapper.findDrawerById('global3')).toBeFalsy(); }); }); });