chore(ui): improve the spacing and highlighting in service workflow form (#11146)

* chore(ui): improve the spacing between switch/action and label in ingestion UI

* fix: negative margin spacing

* chore: increase spacing

* fix: missing icon for databrick pipeline

* fix: copy button styling

* fix: field names

* address comment

* fix: cy test

* fix: markdown first para spacing
This commit is contained in:
Sachin Chaurasiya 2023-04-20 17:44:20 +05:30 committed by GitHub
parent e62a2a80bc
commit 324a428ec3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 94 additions and 165 deletions

View File

@ -29,12 +29,6 @@ jest.mock('../../common/FilterPattern/FilterPattern', () => {
return jest.fn().mockImplementation(() => <div>FilterPattern.component</div>); return jest.fn().mockImplementation(() => <div>FilterPattern.component</div>);
}); });
jest.mock('../../common/toggle-switch/ToggleSwitchV1', () => {
return jest
.fn()
.mockImplementation(() => <div>ToggleSwitchV1.component</div>);
});
const mockConfigureIngestion: ConfigureIngestionProps = { const mockConfigureIngestion: ConfigureIngestionProps = {
pipelineType: PipelineType.Metadata, pipelineType: PipelineType.Metadata,
formType: FormSubmitType.EDIT, formType: FormSubmitType.EDIT,
@ -130,15 +124,10 @@ describe('Test ConfigureIngestion component', () => {
container, container,
'FilterPattern.component' 'FilterPattern.component'
); );
const toggleSwitch = await findAllByText(
container,
'ToggleSwitchV1.component'
);
expect(configureIngestionContainer).toBeInTheDocument(); expect(configureIngestionContainer).toBeInTheDocument();
expect(backButton).toBeInTheDocument(); expect(backButton).toBeInTheDocument();
expect(nextButton).toBeInTheDocument(); expect(nextButton).toBeInTheDocument();
expect(filterPatternComponents).toHaveLength(3); expect(filterPatternComponents).toHaveLength(3);
expect(toggleSwitch).toHaveLength(5);
}); });
}); });

View File

@ -11,7 +11,7 @@
* limitations under the License. * limitations under the License.
*/ */
import { Button, Form } from 'antd'; import { Button, Form, Space } from 'antd';
import { capitalize, isNil } from 'lodash'; import { capitalize, isNil } from 'lodash';
import React, { useMemo, useRef } from 'react'; import React, { useMemo, useRef } from 'react';
import { useTranslation } from 'react-i18next'; import { useTranslation } from 'react-i18next';
@ -23,7 +23,6 @@ import { ServiceCategory } from '../../../enums/service.enum';
import { ProfileSampleType } from '../../../generated/entity/data/table'; import { ProfileSampleType } from '../../../generated/entity/data/table';
import { PipelineType } from '../../../generated/entity/services/ingestionPipelines/ingestionPipeline'; import { PipelineType } from '../../../generated/entity/services/ingestionPipelines/ingestionPipeline';
import { EditorContentRef } from '../../common/rich-text-editor/RichTextEditor.interface'; import { EditorContentRef } from '../../common/rich-text-editor/RichTextEditor.interface';
import { Field } from '../../Field/Field';
import { import {
AddIngestionState, AddIngestionState,
ConfigureIngestionProps, ConfigureIngestionProps,
@ -310,8 +309,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: includeTags, checked: includeTags,
handleCheck: handleIncludeTags, onChange: handleIncludeTags,
testId: 'include-tags', 'data-testid': 'toggle-button-include-tags',
}, },
id: 'root/includeTags', id: 'root/includeTags',
hasSeparator: true, hasSeparator: true,
@ -325,8 +324,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: enableDebugLog, checked: enableDebugLog,
handleCheck: handleEnableDebugLogCheck, onChange: handleEnableDebugLogCheck,
testId: 'enable-debug-log', 'data-testid': 'toggle-button-enable-debug-log',
}, },
id: 'root/loggerLevel', id: 'root/loggerLevel',
hasSeparator: true, hasSeparator: true,
@ -342,8 +341,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: includeDataModels, checked: includeDataModels,
handleCheck: handleIncludeDataModels, onChange: handleIncludeDataModels,
testId: 'include-data-models', 'data-testid': 'toggle-button-include-data-models',
}, },
id: 'root/includeDataModels', id: 'root/includeDataModels',
hasSeparator: true, hasSeparator: true,
@ -359,8 +358,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: ingestSampleData, checked: ingestSampleData,
handleCheck: handleIngestSampleToggle, onChange: handleIngestSampleToggle,
testId: 'ingest-sample-data', 'data-testid': 'toggle-button-ingest-sample-data',
}, },
id: 'root/generateSampleData', id: 'root/generateSampleData',
hasSeparator: true, hasSeparator: true,
@ -410,7 +409,7 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: useFqnFilter, checked: useFqnFilter,
handleCheck: handleFqnFilter, onChange: handleFqnFilter,
}, },
id: 'root/useFqnForFiltering', id: 'root/useFqnForFiltering',
hasSeparator: true, hasSeparator: true,
@ -423,8 +422,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: includeView, checked: includeView,
handleCheck: handleIncludeViewToggle, onChange: handleIncludeViewToggle,
testId: 'include-views', 'data-testid': 'toggle-button-include-views',
}, },
id: 'root/includeViews', id: 'root/includeViews',
hasSeparator: true, hasSeparator: true,
@ -441,8 +440,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: markDeletedTables, checked: markDeletedTables,
handleCheck: handleMarkDeletedTables, onChange: handleMarkDeletedTables,
testId: 'mark-deleted', 'data-testid': 'toggle-button-mark-deleted',
}, },
id: 'root/markDeletedTables', id: 'root/markDeletedTables',
hasSeparator: true, hasSeparator: true,
@ -457,8 +456,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: markAllDeletedTables, checked: markAllDeletedTables,
handleCheck: handleMarkAllDeletedTables, onChange: handleMarkAllDeletedTables,
testId: 'mark-deleted-filter-only', 'data-testid': 'toggle-button-mark-deleted-filter-only',
}, },
id: 'root/markAllDeletedTables', id: 'root/markAllDeletedTables',
hasSeparator: true, hasSeparator: true,
@ -548,8 +547,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: includeOwner, checked: includeOwner,
handleCheck: handleIncludeOwner, onChange: handleIncludeOwner,
testId: 'enabled-override-owner', 'data-testid': 'toggle-button-enabled-override-owner',
}, },
id: 'root/overrideOwner', id: 'root/overrideOwner',
hasSeparator: true, hasSeparator: true,
@ -566,8 +565,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: markDeletedDashboards, checked: markDeletedDashboards,
handleCheck: handleMarkDeletedDashboards, onChange: handleMarkDeletedDashboards,
testId: 'mark-deleted', 'data-testid': 'toggle-button-mark-deleted',
}, },
id: 'root/markDeletedDashboards', id: 'root/markDeletedDashboards',
hasSeparator: true, hasSeparator: true,
@ -608,8 +607,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: markDeletedTopics, checked: markDeletedTopics,
handleCheck: handleMarkDeletedTopics, onChange: handleMarkDeletedTopics,
testId: 'mark-deleted', 'data-testid': 'toggle-button-mark-deleted',
}, },
id: 'root/markDeletedTopics', id: 'root/markDeletedTopics',
hasSeparator: true, hasSeparator: true,
@ -648,8 +647,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: includeLineage, checked: includeLineage,
handleCheck: handleIncludeLineage, onChange: handleIncludeLineage,
testId: 'include-lineage', 'data-testid': 'toggle-button-include-lineage',
}, },
id: 'root/includeLineage', id: 'root/includeLineage',
hasSeparator: true, hasSeparator: true,
@ -666,8 +665,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: markDeletedPipelines, checked: markDeletedPipelines,
handleCheck: handleMarkDeletedPipelines, onChange: handleMarkDeletedPipelines,
testId: 'mark-deleted', 'data-testid': 'toggle-button-mark-deleted',
}, },
id: 'root/markDeletedPipelines', id: 'root/markDeletedPipelines',
hasSeparator: true, hasSeparator: true,
@ -706,8 +705,8 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: markDeletedMlModels, checked: markDeletedMlModels,
handleCheck: handleMarkDeletedMlModels, onChange: handleMarkDeletedMlModels,
testId: 'mark-deleted', 'data-testid': 'toggle-button-mark-deleted',
}, },
id: 'root/markDeletedMlModels', id: 'root/markDeletedMlModels',
hasSeparator: true, hasSeparator: true,
@ -799,7 +798,7 @@ const ConfigureIngestion = ({
value: stageFileLocation, value: stageFileLocation,
onChange: handleStageFileLocation, onChange: handleStageFileLocation,
}, },
id: 'stageFileLocation', id: 'root/stageFileLocation',
required: false, required: false,
}, },
rateLimitField, rateLimitField,
@ -825,7 +824,7 @@ const ConfigureIngestion = ({
...databaseServiceFilterPatternFields, ...databaseServiceFilterPatternFields,
{ {
name: 'profileSampleType', name: 'profileSampleType',
id: 'profileSampleType', id: 'root/profileSampleType',
label: t('label.profile-sample-type', { label: t('label.profile-sample-type', {
type: t('label.type'), type: t('label.type'),
}), }),
@ -842,7 +841,7 @@ const ConfigureIngestion = ({
? [ ? [
{ {
name: 'profileSample', name: 'profileSample',
id: 'profileSample', id: 'root/profileSample',
label: capitalize(ProfileSampleType.Percentage), label: capitalize(ProfileSampleType.Percentage),
helperText: t('message.profile-sample-percentage-message'), helperText: t('message.profile-sample-percentage-message'),
required: false, required: false,
@ -858,7 +857,7 @@ const ConfigureIngestion = ({
? [ ? [
{ {
name: 'profileSample', name: 'profileSample',
id: 'profileSample', id: 'root/profileSample',
label: capitalize(ProfileSampleType.Rows), label: capitalize(ProfileSampleType.Rows),
helperText: t('message.profile-sample-row-count-message'), helperText: t('message.profile-sample-row-count-message'),
required: false, required: false,
@ -878,7 +877,7 @@ const ConfigureIngestion = ({
: []), : []),
{ {
name: 'threadCount', name: 'threadCount',
id: 'threadCount', id: 'root/threadCount',
helperText: t('message.thread-count-message'), helperText: t('message.thread-count-message'),
label: t('label.entity-count', { label: t('label.entity-count', {
entity: t('label.thread'), entity: t('label.thread'),
@ -896,7 +895,7 @@ const ConfigureIngestion = ({
}, },
{ {
name: 'timeoutSeconds', name: 'timeoutSeconds',
id: 'timeoutSeconds', id: 'root/timeoutSeconds',
helperText: t('message.profiler-timeout-seconds-message'), helperText: t('message.profiler-timeout-seconds-message'),
label: t('label.profiler-timeout-second-plural-label'), label: t('label.profiler-timeout-second-plural-label'),
required: false, required: false,
@ -919,10 +918,10 @@ const ConfigureIngestion = ({
required: false, required: false,
props: { props: {
checked: processPii, checked: processPii,
handleCheck: handleProcessPii, onChange: handleProcessPii,
testId: 'process-pii', 'data-testid': 'toggle-button-process-pii',
}, },
id: 'processPiiSensitive', id: 'root/processPiiSensitive',
hasSeparator: processPii, hasSeparator: processPii,
helperText: t('message.process-pii-sensitive-column-message-profiler'), helperText: t('message.process-pii-sensitive-column-message-profiler'),
}, },
@ -930,7 +929,7 @@ const ConfigureIngestion = ({
? [ ? [
{ {
name: 'confidence', name: 'confidence',
id: 'confidence', id: 'root/confidence',
label: null, label: null,
helperText: t('message.confidence-percentage-message'), helperText: t('message.confidence-percentage-message'),
required: false, required: false,
@ -944,7 +943,7 @@ const ConfigureIngestion = ({
: []), : []),
{ {
name: 'description', name: 'description',
id: 'description', id: 'root/description',
label: t('label.description'), label: t('label.description'),
helperText: t('message.pipeline-description-message'), helperText: t('message.pipeline-description-message'),
required: false, required: false,
@ -998,7 +997,7 @@ const ConfigureIngestion = ({
}}> }}>
{getIngestionPipelineFields()} {getIngestionPipelineFields()}
<Field className="d-flex justify-end"> <Space className="w-full justify-end">
<Button <Button
className="m-r-xs" className="m-r-xs"
data-testid="back-button" data-testid="back-button"
@ -1014,7 +1013,7 @@ const ConfigureIngestion = ({
onClick={handleNext}> onClick={handleNext}>
<span>{t('label.next')}</span> <span>{t('label.next')}</span>
</Button> </Button>
</Field> </Space>
</Form> </Form>
); );
}; };

View File

@ -20,12 +20,6 @@ jest.mock('../../common/CronEditor/CronEditor', () => {
return jest.fn().mockImplementation(() => <div>CronEditor.component</div>); return jest.fn().mockImplementation(() => <div>CronEditor.component</div>);
}); });
jest.mock('../../common/toggle-switch/ToggleSwitchV1', () => {
return jest
.fn()
.mockImplementation(() => <div>ToggleSwitchV1.component</div>);
});
const mockScheduleIntervalProps: ScheduleIntervalProps = { const mockScheduleIntervalProps: ScheduleIntervalProps = {
status: 'initial', status: 'initial',
repeatFrequency: '', repeatFrequency: '',

View File

@ -13,34 +13,40 @@
import { PlusOutlined } from '@ant-design/icons'; import { PlusOutlined } from '@ant-design/icons';
import { ObjectFieldTemplateProps } from '@rjsf/core'; import { ObjectFieldTemplateProps } from '@rjsf/core';
import { Button, Col, Row } from 'antd';
import classNames from 'classnames'; import classNames from 'classnames';
import React, { Fragment, FunctionComponent } from 'react'; import React, { Fragment, FunctionComponent } from 'react';
import { Button } from '../buttons/Button/Button';
export const ObjectFieldTemplate: FunctionComponent<ObjectFieldTemplateProps> = export const ObjectFieldTemplate: FunctionComponent<ObjectFieldTemplateProps> =
(props: ObjectFieldTemplateProps) => { (props: ObjectFieldTemplateProps) => {
return ( return (
<Fragment> <Fragment>
<div className="tw-flex tw-justify-between tw-items-center"> <Row>
<label className="control-label" id={`${props.idSchema.$id}__title`}> <Col span={8}>
<label
className="control-label"
id={`${props.idSchema.$id}__title`}>
{props.title} {props.title}
</label> </label>
</Col>
{props.schema.additionalProperties && ( {props.schema.additionalProperties && (
<Col span={16}>
<Button <Button
className="tw-h-7 tw-w-7 tw-px-2"
data-testid={`add-item-${props.title}`} data-testid={`add-item-${props.title}`}
icon={
<PlusOutlined style={{ color: 'white', fontSize: '12px' }} />
}
id={`${props.idSchema.$id}__add`} id={`${props.idSchema.$id}__add`}
size="small" size="small"
theme="primary" type="primary"
variant="contained"
onClick={() => { onClick={() => {
props.onAddClick(props.schema)(); props.onAddClick(props.schema)();
}}> }}
<PlusOutlined /> />
</Button> </Col>
)} )}
</div> </Row>
{props.properties.map((element, index) => ( {props.properties.map((element, index) => (
<div <div
className={classNames('property-wrapper', { className={classNames('property-wrapper', {

View File

@ -60,7 +60,6 @@
border-left: 3px solid @primary-color; border-left: 3px solid @primary-color;
padding-bottom: 12px !important; padding-bottom: 12px !important;
margin-top: 12px; margin-top: 12px;
margin-bottom: -12px;
h1, h1,
h2, h2,
@ -144,6 +143,9 @@
color: @text-color; color: @text-color;
word-break: break-word; word-break: break-word;
line-height: 20px; line-height: 20px;
&:first-child {
margin-top: 0px;
}
&:last-child { &:last-child {
margin-bottom: 0px; margin-bottom: 0px;
} }
@ -215,12 +217,12 @@
} }
.code-copy-button { .code-copy-button {
position: absolute; position: absolute;
top: 8px; top: 16px;
right: 8px; right: 40px;
pointer-events: all; pointer-events: all;
cursor: pointer; cursor: pointer;
z-index: 1; z-index: 1;
width: 16px; width: 32px;
opacity: 0; opacity: 0;
padding: 8px; padding: 8px;
background: @border-color; background: @border-color;
@ -231,7 +233,7 @@
display: none; display: none;
position: absolute; position: absolute;
top: 16px; top: 16px;
right: 16px; right: 40px;
pointer-events: all; pointer-events: all;
cursor: pointer; cursor: pointer;
z-index: 2; z-index: 2;

View File

@ -1,32 +0,0 @@
/*
* Copyright 2022 Collate.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { findByTestId, fireEvent, render } from '@testing-library/react';
import React from 'react';
import ToggleSwitchV1 from './ToggleSwitchV1';
const mockFn = jest.fn();
describe('Test SuccessScreen component', () => {
it('SuccessScreen component should render', async () => {
const { container } = render(
<ToggleSwitchV1 checked={false} handleCheck={mockFn} />
);
const toggleButton = await findByTestId(container, 'toggle-button');
fireEvent.click(toggleButton);
expect(toggleButton).toBeInTheDocument();
expect(mockFn).toHaveBeenCalled();
});
});

View File

@ -1,40 +0,0 @@
/*
* Copyright 2022 Collate.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import classNames from 'classnames';
import React from 'react';
export interface ToggleSwitchV1Props {
checked: boolean;
handleCheck: () => void;
testId?: string;
}
const ToggleSwitchV1 = ({
checked,
handleCheck,
testId,
}: ToggleSwitchV1Props) => {
const id = testId ? `toggle-button-${testId}` : 'toggle-button';
return (
<div
className={classNames('toggle-switch', checked ? 'open' : null)}
data-testid={id}
onClick={handleCheck}>
<div className="switch" />
</div>
);
};
export default ToggleSwitchV1;

View File

@ -270,6 +270,9 @@ export const serviceTypeLogo = (type: string) => {
case PipelineServiceType.DomoPipeline: case PipelineServiceType.DomoPipeline:
return DOMO; return DOMO;
case PipelineServiceType.DatabricksPipeline:
return DATABRICK;
case MlModelServiceType.Mlflow: case MlModelServiceType.Mlflow:
return MLFLOW; return MLFLOW;

View File

@ -11,21 +11,20 @@
* limitations under the License. * limitations under the License.
*/ */
import { import {
Col,
Divider, Divider,
Form, Form,
FormRule, FormRule,
Input, Input,
InputNumber, InputNumber,
Row,
Select, Select,
Space, Switch,
} from 'antd'; } from 'antd';
import FilterPattern from 'components/common/FilterPattern/FilterPattern'; import FilterPattern from 'components/common/FilterPattern/FilterPattern';
import { FilterPatternProps } from 'components/common/FilterPattern/filterPattern.interface'; import { FilterPatternProps } from 'components/common/FilterPattern/filterPattern.interface';
import RichTextEditor from 'components/common/rich-text-editor/RichTextEditor'; import RichTextEditor from 'components/common/rich-text-editor/RichTextEditor';
import { RichTextEditorProp } from 'components/common/rich-text-editor/RichTextEditor.interface'; import { RichTextEditorProp } from 'components/common/rich-text-editor/RichTextEditor.interface';
import ToggleSwitchV1, {
ToggleSwitchV1Props,
} from 'components/common/toggle-switch/ToggleSwitchV1';
import SliderWithInput from 'components/SliderWithInput/SliderWithInput'; import SliderWithInput from 'components/SliderWithInput/SliderWithInput';
import { SliderWithInputProps } from 'components/SliderWithInput/SliderWithInput.interface'; import { SliderWithInputProps } from 'components/SliderWithInput/SliderWithInput.interface';
import React, { ReactNode } from 'react'; import React, { ReactNode } from 'react';
@ -80,11 +79,18 @@ export const getField = (field: FieldProp) => {
switch (type) { switch (type) {
case FieldTypes.TEXT: case FieldTypes.TEXT:
fieldElement = <Input {...props} placeholder={placeholder} />; fieldElement = <Input {...props} id={id} placeholder={placeholder} />;
break; break;
case FieldTypes.NUMBER: case FieldTypes.NUMBER:
fieldElement = <InputNumber {...props} placeholder={placeholder} />; fieldElement = (
<InputNumber
{...props}
id={id}
placeholder={placeholder}
size="small"
/>
);
break; break;
@ -97,15 +103,17 @@ export const getField = (field: FieldProp) => {
case FieldTypes.SWITCH: case FieldTypes.SWITCH:
fieldElement = ( fieldElement = (
<Space> <Row>
{label} <Col span={8}>{label}</Col>
<ToggleSwitchV1 {...(props as unknown as ToggleSwitchV1Props)} /> <Col span={16}>
</Space> <Switch {...props} id={id} />
</Col>
</Row>
); );
break; break;
case FieldTypes.SELECT: case FieldTypes.SELECT:
fieldElement = <Select {...props} />; fieldElement = <Select {...props} id={id} />;
break; break;
case FieldTypes.SLIDER_INPUT: case FieldTypes.SLIDER_INPUT: