From bce2ba067f9ce974dec7ffe6f7f1d3e7bb184b94 Mon Sep 17 00:00:00 2001 From: Boris Serdiuk Date: Mon, 2 Sep 2024 14:03:49 +0200 Subject: [PATCH] chore: Update app layout deduplication order (#2636) --- .../__tests__/global-breadcrumbs.test.tsx | 13 +++-- .../__tests__/multi-layout.test.tsx | 48 +++++++++++-------- .../__tests__/app-layout-widget.test.ts | 34 ++++++------- .../plugins/controllers/app-layout-widget.ts | 2 +- 4 files changed, 54 insertions(+), 43 deletions(-) diff --git a/src/app-layout/__tests__/global-breadcrumbs.test.tsx b/src/app-layout/__tests__/global-breadcrumbs.test.tsx index 68099a7689..7ae5e8fdff 100644 --- a/src/app-layout/__tests__/global-breadcrumbs.test.tsx +++ b/src/app-layout/__tests__/global-breadcrumbs.test.tsx @@ -135,30 +135,31 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () => expect(findRootBreadcrumb().getElement()).toHaveTextContent('Second'); }); - test('when multiple app layouts rendered, only the last instance receives breadcrumbs', async () => { + test('when multiple app layouts rendered, only the first instance receives breadcrumbs', async () => { await renderAsync( <> } /> ); expect(findAllBreadcrumbsInstances()).toHaveLength(1); - expect(wrapper.find('[data-testid="first"]')!.findAppLayout()!.findBreadcrumbs()).toBeFalsy(); expect( wrapper - .find('[data-testid="second"]')! + .find('[data-testid="first"]')! .findAppLayout()! .findBreadcrumbs()! .findBreadcrumbGroup()! .findBreadcrumbLinks() ).toHaveLength(2); + expect(wrapper.find('[data-testid="second"]')!.findAppLayout()!.findBreadcrumbs()).toBeFalsy(); }); - test('when multiple nested app layouts rendered, the inner instance receives breadcrumbs', async () => { + test('when multiple nested app layouts rendered, the outer instance receives breadcrumbs', async () => { await renderAsync( <> } /> } @@ -177,12 +179,13 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () => expect(findAllBreadcrumbsInstances()).toHaveLength(1); expect( wrapper - .find('[data-testid="second"]')! + .find('[data-testid="first"]')! .findAppLayout()! .findBreadcrumbs()! .findBreadcrumbGroup()! .findBreadcrumbLinks() ).toHaveLength(2); + expect(wrapper.find('[data-testid="second"]')!.findAppLayout()!.findBreadcrumbs()).toBeFalsy(); }); test('updates when a single breadcrumbs instance changes', async () => { diff --git a/src/app-layout/__tests__/multi-layout.test.tsx b/src/app-layout/__tests__/multi-layout.test.tsx index 21a079a4b8..82bdeb0bc6 100644 --- a/src/app-layout/__tests__/multi-layout.test.tsx +++ b/src/app-layout/__tests__/multi-layout.test.tsx @@ -41,7 +41,8 @@ async function renderAsync(jsx: React.ReactElement) { const firstLayout = createWrapper().find('[data-testid="first"]')!.findAppLayout()!; const secondLayout = createWrapper().find('[data-testid="second"]')!.findAppLayout()!; expect(findAllToolbars()).toHaveLength(1); - expect(findToolbar(secondLayout)).toBeTruthy(); + expect(findToolbar(firstLayout)).toBeTruthy(); + expect(findToolbar(secondLayout)).toBeFalsy(); return { firstLayout, secondLayout }; } @@ -70,7 +71,7 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () => expect(isDrawerClosed(firstLayout.findNavigation())).toEqual(true); expect(secondLayout.findNavigation()).toBeFalsy(); - secondLayout.findNavigationToggle().click(); + firstLayout.findNavigationToggle().click(); expect(isDrawerClosed(firstLayout.findNavigation())).toEqual(false); }); @@ -86,7 +87,7 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () => expect(firstLayout.findTools()).toBeFalsy(); expect(secondLayout.findTools()).toBeFalsy(); - secondLayout.findToolsToggle().click(); + firstLayout.findToolsToggle().click(); expect(secondLayout.findTools()).toBeTruthy(); }); @@ -102,10 +103,11 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () => ); expect(firstLayout.findSplitPanel()).toBeTruthy(); expect(secondLayout.findSplitPanel()).toBeFalsy(); - expect(secondLayout.findSplitPanelOpenButton()).toBeTruthy(); + expect(firstLayout.findSplitPanelOpenButton()).toBeTruthy(); + expect(secondLayout.findSplitPanelOpenButton()).toBeFalsy(); expect(firstLayout.findSplitPanel()!.findOpenPanelBottom()).toBeFalsy(); - secondLayout.findSplitPanelOpenButton()!.click(); + firstLayout.findSplitPanelOpenButton()!.click(); expect(firstLayout.findSplitPanel()!.findOpenPanelBottom()).toBeTruthy(); }); @@ -117,8 +119,8 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () => const firstLayout = createWrapper().find('[data-testid="first"]')!.findAppLayout()!; const secondLayout = createWrapper().find('[data-testid="second"]')!.findAppLayout()!; - expect(firstLayout.findNavigationToggle()).toBeFalsy(); - expect(secondLayout.findNavigationToggle()).toBeTruthy(); + expect(firstLayout.findNavigationToggle()).toBeTruthy(); + expect(secondLayout.findNavigationToggle()).toBeFalsy(); }); test('merges props from multiple instances', async () => { @@ -149,12 +151,13 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () => ); await delay(); + const firstLayout = createWrapper().find('[data-testid="first"]')!.findAppLayout()!; const thirdLayout = createWrapper().find('[data-testid="third"]')!.findAppLayout()!; - expect(findToolbar(thirdLayout)).toBeTruthy(); + expect(findToolbar(thirdLayout)).toBeFalsy(); expect(findAllToolbars()).toHaveLength(1); - expect(thirdLayout.findNavigationToggle()).toBeTruthy(); - expect(thirdLayout.findToolsToggle()).toBeTruthy(); - expect(thirdLayout.findSplitPanelOpenButton()).toBeTruthy(); + expect(firstLayout.findNavigationToggle()).toBeTruthy(); + expect(firstLayout.findToolsToggle()).toBeTruthy(); + expect(firstLayout.findSplitPanelOpenButton()).toBeTruthy(); }); test('allows manual deduplication control', async () => { @@ -196,7 +199,7 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () => }); test('ignores duplicate properties and prints a warning', async () => { - const { secondLayout } = await renderAsync( + const { firstLayout, secondLayout } = await renderAsync( expect(console.warn).toHaveBeenCalledWith( expect.stringContaining('Another app layout instance on this page already defines navigation property') ); - secondLayout.findNavigationToggle().click(); - expect(isDrawerClosed(secondLayout.findNavigation())).toEqual(false); + expect(isDrawerClosed(firstLayout.findNavigation())).toEqual(true); + expect(isDrawerClosed(secondLayout.findNavigation())).toEqual(true); + firstLayout.findNavigationToggle().click(); + expect(isDrawerClosed(firstLayout.findNavigation())).toEqual(false); + expect(isDrawerClosed(secondLayout.findNavigation())).toEqual(true); }); test('deduplicates tools and drawers in a single entity', async () => { - const { secondLayout } = await renderAsync( + const { firstLayout } = await renderAsync( } + tools="second tools" + content={} /> ); expect(console.warn).toHaveBeenCalledWith( expect.stringContaining('Another app layout instance on this page already defines tools or drawers property') ); - expect(secondLayout.findDrawersTriggers()).toHaveLength(0); - expect(secondLayout.findTools()).toBeFalsy(); + expect(firstLayout.findDrawersTriggers()).toHaveLength(0); + expect(firstLayout.findTools()).toBeFalsy(); - secondLayout.findToolsToggle().click(); - expect(secondLayout.findTools()).toBeTruthy(); + firstLayout.findToolsToggle().click(); + expect(firstLayout.findTools()).toBeTruthy(); }); }); }); diff --git a/src/internal/plugins/controllers/__tests__/app-layout-widget.test.ts b/src/internal/plugins/controllers/__tests__/app-layout-widget.test.ts index 97f8521491..176a92c76c 100644 --- a/src/internal/plugins/controllers/__tests__/app-layout-widget.test.ts +++ b/src/internal/plugins/controllers/__tests__/app-layout-widget.test.ts @@ -28,9 +28,9 @@ test('registers and unregisters multiple instances', async () => { const cleanupSecond = controller.register(undefined, onRegisterSecond); expect(controller.getStateForTesting().registrations).toHaveLength(2); expect(onRegisterFirst).toHaveBeenCalledTimes(1); - expect(onRegisterFirst).toHaveBeenCalledWith({ type: 'secondary', update: expect.any(Function) }); + expect(onRegisterFirst).toHaveBeenCalledWith({ type: 'primary', discoveredProps: [{}] }); expect(onRegisterSecond).toHaveBeenCalledTimes(1); - expect(onRegisterSecond).toHaveBeenCalledWith({ type: 'primary', discoveredProps: [{}] }); + expect(onRegisterSecond).toHaveBeenCalledWith({ type: 'secondary', update: expect.any(Function) }); onRegisterFirst.mockClear(); onRegisterSecond.mockClear(); @@ -50,13 +50,13 @@ test('delivers property updates from secondary to primary instance', async () => const controller = new AppLayoutWidgetController(); controller.register(undefined, onRegisterFirst); controller.register(undefined, onRegisterSecond); - onRegisterSecond.mockClear(); + onRegisterFirst.mockClear(); - onRegisterFirst.mock.lastCall[0].update({ foo: '123' }); + onRegisterSecond.mock.lastCall[0].update({ foo: '123' }); await delay(); - expect(onRegisterSecond).toHaveBeenCalledTimes(1); - expect(onRegisterSecond).toHaveBeenCalledWith({ type: 'primary', discoveredProps: [{ foo: '123' }] }); + expect(onRegisterFirst).toHaveBeenCalledTimes(1); + expect(onRegisterFirst).toHaveBeenCalledWith({ type: 'primary', discoveredProps: [{ foo: '123' }] }); }); test('delivers property updates from multiple secondary instances', async () => { @@ -68,15 +68,15 @@ test('delivers property updates from multiple secondary instances', async () => controller.register(undefined, state => (stateSecond = state)); controller.register(undefined, state => (stateThird = state)); - expect(stateFirst.type).toEqual('secondary'); + expect(stateFirst.type).toEqual('primary'); expect(stateSecond.type).toEqual('secondary'); - expect(stateThird.type).toEqual('primary'); + expect(stateThird.type).toEqual('secondary'); - stateFirst.update({ foo: '123' }); - stateSecond.update({ bar: '456' }); + stateSecond.update({ foo: '123' }); + stateThird.update({ bar: '456' }); await delay(); - expect(stateThird).toEqual({ type: 'primary', discoveredProps: [{ foo: '123' }, { bar: '456' }] }); + expect(stateFirst).toEqual({ type: 'primary', discoveredProps: [{ foo: '123' }, { bar: '456' }] }); }); test('when primary instance is unregistered, the next becomes primary', async () => { @@ -84,18 +84,20 @@ test('when primary instance is unregistered, the next becomes primary', async () let stateSecond: any; let stateThird: any; const controller = new AppLayoutWidgetController(); - controller.register(undefined, state => (stateFirst = state)); + const cleanupFirst = controller.register(undefined, state => (stateFirst = state)); controller.register(undefined, state => (stateSecond = state)); - const cleanupLast = controller.register(undefined, state => (stateThird = state)); + controller.register(undefined, state => (stateThird = state)); - expect(stateThird.type).toEqual('primary'); + expect(stateFirst.type).toEqual('primary'); + expect(stateThird.type).toEqual('secondary'); - cleanupLast(); + cleanupFirst(); expect(stateSecond.type).toEqual('secondary'); + expect(stateThird.type).toEqual('secondary'); await delay(); expect(stateSecond.type).toEqual('primary'); - expect(stateFirst.type).toEqual('secondary'); + expect(stateThird.type).toEqual('secondary'); }); test('supports forced primary registration', () => { diff --git a/src/internal/plugins/controllers/app-layout-widget.ts b/src/internal/plugins/controllers/app-layout-widget.ts index 3e35a07fee..04897ed21b 100644 --- a/src/internal/plugins/controllers/app-layout-widget.ts +++ b/src/internal/plugins/controllers/app-layout-widget.ts @@ -42,7 +42,7 @@ export class AppLayoutWidgetController { if (forcedPrimary) { return forcedPrimary; } - for (const registration of this.#registrations.slice().reverse()) { + for (const registration of this.#registrations.slice()) { if (registration.forceType !== 'secondary') { return registration; }