Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@
export { SchemaCallouts } from './schema_callouts';
export { SchemaTable } from './schema_table';
export { EmptyState } from './empty_state';
export { MetaEnginesSchemaTable } from './meta_engines_schema_table';
export { MetaEnginesConflictsTable } from './meta_engines_conflicts_table';
export { TruncatedEnginesList } from './truncated_engines_list';
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { setMockValues } from '../../../../__mocks__';

import React from 'react';

import { mount } from 'enzyme';

import { EuiTable, EuiTableHeaderCell, EuiTableRow } from '@elastic/eui';

import { MetaEnginesConflictsTable } from './';

describe('MetaEnginesConflictsTable', () => {
const values = {
conflictingFields: {
hello_field: {
text: ['engine1'],
number: ['engine2'],
date: ['engine3'],
},
world_field: {
text: ['engine1'],
location: ['engine2', 'engine3', 'engine4'],
},
},
};

setMockValues(values);
const wrapper = mount(<MetaEnginesConflictsTable />);
const fieldNames = wrapper.find('EuiTableRowCell[data-test-subj="fieldName"]');
const fieldTypes = wrapper.find('EuiTableRowCell[data-test-subj="fieldTypes"]');
const engines = wrapper.find('EuiTableRowCell[data-test-subj="enginesPerFieldType"]');

it('renders', () => {
expect(wrapper.find(EuiTable)).toHaveLength(1);
expect(wrapper.find(EuiTableHeaderCell).at(0).text()).toEqual('Field name');
expect(wrapper.find(EuiTableHeaderCell).at(1).text()).toEqual('Field type conflicts');
expect(wrapper.find(EuiTableHeaderCell).at(2).text()).toEqual('Engines');
});

it('renders a rowspan on the initial field name column so that it stretches to all associated field conflict rows', () => {
expect(fieldNames).toHaveLength(2);
expect(fieldNames.at(0).prop('rowSpan')).toEqual(3);
expect(fieldNames.at(1).prop('rowSpan')).toEqual(2);
});

it('renders a row for each field type conflict and the engines that have that field type', () => {
expect(wrapper.find(EuiTableRow)).toHaveLength(5);

expect(fieldNames.at(0).text()).toEqual('hello_field');
expect(fieldTypes.at(0).text()).toEqual('text');
expect(engines.at(0).text()).toEqual('engine1');
expect(fieldTypes.at(1).text()).toEqual('number');
expect(engines.at(1).text()).toEqual('engine2');
expect(fieldTypes.at(2).text()).toEqual('date');
expect(engines.at(2).text()).toEqual('engine3');

expect(fieldNames.at(1).text()).toEqual('world_field');
expect(fieldTypes.at(3).text()).toEqual('text');
expect(engines.at(3).text()).toEqual('engine1');
expect(fieldTypes.at(4).text()).toEqual('location');
expect(engines.at(4).text()).toEqual('engine2, engine3, +1');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { useValues } from 'kea';

import {
EuiTable,
EuiTableHeader,
EuiTableHeaderCell,
EuiTableBody,
EuiTableRow,
EuiTableRowCell,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { FIELD_NAME } from '../../../../shared/schema/constants';
import { ENGINES_TITLE } from '../../engines';

import { MetaEngineSchemaLogic } from '../schema_meta_engine_logic';

import { TruncatedEnginesList } from './';

export const MetaEnginesConflictsTable: React.FC = () => {
const { conflictingFields } = useValues(MetaEngineSchemaLogic);

return (
<EuiTable tableLayout="auto">
<EuiTableHeader>
<EuiTableHeaderCell>{FIELD_NAME}</EuiTableHeaderCell>
<EuiTableHeaderCell>
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.schema.metaEngine.fieldTypeConflicts',
{ defaultMessage: 'Field type conflicts' }
)}
</EuiTableHeaderCell>
<EuiTableHeaderCell>{ENGINES_TITLE}</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
{Object.entries(conflictingFields).map(([fieldName, conflicts]) =>
Object.entries(conflicts).map(([fieldType, engines], i) => {
const isFirstRow = i === 0;
return (
<EuiTableRow key={`${fieldName}-${fieldType}`}>
{isFirstRow && (
<EuiTableRowCell
rowSpan={Object.values(conflicts).length}
data-test-subj="fieldName"
>
<code>{fieldName}</code>
</EuiTableRowCell>
)}
Comment on lines +45 to +57
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hopefully this isn't too difficult to grok code-wise, but the goal of the first row check and rowSpan behavior is to essentially render this:

TBH it's still less code than the original ent-search view, purely by having less custom design etc :)

<EuiTableRowCell data-test-subj="fieldTypes">{fieldType}</EuiTableRowCell>
<EuiTableRowCell data-test-subj="enginesPerFieldType">
<TruncatedEnginesList engines={engines} cutoff={2} />
</EuiTableRowCell>
</EuiTableRow>
);
})
)}
</EuiTableBody>
</EuiTable>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { setMockValues } from '../../../../__mocks__';

import React from 'react';

import { mount } from 'enzyme';

import { EuiTable, EuiTableHeaderCell, EuiTableRow, EuiTableRowCell } from '@elastic/eui';

import { MetaEnginesSchemaTable } from './';

describe('MetaEnginesSchemaTable', () => {
const values = {
schema: {
some_text_field: 'text',
some_number_field: 'number',
},
fields: {
some_text_field: {
text: ['engine1', 'engine2'],
},
some_number_field: {
number: ['engine1', 'engine2', 'engine3', 'engine4'],
},
},
};

setMockValues(values);
const wrapper = mount(<MetaEnginesSchemaTable />);
const fieldNames = wrapper.find('EuiTableRowCell[data-test-subj="fieldName"]');
const engines = wrapper.find('EuiTableRowCell[data-test-subj="engines"]');
const fieldTypes = wrapper.find('EuiTableRowCell[data-test-subj="fieldType"]');

it('renders', () => {
expect(wrapper.find(EuiTable)).toHaveLength(1);
expect(wrapper.find(EuiTableHeaderCell).at(0).text()).toEqual('Field name');
expect(wrapper.find(EuiTableHeaderCell).at(1).text()).toEqual('Engines');
expect(wrapper.find(EuiTableHeaderCell).at(2).text()).toEqual('Field type');
});

it('always renders an initial ID row', () => {
expect(wrapper.find('code').at(0).text()).toEqual('id');
expect(wrapper.find(EuiTableRowCell).at(1).text()).toEqual('All');
});

it('renders subsequent table rows for each schema field', () => {
expect(wrapper.find(EuiTableRow)).toHaveLength(3);

expect(fieldNames.at(0).text()).toEqual('some_text_field');
expect(engines.at(0).text()).toEqual('engine1, engine2');
expect(fieldTypes.at(0).text()).toEqual('text');

expect(fieldNames.at(1).text()).toEqual('some_number_field');
expect(engines.at(1).text()).toEqual('engine1, engine2, engine3, +1');
expect(fieldTypes.at(1).text()).toEqual('number');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { useValues } from 'kea';

import {
EuiTable,
EuiTableHeader,
EuiTableHeaderCell,
EuiTableBody,
EuiTableRow,
EuiTableRowCell,
EuiText,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { FIELD_NAME, FIELD_TYPE } from '../../../../shared/schema/constants';
import { ENGINES_TITLE } from '../../engines';

import { MetaEngineSchemaLogic } from '../schema_meta_engine_logic';

import { TruncatedEnginesList } from './';

export const MetaEnginesSchemaTable: React.FC = () => {
const { schema, fields } = useValues(MetaEngineSchemaLogic);

return (
<EuiTable tableLayout="auto">
<EuiTableHeader>
<EuiTableHeaderCell>{FIELD_NAME}</EuiTableHeaderCell>
<EuiTableHeaderCell>{ENGINES_TITLE}</EuiTableHeaderCell>
<EuiTableHeaderCell align="right">{FIELD_TYPE}</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableRow>
<EuiTableRowCell>
<EuiText color="subdued">
<code>id</code>
</EuiText>
</EuiTableRowCell>
<EuiTableRowCell>
<EuiText color="subdued" size="s">
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.schema.metaEngine.allEngines',
{ defaultMessage: 'All' }
)}
</EuiText>
</EuiTableRowCell>
<EuiTableRowCell aria-hidden />
</EuiTableRow>
{Object.keys(fields).map((fieldName) => {
const fieldType = schema[fieldName];
const engines = fields[fieldName][fieldType];

return (
<EuiTableRow key={fieldName}>
<EuiTableRowCell data-test-subj="fieldName">
<code>{fieldName}</code>
</EuiTableRowCell>
<EuiTableRowCell data-test-subj="engines">
<TruncatedEnginesList engines={engines} />
</EuiTableRowCell>
<EuiTableRowCell align="right" data-test-subj="fieldType">
{fieldType}
</EuiTableRowCell>
</EuiTableRow>
);
})}
</EuiTableBody>
</EuiTable>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { shallow } from 'enzyme';

import { TruncatedEnginesList } from './';

describe('TruncatedEnginesList', () => {
it('renders a list of engines with links to their schema pages', () => {
const wrapper = shallow(<TruncatedEnginesList engines={['engine1', 'engine2', 'engine3']} />);

expect(wrapper.find('[data-test-subj="displayedEngine"]')).toHaveLength(3);
expect(wrapper.find('[data-test-subj="displayedEngine"]').first().prop('to')).toEqual(
'/engines/engine1/schema'
);
});

it('renders a tooltip when the number of engines is greater than the cutoff', () => {
const wrapper = shallow(
<TruncatedEnginesList engines={['engine1', 'engine2', 'engine3']} cutoff={1} />
);

expect(wrapper.find('[data-test-subj="displayedEngine"]')).toHaveLength(1);
expect(wrapper.find('[data-test-subj="hiddenEnginesTooltip"]')).toHaveLength(1);
expect(wrapper.find('[data-test-subj="hiddenEnginesTooltip"]').prop('content')).toEqual(
'engine2, engine3'
);
});

it('does not render if no engines are passed', () => {
const wrapper = shallow(<TruncatedEnginesList engines={[]} />);

expect(wrapper.isEmptyRender()).toBe(true);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React, { Fragment } from 'react';

import { EuiText, EuiToolTip, EuiButtonEmpty } from '@elastic/eui';

import { EuiLinkTo } from '../../../../shared/react_router_helpers';
import { ENGINE_SCHEMA_PATH } from '../../../routes';
import { generateEncodedPath } from '../../../utils/encode_path_params';

interface Props {
engines?: string[];
cutoff?: number;
}

export const TruncatedEnginesList: React.FC<Props> = ({ engines, cutoff = 3 }) => {
if (!engines?.length) return null;

const displayedEngines = engines.slice(0, cutoff);
const hiddenEngines = engines.slice(cutoff);
const SEPARATOR = ', ';
Copy link
Copy Markdown
Member

@JasonStoltz JasonStoltz May 14, 2021

Choose a reason for hiding this comment

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

I actually wonder if this should be i18ned as well.

I did some digging in the localization channel in slack, I think if you pass an array of values as a value to IntlMessageFormat i18n as an array value, it could do the concatenation for you.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the difficulty here is that I'm using SEPARATOR in various places and not just as a joined array (e.g. conditionally if the tooltip truncation kicks in, in a .map because we're using EuiLinkTo for the engine names, etc. 🤔

To be honest, I think a better approach might be to use something like <EuiBadge> or some other EUI list component to list out engines in any case... Maybe just copy what we do for analytics tags:

Our tag truncation component (link) also uses and X more text instead of a "+X" copy. We avoid all the i18n shenanigans with commas AND the + with that approach 🤔

@daveyholler what do you think?... would engine names in badges be weird?


return (
<EuiText size="s">
{displayedEngines.map((engineName, i) => {
const isLast = i === displayedEngines.length - 1;
return (
<Fragment key={engineName}>
<EuiLinkTo
to={generateEncodedPath(ENGINE_SCHEMA_PATH, { engineName })}
data-test-subj="displayedEngine"
>
{engineName}
</EuiLinkTo>
Comment on lines +34 to +39
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hopefully y'all agree the source engine links are helpful! Very pleased it's such an easy thing for us to do now in Kibana.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very helpful, IMO.

{!isLast ? SEPARATOR : ''}
</Fragment>
);
})}
{hiddenEngines.length > 0 && (
<>
{SEPARATOR}
<EuiToolTip
position="bottom"
content={hiddenEngines.join(SEPARATOR)}
data-test-subj="hiddenEnginesTooltip"
>
<EuiButtonEmpty size="xs" flush="both">
+{hiddenEngines.length}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we i18n this?

I didn't test this, but would it be something like...

Suggested change
+{hiddenEngines.length}
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.schema.metaEngine.additionalEnginesCount',
{ defaultMessage: '+{count, number}', values: { count: hiddenEngines.length } }
})

I'm actually unsure of what the best practice is here. If you're unsure also maybe we can inquire with #kibana-localization

Or perhaps just a FormattedNumber would work here. I guess it depends on whether the + is significant to translate to other languages.

</EuiButtonEmpty>
</EuiToolTip>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Screenshot of truncation/tooltip behavior:

</>
)}
</EuiText>
);
};
Loading