From 324a428ec38a6c4fd1f6ca043f2ac4780405c828 Mon Sep 17 00:00:00 2001 From: Sachin Chaurasiya Date: Thu, 20 Apr 2023 17:44:20 +0530 Subject: [PATCH] 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 --- .../Steps/ConfigureIngestion.test.tsx | 11 --- .../AddIngestion/Steps/ConfigureIngestion.tsx | 83 +++++++++---------- .../Steps/ScheduleInterval.test.tsx | 6 -- .../ObjectFieldTemplate.tsx | 42 ++++++---- .../RichTextEditorPreviewer.less | 12 +-- .../toggle-switch/ToggleSwitchV1.test.tsx | 32 ------- .../common/toggle-switch/ToggleSwitchV1.tsx | 40 --------- .../resources/ui/src/utils/ServiceUtils.tsx | 3 + .../main/resources/ui/src/utils/formUtils.tsx | 30 ++++--- 9 files changed, 94 insertions(+), 165 deletions(-) delete mode 100644 openmetadata-ui/src/main/resources/ui/src/components/common/toggle-switch/ToggleSwitchV1.test.tsx delete mode 100644 openmetadata-ui/src/main/resources/ui/src/components/common/toggle-switch/ToggleSwitchV1.tsx diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.test.tsx index 2aced715494..8bc6d2f8439 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.test.tsx @@ -29,12 +29,6 @@ jest.mock('../../common/FilterPattern/FilterPattern', () => { return jest.fn().mockImplementation(() =>
FilterPattern.component
); }); -jest.mock('../../common/toggle-switch/ToggleSwitchV1', () => { - return jest - .fn() - .mockImplementation(() =>
ToggleSwitchV1.component
); -}); - const mockConfigureIngestion: ConfigureIngestionProps = { pipelineType: PipelineType.Metadata, formType: FormSubmitType.EDIT, @@ -130,15 +124,10 @@ describe('Test ConfigureIngestion component', () => { container, 'FilterPattern.component' ); - const toggleSwitch = await findAllByText( - container, - 'ToggleSwitchV1.component' - ); expect(configureIngestionContainer).toBeInTheDocument(); expect(backButton).toBeInTheDocument(); expect(nextButton).toBeInTheDocument(); expect(filterPatternComponents).toHaveLength(3); - expect(toggleSwitch).toHaveLength(5); }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.tsx index baca36dbdc6..b8bff93398c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ConfigureIngestion.tsx @@ -11,7 +11,7 @@ * limitations under the License. */ -import { Button, Form } from 'antd'; +import { Button, Form, Space } from 'antd'; import { capitalize, isNil } from 'lodash'; import React, { useMemo, useRef } from 'react'; import { useTranslation } from 'react-i18next'; @@ -23,7 +23,6 @@ import { ServiceCategory } from '../../../enums/service.enum'; import { ProfileSampleType } from '../../../generated/entity/data/table'; import { PipelineType } from '../../../generated/entity/services/ingestionPipelines/ingestionPipeline'; import { EditorContentRef } from '../../common/rich-text-editor/RichTextEditor.interface'; -import { Field } from '../../Field/Field'; import { AddIngestionState, ConfigureIngestionProps, @@ -310,8 +309,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: includeTags, - handleCheck: handleIncludeTags, - testId: 'include-tags', + onChange: handleIncludeTags, + 'data-testid': 'toggle-button-include-tags', }, id: 'root/includeTags', hasSeparator: true, @@ -325,8 +324,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: enableDebugLog, - handleCheck: handleEnableDebugLogCheck, - testId: 'enable-debug-log', + onChange: handleEnableDebugLogCheck, + 'data-testid': 'toggle-button-enable-debug-log', }, id: 'root/loggerLevel', hasSeparator: true, @@ -342,8 +341,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: includeDataModels, - handleCheck: handleIncludeDataModels, - testId: 'include-data-models', + onChange: handleIncludeDataModels, + 'data-testid': 'toggle-button-include-data-models', }, id: 'root/includeDataModels', hasSeparator: true, @@ -359,8 +358,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: ingestSampleData, - handleCheck: handleIngestSampleToggle, - testId: 'ingest-sample-data', + onChange: handleIngestSampleToggle, + 'data-testid': 'toggle-button-ingest-sample-data', }, id: 'root/generateSampleData', hasSeparator: true, @@ -410,7 +409,7 @@ const ConfigureIngestion = ({ required: false, props: { checked: useFqnFilter, - handleCheck: handleFqnFilter, + onChange: handleFqnFilter, }, id: 'root/useFqnForFiltering', hasSeparator: true, @@ -423,8 +422,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: includeView, - handleCheck: handleIncludeViewToggle, - testId: 'include-views', + onChange: handleIncludeViewToggle, + 'data-testid': 'toggle-button-include-views', }, id: 'root/includeViews', hasSeparator: true, @@ -441,8 +440,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: markDeletedTables, - handleCheck: handleMarkDeletedTables, - testId: 'mark-deleted', + onChange: handleMarkDeletedTables, + 'data-testid': 'toggle-button-mark-deleted', }, id: 'root/markDeletedTables', hasSeparator: true, @@ -457,8 +456,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: markAllDeletedTables, - handleCheck: handleMarkAllDeletedTables, - testId: 'mark-deleted-filter-only', + onChange: handleMarkAllDeletedTables, + 'data-testid': 'toggle-button-mark-deleted-filter-only', }, id: 'root/markAllDeletedTables', hasSeparator: true, @@ -548,8 +547,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: includeOwner, - handleCheck: handleIncludeOwner, - testId: 'enabled-override-owner', + onChange: handleIncludeOwner, + 'data-testid': 'toggle-button-enabled-override-owner', }, id: 'root/overrideOwner', hasSeparator: true, @@ -566,8 +565,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: markDeletedDashboards, - handleCheck: handleMarkDeletedDashboards, - testId: 'mark-deleted', + onChange: handleMarkDeletedDashboards, + 'data-testid': 'toggle-button-mark-deleted', }, id: 'root/markDeletedDashboards', hasSeparator: true, @@ -608,8 +607,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: markDeletedTopics, - handleCheck: handleMarkDeletedTopics, - testId: 'mark-deleted', + onChange: handleMarkDeletedTopics, + 'data-testid': 'toggle-button-mark-deleted', }, id: 'root/markDeletedTopics', hasSeparator: true, @@ -648,8 +647,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: includeLineage, - handleCheck: handleIncludeLineage, - testId: 'include-lineage', + onChange: handleIncludeLineage, + 'data-testid': 'toggle-button-include-lineage', }, id: 'root/includeLineage', hasSeparator: true, @@ -666,8 +665,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: markDeletedPipelines, - handleCheck: handleMarkDeletedPipelines, - testId: 'mark-deleted', + onChange: handleMarkDeletedPipelines, + 'data-testid': 'toggle-button-mark-deleted', }, id: 'root/markDeletedPipelines', hasSeparator: true, @@ -706,8 +705,8 @@ const ConfigureIngestion = ({ required: false, props: { checked: markDeletedMlModels, - handleCheck: handleMarkDeletedMlModels, - testId: 'mark-deleted', + onChange: handleMarkDeletedMlModels, + 'data-testid': 'toggle-button-mark-deleted', }, id: 'root/markDeletedMlModels', hasSeparator: true, @@ -799,7 +798,7 @@ const ConfigureIngestion = ({ value: stageFileLocation, onChange: handleStageFileLocation, }, - id: 'stageFileLocation', + id: 'root/stageFileLocation', required: false, }, rateLimitField, @@ -825,7 +824,7 @@ const ConfigureIngestion = ({ ...databaseServiceFilterPatternFields, { name: 'profileSampleType', - id: 'profileSampleType', + id: 'root/profileSampleType', label: t('label.profile-sample-type', { type: t('label.type'), }), @@ -842,7 +841,7 @@ const ConfigureIngestion = ({ ? [ { name: 'profileSample', - id: 'profileSample', + id: 'root/profileSample', label: capitalize(ProfileSampleType.Percentage), helperText: t('message.profile-sample-percentage-message'), required: false, @@ -858,7 +857,7 @@ const ConfigureIngestion = ({ ? [ { name: 'profileSample', - id: 'profileSample', + id: 'root/profileSample', label: capitalize(ProfileSampleType.Rows), helperText: t('message.profile-sample-row-count-message'), required: false, @@ -878,7 +877,7 @@ const ConfigureIngestion = ({ : []), { name: 'threadCount', - id: 'threadCount', + id: 'root/threadCount', helperText: t('message.thread-count-message'), label: t('label.entity-count', { entity: t('label.thread'), @@ -896,7 +895,7 @@ const ConfigureIngestion = ({ }, { name: 'timeoutSeconds', - id: 'timeoutSeconds', + id: 'root/timeoutSeconds', helperText: t('message.profiler-timeout-seconds-message'), label: t('label.profiler-timeout-second-plural-label'), required: false, @@ -919,10 +918,10 @@ const ConfigureIngestion = ({ required: false, props: { checked: processPii, - handleCheck: handleProcessPii, - testId: 'process-pii', + onChange: handleProcessPii, + 'data-testid': 'toggle-button-process-pii', }, - id: 'processPiiSensitive', + id: 'root/processPiiSensitive', hasSeparator: processPii, helperText: t('message.process-pii-sensitive-column-message-profiler'), }, @@ -930,7 +929,7 @@ const ConfigureIngestion = ({ ? [ { name: 'confidence', - id: 'confidence', + id: 'root/confidence', label: null, helperText: t('message.confidence-percentage-message'), required: false, @@ -944,7 +943,7 @@ const ConfigureIngestion = ({ : []), { name: 'description', - id: 'description', + id: 'root/description', label: t('label.description'), helperText: t('message.pipeline-description-message'), required: false, @@ -998,7 +997,7 @@ const ConfigureIngestion = ({ }}> {getIngestionPipelineFields()} - + - + ); }; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ScheduleInterval.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ScheduleInterval.test.tsx index 3a27e6abdcf..88ec8d9727c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ScheduleInterval.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AddIngestion/Steps/ScheduleInterval.test.tsx @@ -20,12 +20,6 @@ jest.mock('../../common/CronEditor/CronEditor', () => { return jest.fn().mockImplementation(() =>
CronEditor.component
); }); -jest.mock('../../common/toggle-switch/ToggleSwitchV1', () => { - return jest - .fn() - .mockImplementation(() =>
ToggleSwitchV1.component
); -}); - const mockScheduleIntervalProps: ScheduleIntervalProps = { status: 'initial', repeatFrequency: '', diff --git a/openmetadata-ui/src/main/resources/ui/src/components/JSONSchemaTemplate/ObjectFieldTemplate.tsx b/openmetadata-ui/src/main/resources/ui/src/components/JSONSchemaTemplate/ObjectFieldTemplate.tsx index 2806d458e0a..f59a2c96118 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/JSONSchemaTemplate/ObjectFieldTemplate.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/JSONSchemaTemplate/ObjectFieldTemplate.tsx @@ -13,34 +13,40 @@ import { PlusOutlined } from '@ant-design/icons'; import { ObjectFieldTemplateProps } from '@rjsf/core'; +import { Button, Col, Row } from 'antd'; import classNames from 'classnames'; import React, { Fragment, FunctionComponent } from 'react'; -import { Button } from '../buttons/Button/Button'; export const ObjectFieldTemplate: FunctionComponent = (props: ObjectFieldTemplateProps) => { return ( -
- + + + + {props.schema.additionalProperties && ( - + +
+ {props.properties.map((element, index) => (
{ - it('SuccessScreen component should render', async () => { - const { container } = render( - - ); - - const toggleButton = await findByTestId(container, 'toggle-button'); - fireEvent.click(toggleButton); - - expect(toggleButton).toBeInTheDocument(); - expect(mockFn).toHaveBeenCalled(); - }); -}); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/toggle-switch/ToggleSwitchV1.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/toggle-switch/ToggleSwitchV1.tsx deleted file mode 100644 index 17eb7975255..00000000000 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/toggle-switch/ToggleSwitchV1.tsx +++ /dev/null @@ -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 ( -
-
-
- ); -}; - -export default ToggleSwitchV1; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/ServiceUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/ServiceUtils.tsx index 4a435448508..2452c0ef3b0 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/ServiceUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/ServiceUtils.tsx @@ -270,6 +270,9 @@ export const serviceTypeLogo = (type: string) => { case PipelineServiceType.DomoPipeline: return DOMO; + case PipelineServiceType.DatabricksPipeline: + return DATABRICK; + case MlModelServiceType.Mlflow: return MLFLOW; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx index 972085823e4..9f5b102848a 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx @@ -11,21 +11,20 @@ * limitations under the License. */ import { + Col, Divider, Form, FormRule, Input, InputNumber, + Row, Select, - Space, + Switch, } from 'antd'; import FilterPattern from 'components/common/FilterPattern/FilterPattern'; import { FilterPatternProps } from 'components/common/FilterPattern/filterPattern.interface'; import RichTextEditor from 'components/common/rich-text-editor/RichTextEditor'; 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 { SliderWithInputProps } from 'components/SliderWithInput/SliderWithInput.interface'; import React, { ReactNode } from 'react'; @@ -80,11 +79,18 @@ export const getField = (field: FieldProp) => { switch (type) { case FieldTypes.TEXT: - fieldElement = ; + fieldElement = ; break; case FieldTypes.NUMBER: - fieldElement = ; + fieldElement = ( + + ); break; @@ -97,15 +103,17 @@ export const getField = (field: FieldProp) => { case FieldTypes.SWITCH: fieldElement = ( - - {label} - - + + {label} + + + + ); break; case FieldTypes.SELECT: - fieldElement = ; break; case FieldTypes.SLIDER_INPUT: