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

[Component templates] Address feedback #70912

Merged
merged 12 commits into from
Jul 13, 2020
Merged

[Component templates] Address feedback #70912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6f15fb3
address various feedback
alisonelizabeth Jul 6, 2020
1d94828
Merge branch 'master' of github.com:elastic/kibana into component_tem…
alisonelizabeth Jul 7, 2020
eeaa3f1
update tests and add managed table filter
alisonelizabeth Jul 7, 2020
6d2d796
add learn more link
alisonelizabeth Jul 7, 2020
533cf63
fix import
alisonelizabeth Jul 7, 2020
97c4234
update copy
alisonelizabeth Jul 7, 2020
e11fda6
fix link navigation
alisonelizabeth Jul 8, 2020
13876c8
Merge branch 'master' into component_templates/feedback
elasticmachine Jul 9, 2020
2a37ccf
Merge branch 'master' of github.com:elastic/kibana into component_tem…
alisonelizabeth Jul 9, 2020
cc9d6e4
Merge branch 'master' of github.com:elastic/kibana into component_tem…
alisonelizabeth Jul 10, 2020
48adbd2
fix app navigation and clean up empty fields
alisonelizabeth Jul 11, 2020
b61244e
Merge branch 'master' into component_templates/feedback
elasticmachine Jul 13, 2020
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
2 changes: 2 additions & 0 deletions x-pack/plugins/index_management/common/lib/component_template_serialization.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ describe('Component template serialization', () => {
},
_kbnMeta: {
usedBy: ['my_index_template'],
isManaged: false,
},
});
});
Expand All @@ -105,6 +106,7 @@ describe('Component template serialization', () => {
version: 1,
_kbnMeta: {
usedBy: [],
isManaged: false,
},
_meta: {
serialization: {
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/index_management/common/lib/component_template_serialization.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,26 @@ export function deserializeComponentTemplate(
_meta,
_kbnMeta: {
usedBy: indexTemplatesToUsedBy[name] || [],
isManaged: Boolean(_meta?.managed === true),
},
};

return deserializedComponentTemplate;
}

export function deserializeComponenTemplateList(
export function deserializeComponentTemplateList(
componentTemplateEs: ComponentTemplateFromEs,
indexTemplatesEs: TemplateFromEs[]
) {
const { name, component_template: componentTemplate } = componentTemplateEs;
const { template } = componentTemplate;
const { template, _meta } = componentTemplate;

const indexTemplatesToUsedBy = getIndexTemplatesToUsedBy(indexTemplatesEs);

const componentTemplateListItem: ComponentTemplateListItem = {
name,
usedBy: indexTemplatesToUsedBy[name] || [],
isManaged: Boolean(_meta?.managed === true),
hasSettings: hasEntries(template.settings),
hasMappings: hasEntries(template.mappings),
hasAliases: hasEntries(template.aliases),
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/index_management/common/lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ export { getTemplateParameter } from './utils';

export {
deserializeComponentTemplate,
deserializeComponenTemplateList,
deserializeComponentTemplateList,
serializeComponentTemplate,
} from './component_template_serialization';
2 changes: 2 additions & 0 deletions x-pack/plugins/index_management/common/types/component_templates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface ComponentTemplateDeserialized extends ComponentTemplateSerializ
name: string;
_kbnMeta: {
usedBy: string[];
isManaged: boolean;
};
}

Expand All @@ -36,4 +37,5 @@ export interface ComponentTemplateListItem {
hasMappings: boolean;
hasAliases: boolean;
hasSettings: boolean;
isManaged: boolean;
}
2 changes: 1 addition & 1 deletion ...onents/component_templates/__jest__/client_integration/component_template_create.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ describe('<ComponentTemplateCreate />', () => {
},
aliases: ALIASES,
},
_kbnMeta: { usedBy: [] },
_kbnMeta: { usedBy: [], isManaged: false },
};

expect(JSON.parse(JSON.parse(latestRequest.requestBody).body)).toEqual(expected);
Expand Down
4 changes: 2 additions & 2 deletions ...onents/component_templates/__jest__/client_integration/component_template_details.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ const COMPONENT_TEMPLATE: ComponentTemplateDeserialized = {
},
version: 1,
_meta: { description: 'component template test' },
_kbnMeta: { usedBy: ['template_1'] },
_kbnMeta: { usedBy: ['template_1'], isManaged: false },
};

const COMPONENT_TEMPLATE_ONLY_REQUIRED_FIELDS: ComponentTemplateDeserialized = {
name: 'comp-base',
template: {},
_kbnMeta: { usedBy: [] },
_kbnMeta: { usedBy: [], isManaged: false },
};

describe('<ComponentTemplateDetails />', () => {
Expand Down
2 changes: 1 addition & 1 deletion ...mponents/component_templates/__jest__/client_integration/component_template_edit.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('<ComponentTemplateEdit />', () => {
template: {
settings: { number_of_shards: 1 },
},
_kbnMeta: { usedBy: [] },
_kbnMeta: { usedBy: [], isManaged: false },
};

beforeEach(async () => {
Expand Down
2 changes: 2 additions & 0 deletions ...omponents/component_templates/__jest__/client_integration/component_template_list.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe('<ComponentTemplateList />', () => {
hasAliases: true,
hasSettings: true,
usedBy: [],
isManaged: false,
};

const componentTemplate2: ComponentTemplateListItem = {
Expand All @@ -50,6 +51,7 @@ describe('<ComponentTemplateList />', () => {
hasAliases: true,
hasSettings: true,
usedBy: ['test_index_template_1'],
isManaged: false,
};

const componentTemplates = [componentTemplate1, componentTemplate2];
Expand Down
2 changes: 2 additions & 0 deletions .../components/component_templates/__jest__/client_integration/helpers/setup_environment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { HttpSetup } from 'kibana/public';
import {
notificationServiceMock,
docLinksServiceMock,
applicationServiceMock,
} from '../../../../../../../../../../src/core/public/mocks';

import { ComponentTemplatesProvider } from '../../../component_templates_context';
Expand All @@ -28,6 +29,7 @@ const appDependencies = {
docLinks: docLinksServiceMock.createStartContract(),
toasts: notificationServiceMock.createSetupContract().toasts,
setBreadcrumbs: () => {},
getUrlForApp: applicationServiceMock.createStartContract().getUrlForApp,
};

export const setupEnvironment = () => {
Expand Down
38 changes: 31 additions & 7 deletions .../components/component_templates/component_template_details/component_template_details.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import React, { useState } from 'react';
import { FormattedMessage } from '@kbn/i18n/react';

import {
EuiFlyout,
EuiFlyoutHeader,
Expand All @@ -17,6 +18,7 @@ import {
EuiButtonEmpty,
EuiSpacer,
EuiCallOut,
EuiBadge,
} from '@elastic/eui';

import { SectionLoading, TabSettings, TabAliases, TabMappings } from '../shared_imports';
Expand All @@ -29,14 +31,15 @@ import { attemptToDecodeURI } from '../lib';
interface Props {
componentTemplateName: string;
onClose: () => void;
showFooter?: boolean;
actions?: ManageAction[];
showSummaryCallToAction?: boolean;
}

export const ComponentTemplateDetailsFlyout: React.FunctionComponent<Props> = ({
componentTemplateName,
onClose,
actions,
showSummaryCallToAction,
}) => {
const { api } = useComponentTemplatesContext();

Expand Down Expand Up @@ -81,7 +84,12 @@ export const ComponentTemplateDetailsFlyout: React.FunctionComponent<Props> = ({
} = componentTemplateDetails;

const tabToComponentMap: Record<TabType, React.ReactNode> = {
summary: <TabSummary componentTemplateDetails={componentTemplateDetails} />,
summary: (
<TabSummary
componentTemplateDetails={componentTemplateDetails}
showCallToAction={showSummaryCallToAction}
/>
),
settings: <TabSettings settings={settings} />,
mappings: <TabMappings mappings={mappings} />,
aliases: <TabAliases aliases={aliases} />,
Expand Down Expand Up @@ -109,11 +117,27 @@ export const ComponentTemplateDetailsFlyout: React.FunctionComponent<Props> = ({
maxWidth={500}
>
<EuiFlyoutHeader>
<EuiTitle size="m">
<h2 id="componentTemplateDetailsFlyoutTitle" data-test-subj="title">
{decodedComponentTemplateName}
</h2>
</EuiTitle>
<EuiFlexGroup alignItems="center" gutterSize="s">
<EuiFlexItem grow={false}>
<EuiTitle size="m">
<h2 id="componentTemplateDetailsFlyoutTitle" data-test-subj="title">
{decodedComponentTemplateName}
</h2>
</EuiTitle>
</EuiFlexItem>

{componentTemplateDetails?._kbnMeta.isManaged ? (
<EuiFlexItem grow={false}>
{' '}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this whitespace intended to be included? Perhaps &npsb; would be better if so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is intentional. I'm going to leave as is for now, but will consider changing in a follow-up PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI the empty space that Alison used will allow a line break if the container forces the text to wrap, but a non-breaking space won’t allow this. In most situations I think we want the former behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out @cjcenizal!

Copy link
Contributor

@sebelga sebelga Jul 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

FYI the empty space that Alison used will allow a line break if the container forces the text to wrap, but a non-breaking space won’t allow this. In most situations I think we want the former behavior.

@cjcenizal Not sure I understand correctly. Are we talking about 1 space difference (probably a few pixels) that would not wrap? Ex: hello world (there is a space before the "h"): where would this not wrap after "hello"?

Copy link
Contributor

@cjcenizal cjcenizal Jul 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I’m not sure if this answers your question but I was just pointing out that &nbsp; !== ‘ ‘. The former won’t allow a line break on the space (“hello world” will overflow its container) while the latter will (“hello world” will break into two lines consisting of “hello” and “world”).

Copy link
Contributor

Choose a reason for hiding this comment

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

This is strange, I would need to try it out 😊 It would make sense if we were talking about hello&nbsp;world but here we are adding the   before the 2 words.

<EuiBadge color="hollow">
<FormattedMessage
id="xpack.idxMgmt.componentTemplateDetails.managedBadgeLabel"
defaultMessage="Managed"
/>
</EuiBadge>
</EuiFlexItem>
) : null}
</EuiFlexGroup>
</EuiFlyoutHeader>

<EuiFlyoutBody data-test-subj="content">{content}</EuiFlyoutBody>
Expand Down
48 changes: 46 additions & 2 deletions ...lic/application/components/component_templates/component_template_details/tab_summary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import React from 'react';
import { FormattedMessage } from '@kbn/i18n/react';

import {
EuiDescriptionList,
EuiDescriptionListTitle,
Expand All @@ -14,15 +15,23 @@ import {
EuiTitle,
EuiCallOut,
EuiSpacer,
EuiLink,
} from '@elastic/eui';

import { ComponentTemplateDeserialized } from '../shared_imports';
import { useComponentTemplatesContext } from '../component_templates_context';

interface Props {
componentTemplateDetails: ComponentTemplateDeserialized;
showCallToAction?: boolean;
}

export const TabSummary: React.FunctionComponent<Props> = ({ componentTemplateDetails }) => {
export const TabSummary: React.FunctionComponent<Props> = ({
componentTemplateDetails,
showCallToAction,
}) => {
const { getUrlForApp } = useComponentTemplatesContext();

const { version, _meta, _kbnMeta } = componentTemplateDetails;

const { usedBy } = _kbnMeta;
Expand All @@ -43,7 +52,42 @@ export const TabSummary: React.FunctionComponent<Props> = ({ componentTemplateDe
iconType="pin"
data-test-subj="notInUseCallout"
size="s"
/>
>
{showCallToAction && (
<p>
<FormattedMessage
id="xpack.idxMgmt.componentTemplateDetails.summaryTab.notInUseDescription"
defaultMessage="{createLink} an index template or {editLink} an existing one."
values={{
createLink: (
<EuiLink
href={getUrlForApp('management', {
path: '/data/index_management/create_template',
})}
>
<FormattedMessage
id="xpack.idxMgmt.componentTemplateDetails.summaryTab.createTemplateLink"
defaultMessage="Create"
/>
</EuiLink>
),
editLink: (
<EuiLink
href={getUrlForApp('management', {
path: '/data/index_management/templates',
})}
>
<FormattedMessage
id="xpack.idxMgmt.componentTemplateDetails.summaryTab.updateTemplateLink"
defaultMessage="update"
/>
</EuiLink>
),
}}
/>
</p>
)}
</EuiCallOut>
<EuiSpacer />
</>
)}
Expand Down
42 changes: 32 additions & 10 deletions ...cation/components/component_templates/component_template_list/component_template_list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { RouteComponentProps } from 'react-router-dom';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { ScopedHistory } from 'kibana/public';
import { EuiLink, EuiText, EuiSpacer } from '@elastic/eui';

import { SectionLoading, ComponentTemplateDeserialized } from '../shared_imports';
import { UIM_COMPONENT_TEMPLATE_LIST_LOAD } from '../constants';
Expand All @@ -29,7 +30,7 @@ export const ComponentTemplateList: React.FunctionComponent<Props> = ({
componentTemplateName,
history,
}) => {
const { api, trackMetric } = useComponentTemplatesContext();
const { api, trackMetric, documentation } = useComponentTemplatesContext();

const { data, isLoading, error, sendRequest } = api.useLoadComponentTemplates();

Expand Down Expand Up @@ -65,20 +66,40 @@ export const ComponentTemplateList: React.FunctionComponent<Props> = ({
<SectionLoading data-test-subj="sectionLoading">
<FormattedMessage
id="xpack.idxMgmt.home.componentTemplates.list.loadingMessage"
defaultMessage="Loading component templates..."
defaultMessage="Loading component templates…"
/>
</SectionLoading>
);
} else if (data?.length) {
content = (
<ComponentTable
componentTemplates={data}
onReloadClick={sendRequest}
onDeleteClick={setComponentTemplatesToDelete}
onEditClick={goToEditComponentTemplate}
onCloneClick={goToCloneComponentTemplate}
history={history as ScopedHistory}
/>
<>
<EuiText color="subdued">
<FormattedMessage
id="xpack.idxMgmt.home.componentTemplates.list.componentTemplatesDescription"
defaultMessage="Use component templates to reuse settings, mappings, and aliases configurations in multiple index templates. {learnMoreLink}"
values={{
learnMoreLink: (
<EuiLink href={documentation.componentTemplates} target="_blank" external>
{i18n.translate('xpack.idxMgmt.componentTemplates.list.learnMoreLinkText', {
defaultMessage: 'Learn more.',
})}
</EuiLink>
),
}}
/>
</EuiText>

<EuiSpacer />

<ComponentTable
componentTemplates={data}
onReloadClick={sendRequest}
onDeleteClick={setComponentTemplatesToDelete}
onEditClick={goToEditComponentTemplate}
onCloneClick={goToCloneComponentTemplate}
history={history as ScopedHistory}
/>
</>
);
} else if (data && data.length === 0) {
content = <EmptyPrompt history={history} />;
Expand Down Expand Up @@ -111,6 +132,7 @@ export const ComponentTemplateList: React.FunctionComponent<Props> = ({
<ComponentTemplateDetailsFlyout
onClose={goToComponentTemplateList}
componentTemplateName={componentTemplateName}
showSummaryCallToAction={true}
actions={[
{
name: i18n.translate('xpack.idxMgmt.componentTemplateDetails.editButtonLabel', {
Expand Down
6 changes: 3 additions & 3 deletions ...ublic/application/components/component_templates/component_template_list/empty_prompt.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { FormattedMessage } from '@kbn/i18n/react';
import { RouteComponentProps } from 'react-router-dom';
import { EuiEmptyPrompt, EuiLink, EuiButton } from '@elastic/eui';

import { reactRouterNavigate } from '../../../../../../../../src/plugins/kibana_react/public';
import { reactRouterNavigate } from '../shared_imports';
import { useComponentTemplatesContext } from '../component_templates_context';

interface Props {
Expand All @@ -34,12 +34,12 @@ export const EmptyPrompt: FunctionComponent<Props> = ({ history }) => {
<p>
<FormattedMessage
id="xpack.idxMgmt.home.componentTemplates.emptyPromptDescription"
defaultMessage="For example, you might create a component template that defines index settings that can be reused across index templates."
defaultMessage="For example, you can create a component template for index settings that can be reused across index templates."
/>
<br />
<EuiLink href={documentation.componentTemplates} target="_blank" external>
{i18n.translate('xpack.idxMgmt.home.componentTemplates.emptyPromptDocumentionLink', {
defaultMessage: 'Learn more',
defaultMessage: 'Learn more.',
})}
</EuiLink>
</p>
Expand Down
Loading