Skip to content

Commit

Permalink
chore: Update app layout deduplication order (#2636)
Browse files Browse the repository at this point in the history
  • Loading branch information
@just-boris
just-boris authored Sep 2, 2024
1 parent ca81699 commit bce2ba0
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 43 deletions.
13 changes: 8 additions & 5 deletions src/app-layout/__tests__/global-breadcrumbs.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<>
<AppLayout {...defaultAppLayoutProps} data-testid="first" />
<AppLayout
{...defaultAppLayoutProps}
data-testid="second"
navigationHide={true}
content={<BreadcrumbGroup items={defaultBreadcrumbs} />}
/>
</>
);
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(
<>
<AppLayout
Expand All @@ -168,6 +169,7 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () =>
<AppLayout
{...defaultAppLayoutProps}
data-testid="second"
navigationHide={true}
breadcrumbs={<BreadcrumbGroup items={defaultBreadcrumbs} />}
/>
}
Expand All @@ -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 () => {
Expand Down
48 changes: 27 additions & 21 deletions src/app-layout/__tests__/multi-layout.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}

Expand Down Expand Up @@ -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);
});

Expand All @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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(
<AppLayout
{...defaultAppLayoutProps}
data-testid="first"
Expand All @@ -208,27 +211,30 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () =>
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(
<AppLayout
{...defaultAppLayoutProps}
data-testid="first"
drawers={[testDrawer]}
content={<AppLayout {...defaultAppLayoutProps} data-testid="second" tools="second tools" />}
tools="second tools"
content={<AppLayout {...defaultAppLayoutProps} data-testid="second" drawers={[testDrawer]} />}
/>
);
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();
});
});
});
34 changes: 18 additions & 16 deletions src/internal/plugins/controllers/__tests__/app-layout-widget.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 () => {
Expand All @@ -68,34 +68,36 @@ 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 () => {
let stateFirst: any;
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', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/internal/plugins/controllers/app-layout-widget.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class AppLayoutWidgetController<Props = unknown> {
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;
}
Expand Down

0 comments on commit bce2ba0

Please sign in to comment.