Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🪟 Add catalog changes modal on schema refresh #14074

Merged
merged 48 commits into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
c6fd501
WIP - types, props, components
teallarson Jun 21, 2022
371b2f1
logic tweaks
teallarson Jun 23, 2022
48bb1c0
moving around
teallarson Jun 23, 2022
6ae438f
begin styling and content
teallarson Jun 28, 2022
837e649
modal formatting, section header
teallarson Jun 28, 2022
a30247b
client update, add/removed streams works
teallarson Jun 28, 2022
93a4c21
theme tweaks
teallarson Jun 29, 2022
9fda9f8
WIP -- adding accordion
teallarson Jun 29, 2022
a9b9560
hook for modal display logic
teallarson Jun 30, 2022
05ab7f5
display logic, row/accordion progress
teallarson Jun 30, 2022
fbd4760
fix atrocities of table rendering, move header to own component
teallarson Jul 1, 2022
b215e5e
headers cleanup
teallarson Jul 1, 2022
c9daed9
headers cleanup
teallarson Jul 1, 2022
f94c2af
imageblock more flexible
teallarson Jul 1, 2022
9fd2e03
progress on table todo: consolidate, complete
teallarson Jul 1, 2022
a5d47f8
styling good, animation TODO
teallarson Jul 6, 2022
af61e73
self review pt. 1
teallarson Jul 8, 2022
05dfe0a
cleanup
teallarson Jul 11, 2022
0cd64ab
note
teallarson Jul 11, 2022
631faef
note
teallarson Jul 11, 2022
81b8d6c
accessibility and i18n improvements
teallarson Jul 12, 2022
584d07c
fix typo in scss
teallarson Jul 12, 2022
90498e9
missig i18l things
teallarson Jul 12, 2022
5043f63
move icon to /icons
teallarson Jul 14, 2022
4cad388
Update airbyte-webapp/src/views/Connection/CatalogDiffModal/CatalogDi…
teallarson Jul 14, 2022
5c95e0e
Update airbyte-webapp/src/views/Connection/CatalogDiffModal/component…
teallarson Jul 14, 2022
c1ce5ea
spacing, use ModalFooter
teallarson Jul 14, 2022
7ef0651
Update airbyte-webapp/src/views/Connection/CatalogDiffModal/component…
teallarson Jul 14, 2022
0f0bbdb
begin moving to memoized reducer function
teallarson Jul 14, 2022
4a4800a
memoize diff sorter and remove extra divs
teallarson Jul 19, 2022
1d0ca6d
cleanup
teallarson Jul 19, 2022
1014d26
modal body padding
teallarson Jul 19, 2022
37f713b
up0date to use modal service
teallarson Jul 20, 2022
b769506
move calculated string mode out of component
teallarson Jul 20, 2022
e5a9076
respond to review
teallarson Jul 25, 2022
fdc8ac5
add accordionheader component
teallarson Jul 25, 2022
4d79876
catalog can be undefined
teallarson Jul 25, 2022
b62b67e
cleanup cell rendering
teallarson Jul 25, 2022
a03f452
cleanup and make storybook work again
teallarson Jul 27, 2022
72e7a79
move table styles within a parent class
teallarson Aug 1, 2022
f4870f1
subheading alignment consistency
teallarson Aug 2, 2022
50037d7
more padding/spacing adjustments
teallarson Aug 2, 2022
db022de
cleanup from review
teallarson Aug 2, 2022
d611e4b
mixup from rebase
teallarson Aug 2, 2022
64f61db
set width on modal level not content level
teallarson Aug 3, 2022
fa18475
Update airbyte-webapp/src/views/Connection/CatalogDiffModal/utils.tsx
teallarson Aug 3, 2022
7c23669
Update airbyte-webapp/src/views/Connection/CatalogDiffModal/utils.tsx
teallarson Aug 3, 2022
bb9e18d
linting and unused class cleanup
teallarson Aug 3, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions airbyte-webapp/.storybook/withProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,17 @@ import { FeatureService } from "../src/hooks/services/Feature";
import { ConfigServiceProvider, defaultConfig } from "../src/config";
import { DocumentationPanelProvider } from "../src/views/Connector/ConnectorDocumentationLayout/DocumentationPanelContext";
import { ServicesProvider } from "../src/core/servicesProvider";
import {
analyticsServiceContext,
AnalyticsServiceProviderValue,
} from "../src/hooks/services/Analytics";
import { analyticsServiceContext, AnalyticsServiceProviderValue } from "../src/hooks/services/Analytics";

const AnalyticsContextMock: AnalyticsServiceProviderValue = ({
const AnalyticsContextMock: AnalyticsServiceProviderValue = {
analyticsContext: {},
setContext: () => {},
addContextProps: () => {},
removeContextProps: () => {},
service: {
track: () => {},
},
} as unknown) as AnalyticsServiceProviderValue;
} as unknown as AnalyticsServiceProviderValue;

const queryClient = new QueryClient({
defaultOptions: {
Expand All @@ -45,12 +42,9 @@ export const withProviders = (getStory) => (
<MemoryRouter>
<IntlProvider messages={messages} locale={"en"}>
<ThemeProvider theme={theme}>
<ConfigServiceProvider
defaultConfig={defaultConfig}
providers={[]}
>
<ConfigServiceProvider defaultConfig={defaultConfig} providers={[]}>
<DocumentationPanelProvider>
<FeatureService>
<FeatureService features={[]}>
<GlobalStyle />
{getStory()}
</FeatureService>
Expand Down
19 changes: 19 additions & 0 deletions airbyte-webapp/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions airbyte-webapp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"@fortawesome/free-solid-svg-icons": "^6.1.1",
"@fortawesome/react-fontawesome": "^0.1.18",
"@fullstory/browser": "^1.5.1",
"@headlessui/react": "^1.6.5",
"@monaco-editor/react": "^4.4.5",
"@sentry/react": "^6.19.6",
"@sentry/tracing": "^6.19.6",
Expand Down
18 changes: 18 additions & 0 deletions airbyte-webapp/src/components/ImageBlock/ImageBlock.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@
&.darkBlue {
background: colors.$dark-blue-100;
}
&.green {
background: colors.$green;
color: colors.$white;
}

&.red {
background: colors.$red;
color: colors.$white;
}

&.blue {
background: colors.$blue;
color: colors.$white;
}
}

.small {
Expand All @@ -39,4 +53,8 @@
font-size: 10px;
color: colors.$white;
padding: 3px 0 3px;

&.light {
font-weight: 500;
}
}
18 changes: 14 additions & 4 deletions airbyte-webapp/src/components/ImageBlock/ImageBlock.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,28 @@ interface ImageBlockProps {
img?: string;
num?: number;
small?: boolean;
color?: string;
light?: boolean;
ariaLabel?: string;
}

export const ImageBlock: React.FC<ImageBlockProps> = ({ img, num, small }) => {
export const ImageBlock: React.FC<ImageBlockProps> = ({ img, num, small, color, light, ariaLabel }) => {
const imageCircleClassnames = classnames({
[styles.circle]: num,
[styles.iconContainer]: !num || num === undefined,
[styles.small]: small && !num,
[styles.darkBlue]: !small && num,
[styles.darkBlue]: !small && num && !color,
[styles.green]: color === "green",
[styles.red]: color === "red",
[styles.blue]: color === "blue",
[styles.light]: light,
});

const numberStyles = classnames(styles.number, { [styles.light]: light });

return (
<div className={imageCircleClassnames}>
{num ? <div className={styles.number}>{num}</div> : <div className={styles.icon}>{getIcon(img)}</div>}
<div className={imageCircleClassnames} aria-label={ariaLabel}>
Copy link
Contributor Author

@teallarson teallarson Jul 12, 2022

Choose a reason for hiding this comment

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

Is aria-label right here? I believe that's what @timroes had suggestd for these, though I can't remember why that and not aria-description. This isn't an interactive element.

Good news in general: We can now start adding aria-labels other places that use this component as well (this is used to render logos on our list pages, the little circles with the numbers in it on list pages, etc.)

{num ? <div className={numberStyles}>{num}</div> : <div className={styles.icon}>{getIcon(img)}</div>}
</div>
);
};
Expand Down
3 changes: 3 additions & 0 deletions airbyte-webapp/src/components/Modal/ModalBody.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@
padding: variables.$spacing-lg variables.$spacing-xl;
overflow: auto;
max-width: 100%;
&.paddingNone {
padding: 0;
}
}
10 changes: 8 additions & 2 deletions airbyte-webapp/src/components/Modal/ModalBody.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
import classnames from "classnames";

import styles from "./ModalBody.module.scss";

interface ModalBodyProps {
maxHeight?: number | string;
padded?: boolean;
}

export const ModalBody: React.FC<ModalBodyProps> = ({ children, maxHeight }) => {
export const ModalBody: React.FC<ModalBodyProps> = ({ children, maxHeight, padded = true }) => {
const modalStyles = classnames(styles.modalBody, {
[styles.paddingNone]: !padded,
});
return (
<div className={styles.modalBody} style={{ maxHeight }}>
<div className={modalStyles} style={{ maxHeight }}>
{children}
</div>
);
Expand Down
18 changes: 18 additions & 0 deletions airbyte-webapp/src/components/icons/ModificationIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export const ModificationIcon: React.FC = () => {
return (
<svg width="20" height="20" viewBox="0 0 20 20" fill="none">
<path
fillRule="evenodd"
clipRule="evenodd"
d="M10.8332 9.16663L14.1665 6.66663L10.8332 4.16663V5.83329H4.1665V7.49996H10.8332V9.16663Z"
fill="#CBC8FF"
/>
<path
fillRule="evenodd"
clipRule="evenodd"
d="M9.16683 15.8334L5.8335 13.3334L9.16683 10.8334V12.5H15.8335V14.1667H9.16683V15.8334Z"
fill="#CBC8FF"
/>
</svg>
);
};
2 changes: 1 addition & 1 deletion airbyte-webapp/src/hooks/services/Modal/ModalService.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const ModalServiceProvider: React.FC = ({ children }) => {
<Modal
title={modalOptions.title}
size={modalOptions.size}
onClose={() => resultSubjectRef.current?.next({ type: "canceled" })}
onClose={modalOptions.preventCancel ? undefined : () => resultSubjectRef.current?.next({ type: "canceled" })}
>
<modalOptions.content
onCancel={() => resultSubjectRef.current?.next({ type: "canceled" })}
Copy link
Contributor

Choose a reason for hiding this comment

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

The modal could still cancel here. Is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, our modals remain unchanged in that we should be able to cancel them by clicking "cancel" or pressing escape.

Setting onClose as undefined here when the preventCancel property is true should prevent the modal from closing via cancel. That's what I saw when testing it manually (both by clicking outside the modal and hitting "escape" on the keyboard). Is there something I am not thinking of there?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think if the implementation calls onCancel we would still allow to cancel the dialog, and only prevent the "self closing feature" of it. Though there might be a better property name than preventCancel to express that maybe, but I don't have any better ideas than preventCancel.

Expand Down
1 change: 1 addition & 0 deletions airbyte-webapp/src/hooks/services/Modal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ModalProps } from "components/Modal/Modal";
export interface ModalOptions<T> {
title: ModalProps["title"];
size?: ModalProps["size"];
preventCancel?: boolean;
content: React.ComponentType<ModalContentProps<T>>;
}

Expand Down
11 changes: 11 additions & 0 deletions airbyte-webapp/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,17 @@
"connection.sourceTestAgain": "Test source connection again",
"connection.resetData": "Reset your data",
"connection.updateSchema": "Refresh source schema",
"connection.updateSchema.completed": "Refreshed source schema",
"connection.updateSchema.confirm": "Confirm",
"connection.updateSchema.new": "{value} new {item}",
"connection.updateSchema.removed": "{value} removed {item}",
"connection.updateSchema.changed": "{value} {item} with changes",
"connection.updateSchema.stream": "{count, plural, one {table} other {tables}}",
"connection.updateSchema.field": "{count, plural, one {field} other {fields}}",
"connection.updateSchema.streamName": "Stream name",
"connection.updateSchema.namespace": "Namespace",
"connection.updateSchema.dataType": "Data type",

"connection.newConnection": "+ New connection",
"connection.newConnectionTitle": "New connection",
"connection.noConnections": "Connection list is empty",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
ValuesProps,
} from "hooks/services/useConnectionHook";
import { equal } from "utils/objects";
import { CatalogDiffModal } from "views/Connection/CatalogDiffModal/CatalogDiffModal";
import ConnectionForm from "views/Connection/ConnectionForm";
import { ConnectionFormSubmitResult } from "views/Connection/ConnectionForm/ConnectionForm";
import { FormikConnectionFormValues } from "views/Connection/ConnectionForm/formConfig";
Expand Down Expand Up @@ -87,7 +88,6 @@ export const ReplicationView: React.FC<ReplicationViewProps> = ({ onAfterSaveSch
const [saved, setSaved] = useState(false);
const [connectionFormValues, setConnectionFormValues] = useState<FormikConnectionFormValues>();
const connectionService = useConnectionService();

const { mutateAsync: updateConnection } = useUpdateConnection();

const { connection: initialConnection, refreshConnectionCatalog } = useConnectionLoad(connectionId);
Expand Down Expand Up @@ -177,7 +177,16 @@ export const ReplicationView: React.FC<ReplicationViewProps> = ({ onAfterSaveSch
const onRefreshSourceSchema = async () => {
setSaved(false);
setActiveUpdatingSchemaMode(true);
await refreshCatalog();
const { catalogDiff, syncCatalog } = await refreshCatalog();
if (catalogDiff?.transforms && catalogDiff.transforms.length > 0) {
await openModal<void>({
title: formatMessage({ id: "connection.updateSchema.completed" }),
preventCancel: true,
content: ({ onClose }) => (
<CatalogDiffModal catalogDiff={catalogDiff} catalog={syncCatalog} onClose={onClose} />
),
});
}
};

const onCancelConnectionFormEdit = () => {
Expand Down
2 changes: 1 addition & 1 deletion airbyte-webapp/src/scss/_colors.scss
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ $green-800: #007c84;
$green-900: #005959;
$green: $green-200;

$red-50: #ffbac6;
$red-50: #ffe4e8;
$red-100: #ffbac6;
$red-200: #ff8da1;
$red-300: #ff5e7b;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@use "../../../scss/variables";

.modalContent {
display: flex;
flex-direction: column;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { useMemo } from "react";
import { FormattedMessage } from "react-intl";

import { Button } from "components";

import { AirbyteCatalog, CatalogDiff } from "core/request/AirbyteClient";

import { ModalBody, ModalFooter } from "../../../components/Modal";
import styles from "./CatalogDiffModal.module.scss";
import { DiffSection } from "./components/DiffSection";
import { FieldSection } from "./components/FieldSection";
import { getSortedDiff } from "./utils";

interface CatalogDiffModalProps {
catalogDiff: CatalogDiff;
catalog: AirbyteCatalog;
onClose: () => void;
}

export const CatalogDiffModal: React.FC<CatalogDiffModalProps> = ({ catalogDiff, catalog, onClose }) => {
const { newItems, removedItems, changedItems } = useMemo(
() => getSortedDiff(catalogDiff.transforms),
[catalogDiff.transforms]
);

return (
<>
<ModalBody maxHeight={400} padded={false}>
<div className={styles.modalContent}>
{removedItems.length > 0 && <DiffSection streams={removedItems} diffVerb="removed" catalog={catalog} />}
{newItems.length > 0 && <DiffSection streams={newItems} diffVerb="new" />}
{changedItems.length > 0 && <FieldSection streams={changedItems} diffVerb="changed" />}
</div>
</ModalBody>
<ModalFooter>
<Button onClick={() => onClose()}>
<FormattedMessage id="connection.updateSchema.confirm" />
</Button>
</ModalFooter>
</>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@use "../../../../scss/variables";
@forward "../components/StreamRow.module.scss";
@forward "../components/DiffSection.module.scss";

.accordionContainer {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is DiffAccordion a unique design for the diff modal or is it a more generic component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's unique for this modal built on top of the Headless UI "disclosure" component. I didn't feel like there was enough I did on top of what Headless UI already provided to implement a generic Airbyte "Accordion", although that was my original plan. Happy to revisit the idea if it seems worthwhile.

width: 100%;
}

.accordionButton {
width: 100%;
background: none;
border: none;
height: 40px;
display: flex;
flex-direction: row;
align-items: center;
justify-content: flex-start;
padding: variables.$spacing-sm;
font-weight: 400;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { Disclosure } from "@headlessui/react";
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

Copy link
Contributor

Choose a reason for hiding this comment

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

🚀

import { useMemo } from "react";

import { StreamTransform } from "core/request/AirbyteClient";

import { getSortedDiff } from "../utils";
import styles from "./DiffAccordion.module.scss";
import { DiffAccordionHeader } from "./DiffAccordionHeader";
import { DiffFieldTable } from "./DiffFieldTable";

interface DiffAccordionProps {
streamTransform: StreamTransform;
}

export const DiffAccordion: React.FC<DiffAccordionProps> = ({ streamTransform }) => {
const { newItems, removedItems, changedItems } = useMemo(
() => getSortedDiff(streamTransform.updateStream),
[streamTransform.updateStream]
);

return (
<div className={styles.accordionContainer}>
<Disclosure>
{({ open }) => (
<>
<Disclosure.Button className={styles.accordionButton}>
<DiffAccordionHeader
streamDescriptor={streamTransform.streamDescriptor}
removedCount={removedItems.length}
newCount={newItems.length}
changedCount={changedItems.length}
open={open}
/>
</Disclosure.Button>
<Disclosure.Panel>
{removedItems.length > 0 && <DiffFieldTable fieldTransforms={removedItems} diffVerb="removed" />}
{newItems.length > 0 && <DiffFieldTable fieldTransforms={newItems} diffVerb="new" />}
{changedItems.length > 0 && <DiffFieldTable fieldTransforms={changedItems} diffVerb="changed" />}
</Disclosure.Panel>
</>
)}
</Disclosure>
</div>
);
};
Loading