From 09c1d57ef91c8c991f12aa829ed07d2d3d6adcd0 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Tue, 15 Nov 2022 15:55:07 -0500 Subject: [PATCH 01/25] initial pass and cleanup --- .../next/CatalogTreeTableRow.module.scss | 10 ++- .../CatalogTree/next/CatalogTreeTableRow.tsx | 77 ++++++++++++++----- 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss index 787f8b34bb1b8..642acb682b36a 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss @@ -12,6 +12,10 @@ &.minus { color: colors.$red; } + + &.changed { + color: colors.$blue-900; + } } .streamHeaderContent { @@ -31,11 +35,11 @@ cursor: pointer; } - &.disabledChange { + &.removed { background-color: colors.$red-50; } - &.enabledChange { + &.added { background-color: colors.$green-50; } @@ -43,7 +47,7 @@ border: 1px solid colors.$red; } - &.selected { + &.changed { background-color: colors.$blue-transparent; } diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index 16708cd441845..564a95007d7e7 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -1,20 +1,24 @@ import { faArrowRight, faMinus, faPlus } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import classnames from "classnames"; +import classNames from "classnames"; import React, { useMemo } from "react"; import { FormattedMessage } from "react-intl"; +import { ModificationIcon } from "components/icons/ModificationIcon"; import { Cell, Row } from "components/SimpleTableComponents"; import { CheckBox } from "components/ui/CheckBox"; import { Switch } from "components/ui/Switch"; import { Text } from "components/ui/Text"; import { useBulkEditSelect } from "hooks/services/BulkEdit/BulkEditService"; +import { useConnectionFormService } from "hooks/services/ConnectionForm/ConnectionFormService"; import { StreamHeaderProps } from "../StreamHeader"; import styles from "./CatalogTreeTableRow.module.scss"; import { StreamPathSelect } from "./StreamPathSelect"; import { SyncModeSelect } from "./SyncModeSelect"; + export const CatalogTreeTableRow: React.FC = ({ stream, destName, @@ -30,7 +34,6 @@ export const CatalogTreeTableRow: React.FC = ({ // isRowExpanded, fields, onExpand, - changedSelected, hasError, disabled, }) => { @@ -46,38 +49,74 @@ export const CatalogTreeTableRow: React.FC = ({ [syncMode, destinationSyncMode] ); + const { initialValues } = useConnectionFormService(); + + const statusToDisplay = useMemo(() => { + const rowStatusChanged = + initialValues.syncCatalog.streams.find( + (item) => item.stream?.name === stream.stream?.name && item.stream?.namespace === stream.stream?.namespace + )?.config?.selected !== stream.config?.selected; + + const rowChanged = + initialValues.syncCatalog.streams.find( + (item) => item.stream?.name === stream.stream?.name && item.stream?.namespace === stream.stream?.namespace + )?.config !== stream.config; + + // todo: the following should also evaluate if the stream is "new" from a refresh + is also enabled + if (rowStatusChanged) { + return isStreamEnabled ? "added" : "removed"; + } + // todo: the following should also evaluate if there are changes from the diff + else if (rowChanged) { + return "changed"; + } else if (!isStreamEnabled && !rowStatusChanged) { + return "disabled"; + } + return "unchanged"; + }, [ + initialValues.syncCatalog.streams, + stream.config, + stream.stream?.name, + stream.stream?.namespace, + isStreamEnabled, + ]); + const [isSelected, selectForBulkEdit] = useBulkEditSelect(stream.id); const paths = useMemo(() => primitiveFields.map((field) => field.path), [primitiveFields]); const fieldCount = fields?.length ?? 0; const onRowClick = fieldCount > 0 ? () => onExpand() : undefined; - const iconStyle = classnames(styles.icon, { - [styles.plus]: isStreamEnabled, - [styles.minus]: !isStreamEnabled, - }); - const streamHeaderContentStyle = classnames(styles.streamHeaderContent, { - [styles.enabledChange]: changedSelected && isStreamEnabled, - [styles.disabledChange]: changedSelected && !isStreamEnabled, - [styles.selected]: isSelected, + [styles.added]: statusToDisplay === "added", + [styles.removed]: statusToDisplay === "removed", + [styles.changed]: isSelected || statusToDisplay === "changed", [styles.error]: hasError, - [styles.disabled]: !changedSelected && !isStreamEnabled, + [styles.disabled]: statusToDisplay === "disabled", }); + const statusIcon = useMemo(() => { + if (statusToDisplay === "added") { + return ; + } else if (statusToDisplay === "removed") { + return ; + } else if (statusToDisplay === "changed") { + return ( + // todo: styles need adjusting, especially color and possibly alignment + + + + ); + } + return null; + }, [statusToDisplay]); + + console.log({ statusToDisplay }); return ( {!disabled && (
- {changedSelected && ( -
- {isStreamEnabled ? ( - - ) : ( - - )} -
- )} + {statusIcon}
)} From c41f4d435ae84cf8e774f536497ccd0a126dd945 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Wed, 16 Nov 2022 12:01:46 -0500 Subject: [PATCH 02/25] merge cleanup --- .../connection/CatalogTree/next/CatalogTreeTableRow.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index 564a95007d7e7..c441d785d63f2 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -92,7 +92,6 @@ export const CatalogTreeTableRow: React.FC = ({ [styles.removed]: statusToDisplay === "removed", [styles.changed]: isSelected || statusToDisplay === "changed", [styles.error]: hasError, - [styles.disabled]: statusToDisplay === "disabled", }); const statusIcon = useMemo(() => { @@ -111,11 +110,14 @@ export const CatalogTreeTableRow: React.FC = ({ return null; }, [statusToDisplay]); + const checkboxCellCustomStyle = classnames(styles.checkboxCell, styles.streamRowCheckboxCell); + + console.log({ statusToDisplay }); console.log({ statusToDisplay }); return ( {!disabled && ( -
+
{statusIcon}
From 6c61f99ccab3dc298cf4ed0cccbe547129f6386b Mon Sep 17 00:00:00 2001 From: tealjulia Date: Wed, 16 Nov 2022 13:30:04 -0500 Subject: [PATCH 03/25] get dropdowns colored correctly also --- .../next/CatalogTreeTableRow.module.scss | 6 +-- .../CatalogTree/next/CatalogTreeTableRow.tsx | 50 ++++++++++++------- .../CatalogTree/next/StreamPathSelect.tsx | 2 + .../CatalogTree/next/SyncModeSelect.tsx | 12 ++++- airbyte-webapp/src/scss/_colors.scss | 2 + 5 files changed, 51 insertions(+), 21 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss index 642acb682b36a..17ebb292c5721 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss @@ -36,11 +36,11 @@ } &.removed { - background-color: colors.$red-50; + background-color: colors.$red-40; } &.added { - background-color: colors.$green-50; + background-color: colors.$green-30; } &.error { @@ -48,7 +48,7 @@ } &.changed { - background-color: colors.$blue-transparent; + background-color: colors.$blue-30; } &.disabled { diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index c441d785d63f2..ea9302561084f 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -2,6 +2,7 @@ import { faArrowRight, faMinus, faPlus } from "@fortawesome/free-solid-svg-icons import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import classnames from "classnames"; import classNames from "classnames"; +import { isEqual } from "lodash"; import React, { useMemo } from "react"; import { FormattedMessage } from "react-intl"; @@ -57,29 +58,26 @@ export const CatalogTreeTableRow: React.FC = ({ (item) => item.stream?.name === stream.stream?.name && item.stream?.namespace === stream.stream?.namespace )?.config?.selected !== stream.config?.selected; - const rowChanged = + const rowChanged = !isEqual( initialValues.syncCatalog.streams.find( - (item) => item.stream?.name === stream.stream?.name && item.stream?.namespace === stream.stream?.namespace - )?.config !== stream.config; + (item) => + item.stream && + stream.stream && + item.stream.name === stream.stream.name && + item.stream.namespace === stream.stream.namespace + )?.config, + stream.config + ); - // todo: the following should also evaluate if the stream is "new" from a refresh + is also enabled if (rowStatusChanged) { return isStreamEnabled ? "added" : "removed"; - } - // todo: the following should also evaluate if there are changes from the diff - else if (rowChanged) { + } else if (rowChanged) { return "changed"; } else if (!isStreamEnabled && !rowStatusChanged) { return "disabled"; } return "unchanged"; - }, [ - initialValues.syncCatalog.streams, - stream.config, - stream.stream?.name, - stream.stream?.namespace, - isStreamEnabled, - ]); + }, [initialValues.syncCatalog.streams, stream, isStreamEnabled]); const [isSelected, selectForBulkEdit] = useBulkEditSelect(stream.id); @@ -91,6 +89,7 @@ export const CatalogTreeTableRow: React.FC = ({ [styles.added]: statusToDisplay === "added", [styles.removed]: statusToDisplay === "removed", [styles.changed]: isSelected || statusToDisplay === "changed", + [styles.disabled]: statusToDisplay === "disabled", [styles.error]: hasError, }); @@ -110,10 +109,20 @@ export const CatalogTreeTableRow: React.FC = ({ return null; }, [statusToDisplay]); + const pillButtonVariant = useMemo(() => { + if (statusToDisplay === "added") { + return "green"; + } else if (statusToDisplay === "removed") { + return "red"; + } else if (statusToDisplay === "changed") { + return "blue"; + } + return "grey"; + }, [statusToDisplay]); + const checkboxCellCustomStyle = classnames(styles.checkboxCell, styles.streamRowCheckboxCell); - console.log({ statusToDisplay }); - console.log({ statusToDisplay }); + // todo: the primary key and cursor dropdowns should be full width of the cell return ( {!disabled && ( @@ -145,7 +154,12 @@ export const CatalogTreeTableRow: React.FC = ({ ) : ( // todo: SyncModeSelect should probably have a Tooltip, append/dedupe ends up ellipsing - + )}
@@ -155,6 +169,7 @@ export const CatalogTreeTableRow: React.FC = ({ paths={paths} path={cursorType === "sourceDefined" ? defaultCursorField : cursorField} onPathChange={onCursorChange} + variant={pillButtonVariant} /> )} @@ -166,6 +181,7 @@ export const CatalogTreeTableRow: React.FC = ({ path={primaryKey} isMulti onPathChange={onPrimaryKeyChange} + variant={pillButtonVariant} /> )} diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx index bb871e8f946aa..eb0eeb6264170 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx @@ -2,6 +2,7 @@ import React from "react"; import { FormattedMessage } from "react-intl"; import { PillButtonVariant, PillSelect } from "components/ui/PillSelect"; + import { Text } from "components/ui/Text"; import { Tooltip } from "components/ui/Tooltip"; @@ -69,6 +70,7 @@ export const StreamPathSelect: React.FC = (props) => { const finalValues = Array.isArray(options) ? options.map((op) => op.value) : options.value; props.onPathChange(finalValues); }} + variant={props.variant} /> ); }; diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/SyncModeSelect.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/SyncModeSelect.tsx index 37d0b9478fce0..8b0421542fa8b 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/SyncModeSelect.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/SyncModeSelect.tsx @@ -4,6 +4,7 @@ import { FormattedMessage } from "react-intl"; import { DropDownOptionDataItem } from "components/ui/DropDown"; import { PillSelect, PillButtonVariant } from "components/ui/PillSelect"; +import { PillButtonVariant } from "components/ui/PillSelect/PillButton"; import { DestinationSyncMode, SyncMode } from "core/request/AirbyteClient"; @@ -24,9 +25,17 @@ interface SyncModeSelectProps { options: SyncModeOption[]; value: Partial; variant?: PillButtonVariant; + variant?: PillButtonVariant; } -export const SyncModeSelect: React.FC = ({ className, variant, options, onChange, value }) => { +export const SyncModeSelect: React.FC = ({ + className, + variant, + options, + onChange, + value, + variant, +}) => { const pillSelectOptions = useMemo(() => { return options.map(({ value }) => { const { syncMode, destinationSyncMode } = value; @@ -50,6 +59,7 @@ export const SyncModeSelect: React.FC = ({ className, varia value={value} onChange={onChange} className={classNames(styles.pillSelect, className)} + variant={variant} /> ); }; diff --git a/airbyte-webapp/src/scss/_colors.scss b/airbyte-webapp/src/scss/_colors.scss index 6c849f2d0ad20..ea0e127f05533 100644 --- a/airbyte-webapp/src/scss/_colors.scss +++ b/airbyte-webapp/src/scss/_colors.scss @@ -52,6 +52,7 @@ $orange-800: #d83c24; $orange-900: #bf2f1b; $orange: $orange-400; +$green-30: #f4fcfd; $green-50: #dcf6f8; $green-100: #a7e9ec; $green-200: #67dae1; @@ -64,6 +65,7 @@ $green-800: #007c84; $green-900: #005959; $green: $green-200; +$red-40: #ffeff2; $red-50: #ffe4e8; $red-100: #ffbac6; $red-200: #ff8da1; From 958b6c0c3fb3c7488c24f114f2b070c125bbcf32 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Wed, 16 Nov 2022 14:42:42 -0500 Subject: [PATCH 04/25] colors for modificationIcon --- .../CatalogTree/next/CatalogTreeTableRow.module.scss | 4 ++++ .../CatalogTree/next/CatalogTreeTableRow.tsx | 2 +- .../src/components/icons/ModificationIcon.tsx | 11 ++++++++--- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss index 17ebb292c5721..d42665717e76c 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss @@ -82,3 +82,7 @@ width: variables.$width-wide-menu; min-width: variables.$width-wide-menu; } + +:export { + modificationIconColor: colors.$blue; +} diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index ea9302561084f..69f8bb5edd321 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -102,7 +102,7 @@ export const CatalogTreeTableRow: React.FC = ({ return ( // todo: styles need adjusting, especially color and possibly alignment - + ); } diff --git a/airbyte-webapp/src/components/icons/ModificationIcon.tsx b/airbyte-webapp/src/components/icons/ModificationIcon.tsx index 0804f327b6c33..efe57b2db19a2 100644 --- a/airbyte-webapp/src/components/icons/ModificationIcon.tsx +++ b/airbyte-webapp/src/components/icons/ModificationIcon.tsx @@ -1,17 +1,22 @@ -export const ModificationIcon: React.FC = () => { +import { theme } from "theme"; +interface ModificationIconProps { + color?: string; +} + +export const ModificationIcon: React.FC = ({ color = theme.blue100 }) => { return ( ); From 9b17e013bb69d518225bb6b19155acf07845c7b9 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Wed, 16 Nov 2022 14:52:28 -0500 Subject: [PATCH 05/25] icon alignment --- .../CatalogTree/next/CatalogTreeTableRow.module.scss | 6 ++++-- .../connection/CatalogTree/next/CatalogTreeTableRow.tsx | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss index d42665717e76c..f4df4391d738d 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss @@ -3,7 +3,6 @@ .icon { margin-right: 7px; - margin-top: -1px; &.plus { color: colors.$green; @@ -14,7 +13,9 @@ } &.changed { - color: colors.$blue-900; + display: flex; + flex-direction: row; + align-items: center; } } @@ -66,6 +67,7 @@ flex-direction: row; justify-content: flex-end; padding-left: variables.$spacing-xl + variables.$spacing-sm; + align-items: center; } .arrowCell { diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index 69f8bb5edd321..aaac8e18c70ee 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -101,9 +101,9 @@ export const CatalogTreeTableRow: React.FC = ({ } else if (statusToDisplay === "changed") { return ( // todo: styles need adjusting, especially color and possibly alignment - +
- +
); } return null; From 12f4fa3a264950f3fc4c7afcb2c459cd4ba83b11 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Wed, 16 Nov 2022 16:21:04 -0500 Subject: [PATCH 06/25] testing skeleton --- .../CatalogTree/next/CatalogTreeTableRow.test.tsx | 11 +++++++++++ .../CatalogTree/next/CatalogTreeTableRow.tsx | 2 ++ 2 files changed, 13 insertions(+) create mode 100644 airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.test.tsx diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.test.tsx new file mode 100644 index 0000000000000..611aa033a72e6 --- /dev/null +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.test.tsx @@ -0,0 +1,11 @@ +export {}; + +/** + * test cases: + * - should render with correct enabled-at-start styles + * - should render with correct disabled-at-start styles + * - should render with correct "added" styles + * - should render with correct "removed" styles + * - should render with correct "updated" styles after an update + * - updating an enabled row should still show added styles + */ diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index aaac8e18c70ee..660efd3eef6ba 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -52,6 +52,8 @@ export const CatalogTreeTableRow: React.FC = ({ const { initialValues } = useConnectionFormService(); + // todo: the effects of this can probably be written in a more simplified way! + const statusToDisplay = useMemo(() => { const rowStatusChanged = initialValues.syncCatalog.streams.find( From a05b4654f3685a1bd70ce5d80cd6914dbf9cf71c Mon Sep 17 00:00:00 2001 From: tealjulia Date: Thu, 17 Nov 2022 15:53:10 -0500 Subject: [PATCH 07/25] move calculation to a hook --- .../CatalogTree/next/CatalogTreeTableRow.tsx | 76 +-------------- .../CatalogTree/next/useRowStatus.tsx | 94 +++++++++++++++++++ 2 files changed, 98 insertions(+), 72 deletions(-) create mode 100644 airbyte-webapp/src/components/connection/CatalogTree/next/useRowStatus.tsx diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index 660efd3eef6ba..fd56b96d38572 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -1,24 +1,22 @@ -import { faArrowRight, faMinus, faPlus } from "@fortawesome/free-solid-svg-icons"; +import { faArrowRight } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import classnames from "classnames"; -import classNames from "classnames"; -import { isEqual } from "lodash"; import React, { useMemo } from "react"; import { FormattedMessage } from "react-intl"; -import { ModificationIcon } from "components/icons/ModificationIcon"; import { Cell, Row } from "components/SimpleTableComponents"; import { CheckBox } from "components/ui/CheckBox"; import { Switch } from "components/ui/Switch"; import { Text } from "components/ui/Text"; import { useBulkEditSelect } from "hooks/services/BulkEdit/BulkEditService"; -import { useConnectionFormService } from "hooks/services/ConnectionForm/ConnectionFormService"; import { StreamHeaderProps } from "../StreamHeader"; +// eslint-disable-next-line css-modules/no-unused-class import styles from "./CatalogTreeTableRow.module.scss"; import { StreamPathSelect } from "./StreamPathSelect"; import { SyncModeSelect } from "./SyncModeSelect"; +import { useRowStatus } from "./useRowStatus"; export const CatalogTreeTableRow: React.FC = ({ stream, @@ -35,11 +33,9 @@ export const CatalogTreeTableRow: React.FC = ({ // isRowExpanded, fields, onExpand, - hasError, disabled, }) => { const { primaryKey, cursorField, syncMode, destinationSyncMode } = stream.config ?? {}; - const isStreamEnabled = stream.config?.selected; const { defaultCursorField } = stream.stream ?? {}; const syncSchema = useMemo( @@ -50,77 +46,13 @@ export const CatalogTreeTableRow: React.FC = ({ [syncMode, destinationSyncMode] ); - const { initialValues } = useConnectionFormService(); - - // todo: the effects of this can probably be written in a more simplified way! - - const statusToDisplay = useMemo(() => { - const rowStatusChanged = - initialValues.syncCatalog.streams.find( - (item) => item.stream?.name === stream.stream?.name && item.stream?.namespace === stream.stream?.namespace - )?.config?.selected !== stream.config?.selected; - - const rowChanged = !isEqual( - initialValues.syncCatalog.streams.find( - (item) => - item.stream && - stream.stream && - item.stream.name === stream.stream.name && - item.stream.namespace === stream.stream.namespace - )?.config, - stream.config - ); - - if (rowStatusChanged) { - return isStreamEnabled ? "added" : "removed"; - } else if (rowChanged) { - return "changed"; - } else if (!isStreamEnabled && !rowStatusChanged) { - return "disabled"; - } - return "unchanged"; - }, [initialValues.syncCatalog.streams, stream, isStreamEnabled]); - const [isSelected, selectForBulkEdit] = useBulkEditSelect(stream.id); const paths = useMemo(() => primitiveFields.map((field) => field.path), [primitiveFields]); const fieldCount = fields?.length ?? 0; const onRowClick = fieldCount > 0 ? () => onExpand() : undefined; - const streamHeaderContentStyle = classnames(styles.streamHeaderContent, { - [styles.added]: statusToDisplay === "added", - [styles.removed]: statusToDisplay === "removed", - [styles.changed]: isSelected || statusToDisplay === "changed", - [styles.disabled]: statusToDisplay === "disabled", - [styles.error]: hasError, - }); - - const statusIcon = useMemo(() => { - if (statusToDisplay === "added") { - return ; - } else if (statusToDisplay === "removed") { - return ; - } else if (statusToDisplay === "changed") { - return ( - // todo: styles need adjusting, especially color and possibly alignment -
- -
- ); - } - return null; - }, [statusToDisplay]); - - const pillButtonVariant = useMemo(() => { - if (statusToDisplay === "added") { - return "green"; - } else if (statusToDisplay === "removed") { - return "red"; - } else if (statusToDisplay === "changed") { - return "blue"; - } - return "grey"; - }, [statusToDisplay]); + const { streamHeaderContentStyle, statusIcon, pillButtonVariant } = useRowStatus(stream); const checkboxCellCustomStyle = classnames(styles.checkboxCell, styles.streamRowCheckboxCell); diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useRowStatus.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useRowStatus.tsx new file mode 100644 index 0000000000000..141d982af31b6 --- /dev/null +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useRowStatus.tsx @@ -0,0 +1,94 @@ +/* eslint-disable css-modules/no-unused-class */ +import { faMinus, faPlus } from "@fortawesome/free-solid-svg-icons"; +import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; +import classNames from "classnames"; +import { useField } from "formik"; +import { isEqual } from "lodash"; +import { useMemo } from "react"; + +import { ModificationIcon } from "components/icons/ModificationIcon"; +import { PillButtonVariant } from "components/ui/PillSelect/PillButton"; + +import { SyncSchemaStream } from "core/domain/catalog"; +import { useBulkEditSelect } from "hooks/services/BulkEdit/BulkEditService"; +import { useConnectionFormService } from "hooks/services/ConnectionForm/ConnectionFormService"; + +import styles from "./CatalogTreeTableRow.module.scss"; + +export const useRowStatus = (stream: SyncSchemaStream) => { + const { initialValues } = useConnectionFormService(); + const [isSelected] = useBulkEditSelect(stream.id); + + const [, { error }] = useField(`schema.streams[${stream.id}].config`); + + // in case error is an empty string + const hasError = error !== undefined; + + const isStreamEnabled = stream.config?.selected; + const rowStatusChanged = + initialValues.syncCatalog.streams.find( + (item) => item.stream?.name === stream.stream?.name && item.stream?.namespace === stream.stream?.namespace + )?.config?.selected !== stream.config?.selected; + + const rowChanged = !isEqual( + initialValues.syncCatalog.streams.find( + (item) => + item.stream && + stream.stream && + item.stream.name === stream.stream.name && + item.stream.namespace === stream.stream.namespace + )?.config, + stream.config + ); + + const statusToDisplay = useMemo(() => { + if (rowStatusChanged) { + return isStreamEnabled ? "added" : "removed"; + } else if (rowChanged) { + return "changed"; + } else if (!isStreamEnabled && !rowStatusChanged) { + return "disabled"; + } + return "unchanged"; + }, [isStreamEnabled, rowChanged, rowStatusChanged]); + + const pillButtonVariant: PillButtonVariant = useMemo(() => { + if (statusToDisplay === "added") { + return "green"; + } else if (statusToDisplay === "removed") { + return "red"; + } else if (statusToDisplay === "changed") { + return "blue"; + } + return "grey"; + }, [statusToDisplay]); + + const statusIcon = useMemo(() => { + if (statusToDisplay === "added") { + return ; + } else if (statusToDisplay === "removed") { + return ; + } else if (statusToDisplay === "changed") { + return ( +
+ +
+ ); + } + return null; + }, [statusToDisplay]); + + const streamHeaderContentStyle = classNames(styles.streamHeaderContent, { + [styles.added]: statusToDisplay === "added", + [styles.removed]: statusToDisplay === "removed", + [styles.changed]: isSelected || statusToDisplay === "changed", + [styles.disabled]: statusToDisplay === "disabled", + [styles.error]: hasError, + }); + + return { + streamHeaderContentStyle, + statusIcon, + pillButtonVariant, + }; +}; From e2d76e7b5a819aea22721b7063a8712d4fd1a80e Mon Sep 17 00:00:00 2001 From: tealjulia Date: Mon, 21 Nov 2022 12:11:45 -0500 Subject: [PATCH 08/25] update hover styles on rows with changes --- .../next/CatalogTreeTableRow.module.scss | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss index f4df4391d738d..429226cce77a6 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss @@ -37,11 +37,21 @@ } &.removed { - background-color: colors.$red-40; + background-color: colors.$red-30; + + &:hover { + background: colors.$red-40; + cursor: pointer; + } } &.added { background-color: colors.$green-30; + + &:hover { + background: colors.$green-40; + cursor: pointer; + } } &.error { @@ -50,6 +60,11 @@ &.changed { background-color: colors.$blue-30; + + &:hover { + background: colors.$blue-40; + cursor: pointer; + } } &.disabled { From 6f3ce6ec1b92c227c36cee39727ef2fdaa490c8e Mon Sep 17 00:00:00 2001 From: tealjulia Date: Mon, 21 Nov 2022 17:24:38 -0500 Subject: [PATCH 09/25] add testing Co-authored-by: Krishna (kc) Glick --- .../next/CatalogTreeTableRow.test.tsx | 11 - .../CatalogTree/next/CatalogTreeTableRow.tsx | 4 +- .../next/useCatalogTreeRowProps.test.tsx | 242 ++++++++++++++++++ ...wStatus.tsx => useCatalogTreeRowProps.tsx} | 41 +-- 4 files changed, 265 insertions(+), 33 deletions(-) delete mode 100644 airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.test.tsx create mode 100644 airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.test.tsx rename airbyte-webapp/src/components/connection/CatalogTree/next/{useRowStatus.tsx => useCatalogTreeRowProps.tsx} (74%) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.test.tsx deleted file mode 100644 index 611aa033a72e6..0000000000000 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.test.tsx +++ /dev/null @@ -1,11 +0,0 @@ -export {}; - -/** - * test cases: - * - should render with correct enabled-at-start styles - * - should render with correct disabled-at-start styles - * - should render with correct "added" styles - * - should render with correct "removed" styles - * - should render with correct "updated" styles after an update - * - updating an enabled row should still show added styles - */ diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index fd56b96d38572..cb7f004282e36 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -16,7 +16,7 @@ import { StreamHeaderProps } from "../StreamHeader"; import styles from "./CatalogTreeTableRow.module.scss"; import { StreamPathSelect } from "./StreamPathSelect"; import { SyncModeSelect } from "./SyncModeSelect"; -import { useRowStatus } from "./useRowStatus"; +import { useCatalogTreeRowProps } from "./useCatalogTreeRowProps"; export const CatalogTreeTableRow: React.FC = ({ stream, @@ -52,7 +52,7 @@ export const CatalogTreeTableRow: React.FC = ({ const fieldCount = fields?.length ?? 0; const onRowClick = fieldCount > 0 ? () => onExpand() : undefined; - const { streamHeaderContentStyle, statusIcon, pillButtonVariant } = useRowStatus(stream); + const { streamHeaderContentStyle, statusIcon, pillButtonVariant } = useCatalogTreeRowProps(stream); const checkboxCellCustomStyle = classnames(styles.checkboxCell, styles.streamRowCheckboxCell); diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.test.tsx new file mode 100644 index 0000000000000..b492d5b1db7e8 --- /dev/null +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.test.tsx @@ -0,0 +1,242 @@ +import { renderHook } from "@testing-library/react-hooks"; +import classNames from "classnames"; +import * as formik from "formik"; + +import { AirbyteStreamAndConfiguration } from "core/request/AirbyteClient"; +import * as bulkEditService from "hooks/services/BulkEdit/BulkEditService"; +import * as connectionFormService from "hooks/services/ConnectionForm/ConnectionFormService"; +import { FormikConnectionFormValues } from "views/Connection/ConnectionForm/formConfig"; + +// eslint-disable-next-line css-modules/no-unused-class +import styles from "./CatalogTreeTableRow.module.scss"; +import { useCatalogTreeRowProps } from "./useCatalogTreeRowProps"; + +const mockStream: Partial = { + stream: { + name: "stream_name", + namespace: "stream_namespace", + }, + config: { selected: true, syncMode: "full_refresh", destinationSyncMode: "overwrite" }, +}; + +const mockInitialValues: Partial = { + syncCatalog: { + streams: [ + { + stream: { + name: "stream_name", + namespace: "stream_namespace", + }, + config: { selected: true, syncMode: "full_refresh", destinationSyncMode: "overwrite" }, + }, + ], + }, +}; + +describe("", () => { + it("should return default styles for a row that starts enabled", () => { + // eslint-disable-next-line + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + // eslint-disable-next-line + return { initialValues: mockInitialValues } as any; + }); + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: undefined }] as any; // no error + }); + + const { result } = renderHook(() => useCatalogTreeRowProps(mockStream)); + + expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent)); + expect(result.current.statusIcon).toEqual(null); + expect(result.current.pillButtonVariant).toEqual("grey"); + }); + it("should return disabled styles for a row that starts disabled", () => { + // eslint-disable-next-line + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + return { + initialValues: { + syncCatalog: { + streams: [ + { + ...mockInitialValues.syncCatalog?.streams[0], + config: { ...mockInitialValues.syncCatalog?.streams[0].config, selected: false }, + }, + ], + }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + }); + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: undefined }] as any; // no error + }); + const { result } = renderHook(() => + useCatalogTreeRowProps({ + ...mockStream, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + config: { ...mockStream.config!, selected: false }, + }) + ); + + expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.disabled)); + expect(result.current.statusIcon).toEqual(null); + expect(result.current.pillButtonVariant).toEqual("grey"); + }); + it("should return added styles for a row that is added", () => { + // eslint-disable-next-line + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + return { + initialValues: { + syncCatalog: { + streams: [ + { + ...mockInitialValues.syncCatalog?.streams[0], + config: { ...mockInitialValues.syncCatalog?.streams[0].config, selected: false }, + }, + ], + }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + }); + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: undefined }] as any; // no error + }); + const { result } = renderHook(() => + useCatalogTreeRowProps({ + ...mockStream, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + config: { ...mockStream.config!, selected: true }, // selected true + }) + ); + + expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.added)); + // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + expect(result.current.pillButtonVariant).toEqual("green"); + }); + it("should return removed styles for a row that is removed", () => { + // eslint-disable-next-line + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + return { + initialValues: { ...mockInitialValues }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + }); + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: undefined }] as any; // no error + }); + const { result } = renderHook(() => + useCatalogTreeRowProps({ + ...mockStream, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + config: { ...mockStream.config!, selected: false }, // selected false + }) + ); + + expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.removed)); + // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + expect(result.current.pillButtonVariant).toEqual("red"); + }); + it("should return updated styles for a row that is updated", () => { + // eslint-disable-next-line + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit + // todo: i'm only changing config.selected... this can be cleaned up with a spread operator and the mockInitialValues + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + return { + initialValues: { ...mockInitialValues }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + }); + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: undefined }] as any; // no error + }); + const { result } = renderHook(() => + useCatalogTreeRowProps({ + ...mockStream, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + config: { ...mockStream.config!, syncMode: "incremental", destinationSyncMode: "append_dedup" }, // new sync mode and destination sync mode + }) + ); + + expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); + // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + expect(result.current.pillButtonVariant).toEqual("blue"); + }); + it("should return added styles for a row that is both added and updated", () => { + // eslint-disable-next-line + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit + // todo: i'm only changing config.selected... this can be cleaned up with a spread operator and the mockInitialValues + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + return { + initialValues: { + syncCatalog: { + streams: [ + { + ...mockInitialValues.syncCatalog?.streams[0], + config: { ...mockInitialValues.syncCatalog?.streams[0].config, selected: false }, + }, + ], + }, + }, // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + }); + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: undefined }] as any; // no error + }); + const { result } = renderHook(() => + useCatalogTreeRowProps({ + ...mockStream, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + config: { selected: true, syncMode: "incremental", destinationSyncMode: "append_dedup" }, // selected true, new sync, mode and destination sync mode + }) + ); + + expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.added)); + // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + expect(result.current.pillButtonVariant).toEqual("green"); + }); + it("should return change background color with relevant icon if selected for bulk edit", () => { + // eslint-disable-next-line + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [true, () => null] as any); // not selected for bulk edit + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + // eslint-disable-next-line + return { initialValues: mockInitialValues } as any; + }); + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: undefined }] as any; // no error + }); + + const { result } = renderHook(() => useCatalogTreeRowProps(mockStream)); + + expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); + // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + expect(result.current.pillButtonVariant).toEqual("blue"); + }); + it("should return error styles for a row that has an error", () => { + // eslint-disable-next-line + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + // eslint-disable-next-line + return { initialValues: mockInitialValues } as any; + }); + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: true }] as any; // no error + }); + + const { result } = renderHook(() => useCatalogTreeRowProps(mockStream)); + + expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.error)); + }); +}); diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useRowStatus.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.tsx similarity index 74% rename from airbyte-webapp/src/components/connection/CatalogTree/next/useRowStatus.tsx rename to airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.tsx index 141d982af31b6..60411ef974bea 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useRowStatus.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.tsx @@ -15,7 +15,7 @@ import { useConnectionFormService } from "hooks/services/ConnectionForm/Connecti import styles from "./CatalogTreeTableRow.module.scss"; -export const useRowStatus = (stream: SyncSchemaStream) => { +export const useCatalogTreeRowProps = (stream: SyncSchemaStream) => { const { initialValues } = useConnectionFormService(); const [isSelected] = useBulkEditSelect(stream.id); @@ -25,23 +25,24 @@ export const useRowStatus = (stream: SyncSchemaStream) => { const hasError = error !== undefined; const isStreamEnabled = stream.config?.selected; - const rowStatusChanged = - initialValues.syncCatalog.streams.find( - (item) => item.stream?.name === stream.stream?.name && item.stream?.namespace === stream.stream?.namespace - )?.config?.selected !== stream.config?.selected; - - const rowChanged = !isEqual( - initialValues.syncCatalog.streams.find( - (item) => - item.stream && - stream.stream && - item.stream.name === stream.stream.name && - item.stream.namespace === stream.stream.namespace - )?.config, - stream.config - ); const statusToDisplay = useMemo(() => { + const rowStatusChanged = + initialValues.syncCatalog.streams.find( + (item) => item.stream?.name === stream.stream?.name && item.stream?.namespace === stream.stream?.namespace + )?.config?.selected !== stream.config?.selected; + + const rowChanged = !isEqual( + initialValues.syncCatalog.streams.find( + (item) => + item.stream && + stream.stream && + item.stream.name === stream.stream.name && + item.stream.namespace === stream.stream.namespace + )?.config, + stream.config + ); + if (rowStatusChanged) { return isStreamEnabled ? "added" : "removed"; } else if (rowChanged) { @@ -50,18 +51,18 @@ export const useRowStatus = (stream: SyncSchemaStream) => { return "disabled"; } return "unchanged"; - }, [isStreamEnabled, rowChanged, rowStatusChanged]); + }, [initialValues.syncCatalog.streams, isStreamEnabled, stream.config, stream.stream]); const pillButtonVariant: PillButtonVariant = useMemo(() => { if (statusToDisplay === "added") { return "green"; } else if (statusToDisplay === "removed") { return "red"; - } else if (statusToDisplay === "changed") { + } else if (statusToDisplay === "changed" || isSelected) { return "blue"; } return "grey"; - }, [statusToDisplay]); + }, [isSelected, statusToDisplay]); const statusIcon = useMemo(() => { if (statusToDisplay === "added") { @@ -81,7 +82,7 @@ export const useRowStatus = (stream: SyncSchemaStream) => { const streamHeaderContentStyle = classNames(styles.streamHeaderContent, { [styles.added]: statusToDisplay === "added", [styles.removed]: statusToDisplay === "removed", - [styles.changed]: isSelected || statusToDisplay === "changed", + [styles.changed]: statusToDisplay === "changed" || isSelected, [styles.disabled]: statusToDisplay === "disabled", [styles.error]: hasError, }); From 425845f3a985cb5a629d625993338020fd43426b Mon Sep 17 00:00:00 2001 From: tealjulia Date: Mon, 21 Nov 2022 17:30:22 -0500 Subject: [PATCH 10/25] clarify naming --- .../CatalogTree/next/CatalogTreeTableRow.tsx | 4 ++-- ...sx => useCatalogTreeTableRowProps.test.tsx} | 18 +++++++++--------- ...ops.tsx => useCatalogTreeTableRowProps.tsx} | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) rename airbyte-webapp/src/components/connection/CatalogTree/next/{useCatalogTreeRowProps.test.tsx => useCatalogTreeTableRowProps.test.tsx} (95%) rename airbyte-webapp/src/components/connection/CatalogTree/next/{useCatalogTreeRowProps.tsx => useCatalogTreeTableRowProps.tsx} (97%) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index cb7f004282e36..aa0017b134813 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -16,7 +16,7 @@ import { StreamHeaderProps } from "../StreamHeader"; import styles from "./CatalogTreeTableRow.module.scss"; import { StreamPathSelect } from "./StreamPathSelect"; import { SyncModeSelect } from "./SyncModeSelect"; -import { useCatalogTreeRowProps } from "./useCatalogTreeRowProps"; +import { useCatalogTreeTableRowProps } from "./useCatalogTreeTableRowProps"; export const CatalogTreeTableRow: React.FC = ({ stream, @@ -52,7 +52,7 @@ export const CatalogTreeTableRow: React.FC = ({ const fieldCount = fields?.length ?? 0; const onRowClick = fieldCount > 0 ? () => onExpand() : undefined; - const { streamHeaderContentStyle, statusIcon, pillButtonVariant } = useCatalogTreeRowProps(stream); + const { streamHeaderContentStyle, statusIcon, pillButtonVariant } = useCatalogTreeTableRowProps(stream); const checkboxCellCustomStyle = classnames(styles.checkboxCell, styles.streamRowCheckboxCell); diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx similarity index 95% rename from airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.test.tsx rename to airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx index b492d5b1db7e8..9a0cc74c35967 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.test.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx @@ -9,7 +9,7 @@ import { FormikConnectionFormValues } from "views/Connection/ConnectionForm/form // eslint-disable-next-line css-modules/no-unused-class import styles from "./CatalogTreeTableRow.module.scss"; -import { useCatalogTreeRowProps } from "./useCatalogTreeRowProps"; +import { useCatalogTreeTableRowProps } from "./useCatalogTreeTableRowProps"; const mockStream: Partial = { stream: { @@ -46,7 +46,7 @@ describe("", () => { return [{}, { error: undefined }] as any; // no error }); - const { result } = renderHook(() => useCatalogTreeRowProps(mockStream)); + const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent)); expect(result.current.statusIcon).toEqual(null); @@ -75,7 +75,7 @@ describe("", () => { return [{}, { error: undefined }] as any; // no error }); const { result } = renderHook(() => - useCatalogTreeRowProps({ + useCatalogTreeTableRowProps({ ...mockStream, // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, selected: false }, @@ -109,7 +109,7 @@ describe("", () => { return [{}, { error: undefined }] as any; // no error }); const { result } = renderHook(() => - useCatalogTreeRowProps({ + useCatalogTreeTableRowProps({ ...mockStream, // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, selected: true }, // selected true @@ -134,7 +134,7 @@ describe("", () => { return [{}, { error: undefined }] as any; // no error }); const { result } = renderHook(() => - useCatalogTreeRowProps({ + useCatalogTreeTableRowProps({ ...mockStream, // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, selected: false }, // selected false @@ -160,7 +160,7 @@ describe("", () => { return [{}, { error: undefined }] as any; // no error }); const { result } = renderHook(() => - useCatalogTreeRowProps({ + useCatalogTreeTableRowProps({ ...mockStream, // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, syncMode: "incremental", destinationSyncMode: "append_dedup" }, // new sync mode and destination sync mode @@ -194,7 +194,7 @@ describe("", () => { return [{}, { error: undefined }] as any; // no error }); const { result } = renderHook(() => - useCatalogTreeRowProps({ + useCatalogTreeTableRowProps({ ...mockStream, // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { selected: true, syncMode: "incremental", destinationSyncMode: "append_dedup" }, // selected true, new sync, mode and destination sync mode @@ -217,7 +217,7 @@ describe("", () => { return [{}, { error: undefined }] as any; // no error }); - const { result } = renderHook(() => useCatalogTreeRowProps(mockStream)); + const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes @@ -235,7 +235,7 @@ describe("", () => { return [{}, { error: true }] as any; // no error }); - const { result } = renderHook(() => useCatalogTreeRowProps(mockStream)); + const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.error)); }); diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx similarity index 97% rename from airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.tsx rename to airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx index 60411ef974bea..cb4998f36f926 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeRowProps.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx @@ -15,7 +15,7 @@ import { useConnectionFormService } from "hooks/services/ConnectionForm/Connecti import styles from "./CatalogTreeTableRow.module.scss"; -export const useCatalogTreeRowProps = (stream: SyncSchemaStream) => { +export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { const { initialValues } = useConnectionFormService(); const [isSelected] = useBulkEditSelect(stream.id); From f9678316d4fd16c1373f72640f0bb532710a59e9 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Tue, 22 Nov 2022 10:48:58 -0500 Subject: [PATCH 11/25] add mixin, fix selected added/removed rows --- .../next/CatalogTreeTableRow.module.scss | 43 ++++++++----------- .../next/useCatalogTreeTableRowProps.tsx | 22 +++++++--- airbyte-webapp/src/scss/_colors.scss | 2 + 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss index 429226cce77a6..4768a22091946 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss @@ -1,6 +1,15 @@ @use "scss/colors"; @use "scss/variables"; +@mixin header-background($color, $hoverColor) { + background-color: $color; + + &:hover { + background-color: $hoverColor; + cursor: pointer; + } +} + .icon { margin-right: 7px; @@ -13,6 +22,7 @@ } &.changed { + color: colors.$blue; display: flex; flex-direction: row; align-items: center; @@ -20,7 +30,8 @@ } .streamHeaderContent { - background: colors.$white; + @include header-background(colors.$white, colors.$grey-30); + border-bottom: 1px solid colors.$grey-50; padding: 0 variables.$spacing-md; margin-bottom: 1px; @@ -31,42 +42,22 @@ overflow-y: auto; scrollbar-gutter: stable; - &:hover { - background: colors.$grey-30; - cursor: pointer; - } - &.removed { - background-color: colors.$red-30; - - &:hover { - background: colors.$red-40; - cursor: pointer; - } + @include header-background(colors.$red-30, colors.$red-40); } &.added { - background-color: colors.$green-30; + @include header-background(colors.$green-30, colors.$green-40); + } - &:hover { - background: colors.$green-40; - cursor: pointer; - } + &.changed { + @include header-background(colors.$blue-30, colors.$blue-40); } &.error { border: 1px solid colors.$red; } - &.changed { - background-color: colors.$blue-30; - - &:hover { - background: colors.$blue-40; - cursor: pointer; - } - } - &.disabled { background-color: colors.$grey-50; } diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx index cb4998f36f926..0e8dc54b22d2a 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx @@ -54,9 +54,9 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { }, [initialValues.syncCatalog.streams, isStreamEnabled, stream.config, stream.stream]); const pillButtonVariant: PillButtonVariant = useMemo(() => { - if (statusToDisplay === "added") { + if (statusToDisplay === "added" && !isSelected) { return "green"; - } else if (statusToDisplay === "removed") { + } else if (statusToDisplay === "removed" && !isSelected) { return "red"; } else if (statusToDisplay === "changed" || isSelected) { return "blue"; @@ -66,9 +66,21 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { const statusIcon = useMemo(() => { if (statusToDisplay === "added") { - return ; + return ( + + ); } else if (statusToDisplay === "removed") { - return ; + return ( + + ); } else if (statusToDisplay === "changed") { return (
@@ -77,7 +89,7 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { ); } return null; - }, [statusToDisplay]); + }, [isSelected, statusToDisplay]); const streamHeaderContentStyle = classNames(styles.streamHeaderContent, { [styles.added]: statusToDisplay === "added", diff --git a/airbyte-webapp/src/scss/_colors.scss b/airbyte-webapp/src/scss/_colors.scss index ea0e127f05533..282110f01ebb6 100644 --- a/airbyte-webapp/src/scss/_colors.scss +++ b/airbyte-webapp/src/scss/_colors.scss @@ -53,6 +53,7 @@ $orange-900: #bf2f1b; $orange: $orange-400; $green-30: #f4fcfd; +$green-40: #f0fcfd; $green-50: #dcf6f8; $green-100: #a7e9ec; $green-200: #67dae1; @@ -65,6 +66,7 @@ $green-800: #007c84; $green-900: #005959; $green: $green-200; +$red-30: #fff4f6; $red-40: #ffeff2; $red-50: #ffe4e8; $red-100: #ffbac6; From aeabaf8d82d52cf3f35ada8b89ca6c326bc8a381 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Tue, 22 Nov 2022 12:55:25 -0500 Subject: [PATCH 12/25] WIP test icons using snapshots --- .../useCatalogTreeTableRowProps.test.tsx.snap | 151 ++++++++++++++++++ .../next/useCatalogTreeTableRowProps.test.tsx | 59 +++++-- 2 files changed, 198 insertions(+), 12 deletions(-) create mode 100644 airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap b/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap new file mode 100644 index 0000000000000..0ea4665c17da5 --- /dev/null +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap @@ -0,0 +1,151 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` should return added styles for a row that is added 1`] = ` + +`; + +exports[` should return added styles for a row that is both added and updated 1`] = ` + +`; + +exports[` should return change background color and override icon color with relevant icon if selected for bulk edit 1`] = `null`; + +exports[` should return change background color with relevant icon if selected for bulk edit 1`] = `null`; + +exports[` should return removed styles for a row that is removed 1`] = ` + +`; + +exports[` should return updated styles for a row that is updated 1`] = ` +
+ +
+`; diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx index 9a0cc74c35967..f90eb85f440be 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ import { renderHook } from "@testing-library/react-hooks"; import classNames from "classnames"; import * as formik from "formik"; @@ -77,7 +78,6 @@ describe("", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, selected: false }, }) ); @@ -111,13 +111,13 @@ describe("", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, selected: true }, // selected true }) ); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.added)); - // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + // we care that iconName="plus" and className="icon plus" but it is tricky to test a react node returned from a hook like this + expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("green"); }); it("should return removed styles for a row that is removed", () => { @@ -136,19 +136,19 @@ describe("", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, selected: false }, // selected false }) ); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.removed)); - // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + + // we care that iconName="minus" and className="icon minus" but it is tricky to test a react node returned from a hook like this + expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("red"); }); it("should return updated styles for a row that is updated", () => { // eslint-disable-next-line jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit - // todo: i'm only changing config.selected... this can be cleaned up with a spread operator and the mockInitialValues jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { return { initialValues: { ...mockInitialValues }, @@ -162,19 +162,18 @@ describe("", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, syncMode: "incremental", destinationSyncMode: "append_dedup" }, // new sync mode and destination sync mode }) ); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); - // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + // we care that this is our custom ModificationIcon component with modificationColor as its color + expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("blue"); }); it("should return added styles for a row that is both added and updated", () => { // eslint-disable-next-line jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit - // todo: i'm only changing config.selected... this can be cleaned up with a spread operator and the mockInitialValues jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { return { initialValues: { @@ -196,16 +195,51 @@ describe("", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { selected: true, syncMode: "incremental", destinationSyncMode: "append_dedup" }, // selected true, new sync, mode and destination sync mode }) ); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.added)); - // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + // we care that iconName="plus" and className="icon plus" but it is tricky to test a react node returned from a hook like this + expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("green"); }); it("should return change background color with relevant icon if selected for bulk edit", () => { + // eslint-disable-next-line + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [true, () => null] as any); // not selected for bulk edit + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + return { + initialValues: { + syncCatalog: { + streams: [ + { + ...mockInitialValues.syncCatalog?.streams[0], + config: { ...mockInitialValues.syncCatalog?.streams[0].config, selected: false }, + }, + ], + }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; + }); + + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: undefined }] as any; // no error + }); + + const { result } = renderHook(() => + useCatalogTreeTableRowProps({ + ...mockStream, + config: { ...mockStream.config!, selected: true }, // selected true, new sync, mode and destination sync mode + }) + ); + expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); + // we care that iconName="plus" and className="icon plus" but it is tricky to test a react node returned from a hook like this + expect(result.current.statusIcon).toMatchSnapshot(); + expect(result.current.pillButtonVariant).toEqual("blue"); + }); + it("should return change background color and override icon color with relevant icon if selected for bulk edit", () => { // eslint-disable-next-line jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [true, () => null] as any); // not selected for bulk edit jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { @@ -220,7 +254,8 @@ describe("", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); - // expect(result.current.statusIcon).(); TODO: jest is weird at comparing react nodes + // we care that iconName="plus" and className="icon plus" but it is tricky to test a react node returned from a hook like this + expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("blue"); }); it("should return error styles for a row that has an error", () => { From e8049d0bc9c92ab2e37ea27cf90e14af1c847061 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Tue, 22 Nov 2022 13:32:35 -0500 Subject: [PATCH 13/25] fix snapshots, tests for catalog tree table row hook --- .../useCatalogTreeTableRowProps.test.tsx.snap | 47 +++++++++++++++++-- .../next/useCatalogTreeTableRowProps.test.tsx | 19 -------- .../next/useCatalogTreeTableRowProps.tsx | 4 +- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap b/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap index 0ea4665c17da5..b09cf64b064c4 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap @@ -90,9 +90,50 @@ exports[` should return added styles for a row that is bo /> `; -exports[` should return change background color and override icon color with relevant icon if selected for bulk edit 1`] = `null`; - -exports[` should return change background color with relevant icon if selected for bulk edit 1`] = `null`; +exports[` should return change background color with relevant icon if selected for bulk edit 1`] = ` + +`; exports[` should return removed styles for a row that is removed 1`] = ` ", () => { expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("blue"); }); - it("should return change background color and override icon color with relevant icon if selected for bulk edit", () => { - // eslint-disable-next-line - jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [true, () => null] as any); // not selected for bulk edit - jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - // eslint-disable-next-line - return { initialValues: mockInitialValues } as any; - }); - jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line - return [{}, { error: undefined }] as any; // no error - }); - - const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); - - expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); - // we care that iconName="plus" and className="icon plus" but it is tricky to test a react node returned from a hook like this - expect(result.current.statusIcon).toMatchSnapshot(); - expect(result.current.pillButtonVariant).toEqual("blue"); - }); it("should return error styles for a row that has an error", () => { // eslint-disable-next-line jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx index 0e8dc54b22d2a..c98000df0160e 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx @@ -92,8 +92,8 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { }, [isSelected, statusToDisplay]); const streamHeaderContentStyle = classNames(styles.streamHeaderContent, { - [styles.added]: statusToDisplay === "added", - [styles.removed]: statusToDisplay === "removed", + [styles.added]: statusToDisplay === "added" && !isSelected, + [styles.removed]: statusToDisplay === "removed" && !isSelected, [styles.changed]: statusToDisplay === "changed" || isSelected, [styles.disabled]: statusToDisplay === "disabled", [styles.error]: hasError, From 31c481f84138a318c47c66434ac7dbfe8832ce0c Mon Sep 17 00:00:00 2001 From: tealjulia Date: Tue, 22 Nov 2022 15:30:59 -0500 Subject: [PATCH 14/25] fix order of logic for disabled rows (disabled trumps everything) --- .../CatalogTree/next/useCatalogTreeTableRowProps.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx index c98000df0160e..2b4e1869a68f4 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx @@ -43,12 +43,12 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { stream.config ); - if (rowStatusChanged) { + if (!isStreamEnabled && !rowStatusChanged) { + return "disabled"; + } else if (rowStatusChanged) { return isStreamEnabled ? "added" : "removed"; } else if (rowChanged) { return "changed"; - } else if (!isStreamEnabled && !rowStatusChanged) { - return "disabled"; } return "unchanged"; }, [initialValues.syncCatalog.streams, isStreamEnabled, stream.config, stream.stream]); From 9d8d520597cb906e27228d96dcbfa9654a033f73 Mon Sep 17 00:00:00 2001 From: Teal Larson Date: Tue, 22 Nov 2022 16:02:51 -0500 Subject: [PATCH 15/25] Update airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx Co-authored-by: Edmundo Ruiz Ghanem <168664+edmundito@users.noreply.github.com> --- .../CatalogTree/next/useCatalogTreeTableRowProps.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx index c787ebcd55253..481781bf573fc 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx @@ -34,7 +34,7 @@ const mockInitialValues: Partial = { }, }; -describe("", () => { +describe("useCatalogTreeTableRowProps", () => { it("should return default styles for a row that starts enabled", () => { // eslint-disable-next-line jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit From 0a3656b124416961c0f58384473f0bb87285bb5c Mon Sep 17 00:00:00 2001 From: tealjulia Date: Wed, 23 Nov 2022 17:13:29 -0500 Subject: [PATCH 16/25] move icon to its own component, clean up tests --- .../next/CatalogTreeTableRow.module.scss | 23 --- .../CatalogTree/next/CatalogTreeTableRow.tsx | 5 +- .../next/CatalogTreeTableRowIcon.module.scss | 24 +++ .../next/CatalogTreeTableRowIcon.tsx | 42 ++++ .../useCatalogTreeTableRowProps.test.tsx.snap | 192 ------------------ .../next/useCatalogTreeTableRowProps.test.tsx | 191 ++++------------- .../next/useCatalogTreeTableRowProps.tsx | 33 +-- 7 files changed, 113 insertions(+), 397 deletions(-) create mode 100644 airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRowIcon.module.scss create mode 100644 airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRowIcon.tsx delete mode 100644 airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss index 4768a22091946..a7eacc8133428 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss @@ -10,25 +10,6 @@ } } -.icon { - margin-right: 7px; - - &.plus { - color: colors.$green; - } - - &.minus { - color: colors.$red; - } - - &.changed { - color: colors.$blue; - display: flex; - flex-direction: row; - align-items: center; - } -} - .streamHeaderContent { @include header-background(colors.$white, colors.$grey-30); @@ -90,7 +71,3 @@ width: variables.$width-wide-menu; min-width: variables.$width-wide-menu; } - -:export { - modificationIconColor: colors.$blue; -} diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index aa0017b134813..4633d9a7e0258 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -14,6 +14,7 @@ import { useBulkEditSelect } from "hooks/services/BulkEdit/BulkEditService"; import { StreamHeaderProps } from "../StreamHeader"; // eslint-disable-next-line css-modules/no-unused-class import styles from "./CatalogTreeTableRow.module.scss"; +import { CatalogTreeTableRowIcon } from "./CatalogTreeTableRowIcon"; import { StreamPathSelect } from "./StreamPathSelect"; import { SyncModeSelect } from "./SyncModeSelect"; import { useCatalogTreeTableRowProps } from "./useCatalogTreeTableRowProps"; @@ -52,7 +53,7 @@ export const CatalogTreeTableRow: React.FC = ({ const fieldCount = fields?.length ?? 0; const onRowClick = fieldCount > 0 ? () => onExpand() : undefined; - const { streamHeaderContentStyle, statusIcon, pillButtonVariant } = useCatalogTreeTableRowProps(stream); + const { streamHeaderContentStyle, pillButtonVariant } = useCatalogTreeTableRowProps(stream); const checkboxCellCustomStyle = classnames(styles.checkboxCell, styles.streamRowCheckboxCell); @@ -61,7 +62,7 @@ export const CatalogTreeTableRow: React.FC = ({ {!disabled && (
- {statusIcon} +
)} diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRowIcon.module.scss b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRowIcon.module.scss new file mode 100644 index 0000000000000..f14a40a0d502d --- /dev/null +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRowIcon.module.scss @@ -0,0 +1,24 @@ +@use "scss/colors"; + +.icon { + margin-right: 7px; + + &.plus { + color: colors.$green; + } + + &.minus { + color: colors.$red; + } + + &.changed { + color: colors.$blue; + display: flex; + flex-direction: row; + align-items: center; + } +} + +:export { + modificationIconColor: colors.$blue; +} diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRowIcon.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRowIcon.tsx new file mode 100644 index 0000000000000..bf74cc1a35dc8 --- /dev/null +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRowIcon.tsx @@ -0,0 +1,42 @@ +import { faMinus, faPlus } from "@fortawesome/free-solid-svg-icons"; +import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; +import classNames from "classnames"; + +import { ModificationIcon } from "components/icons/ModificationIcon"; + +import { SyncSchemaStream } from "core/domain/catalog"; + +import styles from "./CatalogTreeTableRowIcon.module.scss"; +import { useCatalogTreeTableRowProps } from "./useCatalogTreeTableRowProps"; + +interface CatalogTreeTableRowIconProps { + stream: SyncSchemaStream; +} +export const CatalogTreeTableRowIcon: React.FC = ({ stream }) => { + const { statusToDisplay, isSelected } = useCatalogTreeTableRowProps(stream); + + if (statusToDisplay === "added") { + return ( + + ); + } else if (statusToDisplay === "removed") { + return ( + + ); + } else if (statusToDisplay === "changed") { + return ( +
+ +
+ ); + } + return
; +}; diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap b/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap deleted file mode 100644 index b09cf64b064c4..0000000000000 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/__snapshots__/useCatalogTreeTableRowProps.test.tsx.snap +++ /dev/null @@ -1,192 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[` should return added styles for a row that is added 1`] = ` - -`; - -exports[` should return added styles for a row that is both added and updated 1`] = ` - -`; - -exports[` should return change background color with relevant icon if selected for bulk edit 1`] = ` - -`; - -exports[` should return removed styles for a row that is removed 1`] = ` - -`; - -exports[` should return updated styles for a row that is updated 1`] = ` -
- -
-`; diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx index 481781bf573fc..d6534c2823b4e 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx @@ -34,47 +34,42 @@ const mockInitialValues: Partial = { }, }; +const mockDisabledInitialValues: Partial = { + syncCatalog: { + streams: [ + { + ...mockInitialValues.syncCatalog?.streams[0], + config: { ...mockInitialValues.syncCatalog!.streams[0].config!, selected: false }, + }, + ], + }, +}; + +const testSetup = (initialValues: Partial, isBulkEdit: boolean, error: unknown) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [isBulkEdit, () => null] as any); // not selected for bulk edit + jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { + // eslint-disable-next-line + return { initialValues: initialValues } as any; + }); + jest.spyOn(formik, "useField").mockImplementationOnce(() => { + // eslint-disable-next-line + return [{}, { error: error }] as any; // no error + }); +}; + describe("useCatalogTreeTableRowProps", () => { it("should return default styles for a row that starts enabled", () => { - // eslint-disable-next-line - jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit - jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - // eslint-disable-next-line - return { initialValues: mockInitialValues } as any; - }); - jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line - return [{}, { error: undefined }] as any; // no error - }); + testSetup(mockInitialValues, false, undefined); const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent)); - expect(result.current.statusIcon).toEqual(null); expect(result.current.pillButtonVariant).toEqual("grey"); }); it("should return disabled styles for a row that starts disabled", () => { - // eslint-disable-next-line - jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit - jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - return { - initialValues: { - syncCatalog: { - streams: [ - { - ...mockInitialValues.syncCatalog?.streams[0], - config: { ...mockInitialValues.syncCatalog?.streams[0].config, selected: false }, - }, - ], - }, - }, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; - }); - jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line - return [{}, { error: undefined }] as any; // no error - }); + testSetup(mockDisabledInitialValues, false, undefined); + const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, @@ -83,115 +78,47 @@ describe("useCatalogTreeTableRowProps", () => { ); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.disabled)); - expect(result.current.statusIcon).toEqual(null); expect(result.current.pillButtonVariant).toEqual("grey"); }); it("should return added styles for a row that is added", () => { - // eslint-disable-next-line - jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit - jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - return { - initialValues: { - syncCatalog: { - streams: [ - { - ...mockInitialValues.syncCatalog?.streams[0], - config: { ...mockInitialValues.syncCatalog?.streams[0].config, selected: false }, - }, - ], - }, - }, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; - }); - jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line - return [{}, { error: undefined }] as any; // no error - }); - const { result } = renderHook(() => - useCatalogTreeTableRowProps({ - ...mockStream, - config: { ...mockStream.config!, selected: true }, // selected true - }) - ); + testSetup(mockDisabledInitialValues, false, undefined); + + const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.added)); - // we care that iconName="plus" and className="icon plus" but it is tricky to test a react node returned from a hook like this - expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("green"); }); it("should return removed styles for a row that is removed", () => { - // eslint-disable-next-line - jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit - jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - return { - initialValues: { ...mockInitialValues }, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; - }); - jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line - return [{}, { error: undefined }] as any; // no error - }); + testSetup(mockInitialValues, false, undefined); + const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, - config: { ...mockStream.config!, selected: false }, // selected false + config: { ...mockStream.config!, selected: false }, }) ); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.removed)); - - // we care that iconName="minus" and className="icon minus" but it is tricky to test a react node returned from a hook like this - expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("red"); }); it("should return updated styles for a row that is updated", () => { // eslint-disable-next-line - jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit - jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - return { - initialValues: { ...mockInitialValues }, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; - }); - jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line - return [{}, { error: undefined }] as any; // no error - }); + testSetup(mockInitialValues, false, undefined); + const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, - config: { ...mockStream.config!, syncMode: "incremental", destinationSyncMode: "append_dedup" }, // new sync mode and destination sync mode + config: { ...mockStream.config!, syncMode: "incremental", destinationSyncMode: "append_dedup" }, }) ); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); - // we care that this is our custom ModificationIcon component with modificationColor as its color - expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("blue"); }); + it("should return added styles for a row that is both added and updated", () => { - // eslint-disable-next-line - jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit - jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - return { - initialValues: { - syncCatalog: { - streams: [ - { - ...mockInitialValues.syncCatalog?.streams[0], - config: { ...mockInitialValues.syncCatalog?.streams[0].config, selected: false }, - }, - ], - }, - }, // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; - }); - jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line - return [{}, { error: undefined }] as any; // no error - }); + testSetup(mockDisabledInitialValues, false, undefined); + const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, @@ -200,33 +127,10 @@ describe("useCatalogTreeTableRowProps", () => { ); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.added)); - // we care that iconName="plus" and className="icon plus" but it is tricky to test a react node returned from a hook like this - expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("green"); }); - it("should return change background color with relevant icon if selected for bulk edit", () => { - // eslint-disable-next-line - jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [true, () => null] as any); // not selected for bulk edit - jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - return { - initialValues: { - syncCatalog: { - streams: [ - { - ...mockInitialValues.syncCatalog?.streams[0], - config: { ...mockInitialValues.syncCatalog?.streams[0].config, selected: false }, - }, - ], - }, - }, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; - }); - - jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line - return [{}, { error: undefined }] as any; // no error - }); + it("should return change background color if selected for bulk edit", () => { + testSetup(mockInitialValues, true, undefined); const { result } = renderHook(() => useCatalogTreeTableRowProps({ @@ -235,21 +139,10 @@ describe("useCatalogTreeTableRowProps", () => { }) ); expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); - // we care that iconName="plus" and className="icon plus" but it is tricky to test a react node returned from a hook like this - expect(result.current.statusIcon).toMatchSnapshot(); expect(result.current.pillButtonVariant).toEqual("blue"); }); it("should return error styles for a row that has an error", () => { - // eslint-disable-next-line - jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [false, () => null] as any); // not selected for bulk edit - jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - // eslint-disable-next-line - return { initialValues: mockInitialValues } as any; - }); - jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line - return [{}, { error: true }] as any; // no error - }); + testSetup(mockInitialValues, false, "error"); const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx index 2b4e1869a68f4..513e0f873f223 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx @@ -1,12 +1,9 @@ /* eslint-disable css-modules/no-unused-class */ -import { faMinus, faPlus } from "@fortawesome/free-solid-svg-icons"; -import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import classNames from "classnames"; import { useField } from "formik"; import { isEqual } from "lodash"; import { useMemo } from "react"; -import { ModificationIcon } from "components/icons/ModificationIcon"; import { PillButtonVariant } from "components/ui/PillSelect/PillButton"; import { SyncSchemaStream } from "core/domain/catalog"; @@ -64,33 +61,6 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { return "grey"; }, [isSelected, statusToDisplay]); - const statusIcon = useMemo(() => { - if (statusToDisplay === "added") { - return ( - - ); - } else if (statusToDisplay === "removed") { - return ( - - ); - } else if (statusToDisplay === "changed") { - return ( -
- -
- ); - } - return null; - }, [isSelected, statusToDisplay]); - const streamHeaderContentStyle = classNames(styles.streamHeaderContent, { [styles.added]: statusToDisplay === "added" && !isSelected, [styles.removed]: statusToDisplay === "removed" && !isSelected, @@ -101,7 +71,8 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { return { streamHeaderContentStyle, - statusIcon, + isSelected, + statusToDisplay, pillButtonVariant, }; }; From 41042ea99de2dc363d0b1e672ffffe57fe449985 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Wed, 23 Nov 2022 17:15:20 -0500 Subject: [PATCH 17/25] tidy up test linting disables --- .../CatalogTree/next/useCatalogTreeTableRowProps.test.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx index d6534c2823b4e..d09f34191d859 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/no-non-null-assertion */ import { renderHook } from "@testing-library/react-hooks"; import classNames from "classnames"; import * as formik from "formik"; @@ -39,6 +38,7 @@ const mockDisabledInitialValues: Partial = { streams: [ { ...mockInitialValues.syncCatalog?.streams[0], + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockInitialValues.syncCatalog!.streams[0].config!, selected: false }, }, ], @@ -73,6 +73,7 @@ describe("useCatalogTreeTableRowProps", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, selected: false }, }) ); @@ -94,6 +95,7 @@ describe("useCatalogTreeTableRowProps", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, selected: false }, }) ); @@ -108,6 +110,7 @@ describe("useCatalogTreeTableRowProps", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, syncMode: "incremental", destinationSyncMode: "append_dedup" }, }) ); @@ -135,6 +138,7 @@ describe("useCatalogTreeTableRowProps", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps({ ...mockStream, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion config: { ...mockStream.config!, selected: true }, // selected true, new sync, mode and destination sync mode }) ); From 68a45278bffb6eaa4750dee8fcd7670ec07e2cdb Mon Sep 17 00:00:00 2001 From: Teal Larson Date: Mon, 5 Dec 2022 10:05:02 -0500 Subject: [PATCH 18/25] Update airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx Co-authored-by: Krishna (kc) Glick --- .../connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx index 513e0f873f223..bcc44cd75e13e 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx @@ -11,6 +11,7 @@ import { useBulkEditSelect } from "hooks/services/BulkEdit/BulkEditService"; import { useConnectionFormService } from "hooks/services/ConnectionForm/ConnectionFormService"; import styles from "./CatalogTreeTableRow.module.scss"; +type StatusToDisplay = "disabled" | "added" | "removed" | "changed" | "unchanged"; export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { const { initialValues } = useConnectionFormService(); From 91c5825197fb61504d55502453b56e3bb1bdddde Mon Sep 17 00:00:00 2001 From: Teal Larson Date: Mon, 5 Dec 2022 10:05:10 -0500 Subject: [PATCH 19/25] Update airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx Co-authored-by: Krishna (kc) Glick --- .../connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx index bcc44cd75e13e..8323b9f9ed6dd 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx @@ -24,7 +24,7 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { const isStreamEnabled = stream.config?.selected; - const statusToDisplay = useMemo(() => { + const statusToDisplay = useMemo(() => { const rowStatusChanged = initialValues.syncCatalog.streams.find( (item) => item.stream?.name === stream.stream?.name && item.stream?.namespace === stream.stream?.namespace From 202687f54e4db997dbc1897f991379e306a56bfc Mon Sep 17 00:00:00 2001 From: Teal Larson Date: Mon, 5 Dec 2022 10:05:24 -0500 Subject: [PATCH 20/25] Update airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx Co-authored-by: Krishna (kc) Glick --- .../connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx index 8323b9f9ed6dd..344325098e96a 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx @@ -51,7 +51,7 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => { return "unchanged"; }, [initialValues.syncCatalog.streams, isStreamEnabled, stream.config, stream.stream]); - const pillButtonVariant: PillButtonVariant = useMemo(() => { + const pillButtonVariant = useMemo(() => { if (statusToDisplay === "added" && !isSelected) { return "green"; } else if (statusToDisplay === "removed" && !isSelected) { From da64e4aeeb500fc1f46b496a21e2cb57fc270994 Mon Sep 17 00:00:00 2001 From: Teal Larson Date: Mon, 5 Dec 2022 10:05:31 -0500 Subject: [PATCH 21/25] Update airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx Co-authored-by: Krishna (kc) Glick --- .../CatalogTree/next/useCatalogTreeTableRowProps.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx index d09f34191d859..346a39eb1354f 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx @@ -49,7 +49,7 @@ const testSetup = (initialValues: Partial, isBulkEdi // eslint-disable-next-line @typescript-eslint/no-explicit-any jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [isBulkEdit, () => null] as any); // not selected for bulk edit jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { - // eslint-disable-next-line +// eslint-disable-next-line @typescript-eslint/no-explicit-any return { initialValues: initialValues } as any; }); jest.spyOn(formik, "useField").mockImplementationOnce(() => { From b5e3b87dc756c1c760746e4ac0b8280c72014446 Mon Sep 17 00:00:00 2001 From: Teal Larson Date: Mon, 5 Dec 2022 10:50:51 -0500 Subject: [PATCH 22/25] Update airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx Co-authored-by: Krishna (kc) Glick --- .../CatalogTree/next/useCatalogTreeTableRowProps.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx index 346a39eb1354f..de43f762dfb53 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx @@ -53,7 +53,7 @@ const testSetup = (initialValues: Partial, isBulkEdi return { initialValues: initialValues } as any; }); jest.spyOn(formik, "useField").mockImplementationOnce(() => { - // eslint-disable-next-line + // eslint-disable-next-line @typescript-eslint/no-explicit-any return [{}, { error: error }] as any; // no error }); }; From 31a94ce54808e6beb7faca80211aaccc9b99ce02 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Mon, 5 Dec 2022 10:56:54 -0500 Subject: [PATCH 23/25] clean up tests --- .../next/useCatalogTreeTableRowProps.test.tsx | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx index de43f762dfb53..e1a5c4710ac44 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx @@ -1,5 +1,4 @@ import { renderHook } from "@testing-library/react-hooks"; -import classNames from "classnames"; import * as formik from "formik"; import { AirbyteStreamAndConfiguration } from "core/request/AirbyteClient"; @@ -8,7 +7,6 @@ import * as connectionFormService from "hooks/services/ConnectionForm/Connection import { FormikConnectionFormValues } from "views/Connection/ConnectionForm/formConfig"; // eslint-disable-next-line css-modules/no-unused-class -import styles from "./CatalogTreeTableRow.module.scss"; import { useCatalogTreeTableRowProps } from "./useCatalogTreeTableRowProps"; const mockStream: Partial = { @@ -49,12 +47,12 @@ const testSetup = (initialValues: Partial, isBulkEdi // eslint-disable-next-line @typescript-eslint/no-explicit-any jest.spyOn(bulkEditService, "useBulkEditSelect").mockImplementation(() => [isBulkEdit, () => null] as any); // not selected for bulk edit jest.spyOn(connectionFormService, "useConnectionFormService").mockImplementation(() => { -// eslint-disable-next-line @typescript-eslint/no-explicit-any - return { initialValues: initialValues } as any; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return { initialValues } as any; }); jest.spyOn(formik, "useField").mockImplementationOnce(() => { // eslint-disable-next-line @typescript-eslint/no-explicit-any - return [{}, { error: error }] as any; // no error + return [{}, { error }] as any; // no error }); }; @@ -64,7 +62,7 @@ describe("useCatalogTreeTableRowProps", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); - expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent)); + expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent"); expect(result.current.pillButtonVariant).toEqual("grey"); }); it("should return disabled styles for a row that starts disabled", () => { @@ -78,7 +76,7 @@ describe("useCatalogTreeTableRowProps", () => { }) ); - expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.disabled)); + expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent disabled"); expect(result.current.pillButtonVariant).toEqual("grey"); }); it("should return added styles for a row that is added", () => { @@ -86,7 +84,7 @@ describe("useCatalogTreeTableRowProps", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); - expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.added)); + expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent added"); expect(result.current.pillButtonVariant).toEqual("green"); }); it("should return removed styles for a row that is removed", () => { @@ -100,7 +98,7 @@ describe("useCatalogTreeTableRowProps", () => { }) ); - expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.removed)); + expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent removed"); expect(result.current.pillButtonVariant).toEqual("red"); }); it("should return updated styles for a row that is updated", () => { @@ -115,7 +113,7 @@ describe("useCatalogTreeTableRowProps", () => { }) ); - expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); + expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent changed"); expect(result.current.pillButtonVariant).toEqual("blue"); }); @@ -129,7 +127,7 @@ describe("useCatalogTreeTableRowProps", () => { }) ); - expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.added)); + expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent added"); expect(result.current.pillButtonVariant).toEqual("green"); }); it("should return change background color if selected for bulk edit", () => { @@ -142,7 +140,7 @@ describe("useCatalogTreeTableRowProps", () => { config: { ...mockStream.config!, selected: true }, // selected true, new sync, mode and destination sync mode }) ); - expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.changed)); + expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent changed"); expect(result.current.pillButtonVariant).toEqual("blue"); }); it("should return error styles for a row that has an error", () => { @@ -150,6 +148,6 @@ describe("useCatalogTreeTableRowProps", () => { const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream)); - expect(result.current.streamHeaderContentStyle).toEqual(classNames(styles.streamHeaderContent, styles.error)); + expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent error"); }); }); From e9cb95f26c4ed78f407226573a27b32ed645d616 Mon Sep 17 00:00:00 2001 From: tealjulia Date: Mon, 5 Dec 2022 11:25:10 -0500 Subject: [PATCH 24/25] make path-select dropdown full column width --- .../connection/CatalogTree/next/CatalogTreeTableRow.tsx | 1 - .../CatalogTree/next/StreamPathSelect.module.scss | 6 +++++- .../connection/CatalogTree/next/StreamPathSelect.tsx | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx index 4633d9a7e0258..7da5ec35ec243 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx @@ -57,7 +57,6 @@ export const CatalogTreeTableRow: React.FC = ({ const checkboxCellCustomStyle = classnames(styles.checkboxCell, styles.streamRowCheckboxCell); - // todo: the primary key and cursor dropdowns should be full width of the cell return ( {!disabled && ( diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.module.scss b/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.module.scss index c498cd19a1b09..6653a7af6fe52 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.module.scss +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.module.scss @@ -4,6 +4,10 @@ white-space: nowrap; } -.pillSelect { +.streamPathSelect { width: 100%; + min-width: 0; + display: flex; + flex-direction: row; + justify-content: space-between; } diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx index eb0eeb6264170..3e95eec206061 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx @@ -71,6 +71,7 @@ export const StreamPathSelect: React.FC = (props) => { props.onPathChange(finalValues); }} variant={props.variant} + className={styles.streamPathSelect} /> ); }; From 749d257bf8c5093f79ed7dc2d60befe64bf1df1e Mon Sep 17 00:00:00 2001 From: tealjulia Date: Mon, 5 Dec 2022 11:54:02 -0500 Subject: [PATCH 25/25] cleanup from rebase --- .../connection/CatalogTree/next/StreamPathSelect.tsx | 3 --- .../connection/CatalogTree/next/SyncModeSelect.tsx | 12 +----------- .../CatalogTree/next/useCatalogTreeTableRowProps.tsx | 2 +- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx index 3e95eec206061..c4e7df8e77682 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx @@ -2,7 +2,6 @@ import React from "react"; import { FormattedMessage } from "react-intl"; import { PillButtonVariant, PillSelect } from "components/ui/PillSelect"; - import { Text } from "components/ui/Text"; import { Tooltip } from "components/ui/Tooltip"; @@ -62,7 +61,6 @@ export const StreamPathSelect: React.FC = (props) => { = (props) => { const finalValues = Array.isArray(options) ? options.map((op) => op.value) : options.value; props.onPathChange(finalValues); }} - variant={props.variant} className={styles.streamPathSelect} /> ); diff --git a/airbyte-webapp/src/components/connection/CatalogTree/next/SyncModeSelect.tsx b/airbyte-webapp/src/components/connection/CatalogTree/next/SyncModeSelect.tsx index 8b0421542fa8b..ee271bd999336 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/next/SyncModeSelect.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/next/SyncModeSelect.tsx @@ -4,7 +4,6 @@ import { FormattedMessage } from "react-intl"; import { DropDownOptionDataItem } from "components/ui/DropDown"; import { PillSelect, PillButtonVariant } from "components/ui/PillSelect"; -import { PillButtonVariant } from "components/ui/PillSelect/PillButton"; import { DestinationSyncMode, SyncMode } from "core/request/AirbyteClient"; @@ -25,17 +24,9 @@ interface SyncModeSelectProps { options: SyncModeOption[]; value: Partial; variant?: PillButtonVariant; - variant?: PillButtonVariant; } -export const SyncModeSelect: React.FC = ({ - className, - variant, - options, - onChange, - value, - variant, -}) => { +export const SyncModeSelect: React.FC = ({ className, options, onChange, value, variant }) => { const pillSelectOptions = useMemo(() => { return options.map(({ value }) => { const { syncMode, destinationSyncMode } = value; @@ -54,7 +45,6 @@ export const SyncModeSelect: React.FC = ({ return (