Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Migrate ILM to ManagementPageLayout #100838

Closed
wants to merge 4 commits into from
Closed

Migrate ILM to ManagementPageLayout #100838

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b57b80e
Migrate ILM to ManagementPageLayout.
cjcenizal May 27, 2021
1f184f1
Fix Jest tests.
cjcenizal May 27, 2021
b704ef1
Fix typos in i18n keys.
cjcenizal May 28, 2021
f907a08
Replace use of isEmptyState with centeredContent template.
cjcenizal May 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 72 additions & 56 deletions x-pack/plugins/index_lifecycle_management/__jest__/__snapshots__/policy_table.test.tsx.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion x-pack/plugins/index_lifecycle_management/__jest__/client_integration/app/app.helpers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import React from 'react';
import { act } from 'react-dom/test-utils';
import { registerTestBed, TestBed, TestBedConfig } from '@kbn/test/jest';
import { KibanaPageTemplate } from '../../../../../../src/plugins/kibana_react/public';
import { KibanaContextProvider } from '../../../../../../src/plugins/kibana_react/public/context';
import { createBreadcrumbsMock } from '../../../public/application/services/breadcrumbs.mock';
import { licensingMock } from '../../../../licensing/public/mocks';
Expand All @@ -17,7 +18,13 @@ const breadcrumbService = createBreadcrumbsMock();

const AppWithContext = (props: any) => {
return (
<KibanaContextProvider services={{ breadcrumbService, license: licensingMock.createLicense() }}>
<KibanaContextProvider
services={{
breadcrumbService,
license: licensingMock.createLicense(),
managementPageLayout: KibanaPageTemplate,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Step 4: Update Jest tests to set up testbed by stubbing the injected managementPageLayout with KibanaPageTemplate.

}}
>
<App {...props} />
</KibanaContextProvider>
);
Expand Down
2 changes: 2 additions & 0 deletions ...gins/index_lifecycle_management/__jest__/client_integration/edit_policy/init_test_bed.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { registerTestBed, TestBedConfig } from '@kbn/test/jest';

import '../helpers/global_mocks';

import { KibanaPageTemplate } from '../../../../../../src/plugins/kibana_react/public';
import { licensingMock } from '../../../../licensing/public/mocks';
import { EditPolicy } from '../../../public/application/sections/edit_policy';
import { KibanaContextProvider } from '../../../public/shared_imports';
Expand Down Expand Up @@ -38,6 +39,7 @@ const EditPolicyContainer = ({ appServicesContext, ...rest }: any) => {
services={{
breadcrumbService,
license: licensingMock.createLicense({ license: { type: 'enterprise' } }),
managementPageLayout: KibanaPageTemplate,
...appServicesContext,
}}
>
Expand Down
38 changes: 26 additions & 12 deletions x-pack/plugins/index_lifecycle_management/__jest__/policy_table.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {
} from '../../../../src/core/public/mocks';
import { HttpService } from '../../../../src/core/public/http';
import { usageCollectionPluginMock } from '../../../../src/plugins/usage_collection/public/mocks';
import { KibanaPageTemplate } from '../../../../src/plugins/kibana_react/public';
import { KibanaContextProvider } from '../public/shared_imports';

import { PolicyFromES } from '../common/types';
import { PolicyTable } from '../public/application/sections/policy_table/policy_table';
Expand Down Expand Up @@ -84,23 +86,35 @@ const openContextMenu = (buttonIndex: number) => {
describe('policy table', () => {
beforeEach(() => {
component = (
<PolicyTable
policies={policies}
history={scopedHistoryMock.create()}
navigateToApp={jest.fn()}
updatePolicies={jest.fn()}
/>
<KibanaContextProvider
services={{
managementPageLayout: KibanaPageTemplate,
}}
>
<PolicyTable
policies={policies}
history={scopedHistoryMock.create()}
navigateToApp={jest.fn()}
updatePolicies={jest.fn()}
/>
</KibanaContextProvider>
);
});

test('should show empty state when there are not any policies', () => {
component = (
<PolicyTable
policies={[]}
history={scopedHistoryMock.create()}
navigateToApp={jest.fn()}
updatePolicies={jest.fn()}
/>
<KibanaContextProvider
services={{
managementPageLayout: KibanaPageTemplate,
}}
>
<PolicyTable
policies={[]}
history={scopedHistoryMock.create()}
navigateToApp={jest.fn()}
updatePolicies={jest.fn()}
/>
</KibanaContextProvider>
);
const rendered = mountWithIntl(component);
mountedSnapshot(rendered);
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/index_lifecycle_management/public/application/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import React from 'react';
import { render, unmountComponentAtNode } from 'react-dom';
import { I18nStart, ScopedHistory, ApplicationStart } from 'kibana/public';
import { UnmountCallback } from 'src/core/public';

import type { ManagementAppMountParams } from '../../../../../src/plugins/management/public';
import { CloudSetup } from '../../../cloud/public';
import { ILicense } from '../../../licensing/public';

Expand All @@ -26,11 +28,12 @@ export const renderApp = (
getUrlForApp: ApplicationStart['getUrlForApp'],
breadcrumbService: BreadcrumbService,
license: ILicense,
managementPageLayout: ManagementAppMountParams['managementPageLayout'],
cloud?: CloudSetup
): UnmountCallback => {
render(
<I18nContext>
<KibanaContextProvider services={{ cloud, breadcrumbService, license }}>
<KibanaContextProvider services={{ cloud, breadcrumbService, license, managementPageLayout }}>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Step 2: Use KibanaContextProvider to provide it to the app using React Context. Note that we have to update our types in the types.ts file to support the change to this interface.

<AppWithRouter
history={history}
navigateToApp={navigateToApp}
Expand Down
71 changes: 38 additions & 33 deletions ...ex_lifecycle_management/public/application/sections/edit_policy/edit_policy.container.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const EditPolicy: React.FunctionComponent<Props & RouteComponentProps<Rou
history,
}) => {
const {
services: { breadcrumbService, license },
services: { breadcrumbService, license, managementPageLayout: ManagementPageLayout },
} = useKibana();
const { error, isLoading, data: policies, resendRequest } = useLoadPoliciesList(false);

Expand All @@ -52,43 +52,48 @@ export const EditPolicy: React.FunctionComponent<Props & RouteComponentProps<Rou

if (isLoading) {
return (
<EuiEmptyPrompt
title={<EuiLoadingSpinner size="xl" />}
body={
<FormattedMessage
id="xpack.indexLifecycleMgmt.editPolicy.policiesLoading"
defaultMessage="Loading policies..."
/>
}
/>
<ManagementPageLayout isEmptyState={true}>
Copy link
Member

@sabarasaba sabarasaba May 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using the isEmptyState prop, the content panel ends up with a grey background which is the same the apps navigation has. This makes it look a bit confusing as it seems its all one big section instead of two clearly distinct regions.

Screenshot 2021-05-28 at 11 59 15

We could try following a similar approach to what the observability team did, which is instead of using isEmpyState={true} use template="centeredContent". And while this will give us a more clear division between the navigation and content sections, it has one small drawback which is that we end up with an empty pageHeader (observability seems to have left it there regardless). We could fix that issue though by adding a pageTitle, but I cant find a way to just hide it.

Screenshot 2021-05-28 at 11 50 33

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sabarasaba Great call! @cchaos This seems like a design problem. Do you have any suggestions on the best way to solve this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+++ Yes, don't use the centeredBody when we've got side-nav. I'm currently working on an upgrade to create a pattern similar to the empty state one for loading.

<EuiEmptyPrompt
title={<EuiLoadingSpinner size="xl" />}
body={
<FormattedMessage
id="xpack.indexLifecycleMgmt.editPolicy.policiesLoading"
defaultMessage="Loading policies..."
/>
}
/>
</ManagementPageLayout>
);
}

if (error || !policies) {
const { statusCode, message } = error ? error : { statusCode: '', message: '' };
return (
<EuiEmptyPrompt
title={
<h2>
<FormattedMessage
id="xpack.indexLifecycleMgmt.editPolicy.lifecyclePoliciesLoadingFailedTitle"
defaultMessage="Unable to load existing lifecycle policies"
/>
</h2>
}
body={
<p>
{message} ({statusCode})
</p>
}
actions={
<EuiButton onClick={resendRequest} iconType="refresh" color="danger">
<FormattedMessage
id="xpack.indexLifecycleMgmt.editPolicy.lifecyclePoliciesReloadButton"
defaultMessage="Try again"
/>
</EuiButton>
}
/>
<ManagementPageLayout isEmptyState={true}>
<EuiEmptyPrompt
title={
<h2>
<FormattedMessage
id="xpack.indexLifecycleMgmt.editPolicy.lifecyclePoliciesLoadingFailedTitle"
defaultMessage="Unable to load existing lifecycle policies"
/>
</h2>
}
body={
<p>
{message} ({statusCode})
</p>
}
actions={
<EuiButton onClick={resendRequest} iconType="refresh" color="danger">
<FormattedMessage
id="xpack.indexLifecycleMgmt.editPolicy.lifecyclePoliciesReloadButton"
defaultMessage="Try again"
/>
</EuiButton>
}
/>
</ManagementPageLayout>
);
}

Expand Down
Loading