Skip to content

Commit

Permalink
[Enterprise Search] Fix unstyled UI for Schema Errors (#97776)
Browse files Browse the repository at this point in the history
* Refactor main component

* Refactor faux view button

The original design called for a button-looking component on the accordion header so it looks clickable to reveal the error underneath. Using a button element caused the console to error because the entire header is a button and this is a child. Adding an href fixes the error and allows for styling as a button

* Use empty button rather than styles components for view button

* Remove extra classNames and fix layout to stretch cells

* Add stylesheet

Without the width: 100% on .euiIEFlexWrapFix, the header table does not stretch across the screen. Couldn’t find another way around this and happy to take suggestions of a better idea.

* Add prop to prevent line break on IDs

Existing implementation was causing multi-character IDs to break to a new line. So for 12, its was:

1
2
  • Loading branch information
scottybollinger authored Apr 22, 2021
1 parent 967b172 commit 65287df
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 45 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* 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.
*/

.schemaFieldError {
border-top: 1px solid $euiColorLightShade;

&:last-child {
border-bottom: 1px solid $euiColorLightShade;
}

// Something about the EuiFlexGroup being inside a button collapses the row of items.
// This wrapper div was injected by EUI and had 'with: auto' on it.
.euiIEFlexWrapFix {
width: 100%;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { shallow } from 'enzyme';

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

import { EuiLinkTo } from '../react_router_helpers';
import { EuiButtonEmptyTo } from '../react_router_helpers';

import { SchemaErrorsAccordion } from './schema_errors_accordion';

Expand Down Expand Up @@ -40,12 +40,12 @@ describe('SchemaErrorsAccordion', () => {

expect(wrapper.find(EuiAccordion)).toHaveLength(1);
expect(wrapper.find(EuiTableRow)).toHaveLength(2);
expect(wrapper.find(EuiLinkTo)).toHaveLength(0);
expect(wrapper.find(EuiButtonEmptyTo)).toHaveLength(0);
});

it('renders document buttons', () => {
const wrapper = shallow(<SchemaErrorsAccordion {...props} itemId="123" getRoute={jest.fn()} />);

expect(wrapper.find(EuiLinkTo)).toHaveLength(2);
expect(wrapper.find(EuiButtonEmptyTo)).toHaveLength(2);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import React from 'react';

import {
EuiAccordion,
EuiButton,
EuiFlexGroup,
EuiFlexItem,
EuiTable,
Expand All @@ -19,10 +20,12 @@ import {
EuiTableRowCell,
} from '@elastic/eui';

import { EuiLinkTo } from '../react_router_helpers';
import { EuiButtonEmptyTo } from '../react_router_helpers';

import { TruncatedContent } from '../truncate';

import './schema_errors_accordion.scss';

import {
ERROR_TABLE_ID_HEADER,
ERROR_TABLE_ERROR_HEADER,
Expand Down Expand Up @@ -60,14 +63,19 @@ export const SchemaErrorsAccordion: React.FC<ISchemaErrorsAccordionProps> = ({
<EuiFlexGroup alignItems="center" justifyContent="spaceBetween" gutterSize="none">
<EuiFlexItem grow={false}>
<EuiFlexGroup alignItems="center" gutterSize="xl">
<EuiFlexItem className="field-error__field-name">
<TruncatedContent content={fieldName} length={32} />
<EuiFlexItem>
<strong>
<TruncatedContent content={fieldName} length={32} />
</strong>
</EuiFlexItem>
<EuiFlexItem className="field-error__field-type">{schema[fieldName]}</EuiFlexItem>
<EuiFlexItem>{schema[fieldName]}</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<span className="field-error__control button">{ERROR_TABLE_REVIEW_CONTROL}</span>
{/* href is needed here because a button cannot be nested in a button or console will error and EuiAccordion uses a button to wrap this. */}
<EuiButton size="s" href="#">
{ERROR_TABLE_REVIEW_CONTROL}
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
);
Expand All @@ -76,12 +84,12 @@ export const SchemaErrorsAccordion: React.FC<ISchemaErrorsAccordionProps> = ({
<EuiAccordion
key={fieldNameIndex}
id={`accordion${fieldNameIndex}`}
className="euiAccordionForm field-error"
className="schemaFieldError"
buttonClassName="euiAccordionForm__button field-error__header"
buttonContent={accordionHeader}
paddingSize="xl"
>
<EuiTable>
<EuiTable tableLayout="auto">
<EuiTableHeader>
<EuiTableHeaderCell>{ERROR_TABLE_ID_HEADER}</EuiTableHeaderCell>
<EuiTableHeaderCell>{ERROR_TABLE_ERROR_HEADER}</EuiTableHeaderCell>
Expand All @@ -93,34 +101,21 @@ export const SchemaErrorsAccordion: React.FC<ISchemaErrorsAccordionProps> = ({
const documentPath = getRoute && itemId ? getRoute(itemId, error.external_id) : '';

const viewButton = showViewButton && (
<EuiTableRowCell className="field-error-document__actions">
<EuiLinkTo
className="euiButtonEmpty euiButtonEmpty--primary euiButtonEmpty--xSmall"
to={documentPath}
>
<span className="euiButtonEmpty__content">
<span className="euiButtonEmpty__text">{ERROR_TABLE_VIEW_LINK}</span>
</span>
</EuiLinkTo>
<EuiTableRowCell>
<EuiButtonEmptyTo to={documentPath}>{ERROR_TABLE_VIEW_LINK}</EuiButtonEmptyTo>
</EuiTableRowCell>
);

return (
<EuiTableRow
key={`schema-change-document-error-${fieldName}-${errorIndex} field-error-document`}
>
<EuiTableRowCell className="field-error-document__id">
<div className="data-type--id">
<TruncatedContent
tooltipType="title"
content={error.external_id}
length={22}
/>
</div>
</EuiTableRowCell>
<EuiTableRowCell className="field-error-document__field-content">
{error.error}
<EuiTableRow key={`schema-change-document-error-${fieldName}-${errorIndex}`}>
<EuiTableRowCell truncateText>
<TruncatedContent
tooltipType="title"
content={error.external_id}
length={22}
/>
</EuiTableRowCell>
<EuiTableRowCell>{error.error}</EuiTableRowCell>
{showViewButton ? viewButton : <EuiTableRowCell />}
</EuiTableRow>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import { useParams } from 'react-router-dom';

import { useActions, useValues } from 'kea';

import { EuiSpacer } from '@elastic/eui';

import { SchemaErrorsAccordion } from '../../../../../shared/schema/schema_errors_accordion';
import { ViewContentHeader } from '../../../../components/shared/view_content_header';

Expand All @@ -32,16 +30,13 @@ export const SchemaChangeErrors: React.FC = () => {
}, []);

return (
<div>
<>
<ViewContentHeader title={SCHEMA_ERRORS_HEADING} />
<EuiSpacer size="xl" />
<main>
<SchemaErrorsAccordion
fieldCoercionErrors={fieldCoercionErrors}
schema={serverSchema}
itemId={sourceId}
/>
</main>
</div>
<SchemaErrorsAccordion
fieldCoercionErrors={fieldCoercionErrors}
schema={serverSchema}
itemId={sourceId}
/>
</>
);
};

0 comments on commit 65287df

Please sign in to comment.