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

🪟 🧹 Custom connectors in Cloud UI updates #21034

Merged
merged 17 commits into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
4 changes: 3 additions & 1 deletion airbyte-webapp/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -507,10 +507,12 @@
"admin.connectorName": "Connector display name",
"admin.connectorName.placeholder": "Connector name",
"admin.dockerRepository": "Docker repository name",
"admin.dockerFullImageName": "Docker full image name",
"admin.dockerRepository.placeholder": "airbytehq/postgres",
"admin.dockerFullImageName.placeholder": "customer-name/customer-image",
Comment on lines 509 to +512
Copy link
Contributor

Choose a reason for hiding this comment

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

On second though, I'm not sure if the distinction between OSS & cloud is very useful. I don't think customer-name/customer-image is self-explanatory - customers will need to be informed or read the docs to know exactly what to put in this field (not just e.g. xiaohan-test/xiaohan-custom-fake but rather the full path like us-central1-docker.pkg.dev/airbyte-custom-connectors/xiaohan-test/xiaohan-custom-fake). Since we have a link to the docs already, I think putting explicit instructions there is sufficient.

I would suggest we keep more or less what we had before, just changing Docker repository name to Docker image name to align with Docker image tag WDYT? Cc @xiaohansong @sophia-wiley

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, but I'll wait for those tagged to chime in before I make the change!

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that using Docker image name is the best way to go!

"admin.dockerImageTag": "Docker image tag",
"admin.dockerImageTag.placeholder": "12.3",
"admin.documentationUrl": "Connector Documentation URL",
"admin.documentationUrl": "Connector documentation URL",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

casing was inconsistent with other labels

"admin.reloadAfterSuccess": "Note: The app will be reloaded after success submit",
"admin.documentationUrl.placeholder": "https://hub.docker.com/r/airbyte/integration-singer-postgres-source",
"admin.learnMore": "<lnk>Learn more from our documentation</lnk> to understand how to fill these fields.",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@use "src/scss/variables";
@use "scss/variables";

.buttonWithMargin {
margin: 0 variables.$spacing-md;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import { LabeledInput, Link } from "components";
import { Button } from "components/ui/Button";
import { Modal } from "components/ui/Modal";
import { StatusIcon } from "components/ui/StatusIcon";
import { Text } from "components/ui/Text";

import { isCloudApp } from "utils/app";
import { links } from "utils/links";

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

export interface IProps {
export interface CreateConnectorModalProps {
errorMessage?: string;
onClose: () => void;
onSubmit: (sourceDefinition: {
Expand All @@ -36,12 +38,6 @@ const ButtonContent = styled.div`
min-height: 40px;
`;

const Label = styled.div`
font-weight: bold;
color: ${({ theme }) => theme.darkPrimaryColor};
margin-bottom: 8px;
`;

const FieldContainer = styled.div`
margin-bottom: 21px;
`;
Expand Down Expand Up @@ -84,13 +80,21 @@ const ErrorText = styled.div`
max-width: 400px;
`;
const validationSchema = yup.object().shape({
name: yup.string().required("form.empty.error"),
documentationUrl: yup.string().required("form.empty.error"),
dockerImageTag: yup.string().required("form.empty.error"),
dockerRepository: yup.string().required("form.empty.error"),
name: yup.string().trim().required("form.empty.error"),
documentationUrl: yup.string().trim().url("form.url.error").notRequired(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this makes this field optional for OSS as well, which also does not appear to do anything with it beyond the link on the connectors list in settings + the docs panel.

dockerImageTag: yup.string().trim().required("form.empty.error"),
dockerRepository: yup.string().trim().required("form.empty.error"),
});

const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessage }) => {
const Label: React.FC<React.PropsWithChildren<unknown>> = ({ children }) => {
return (
<Text as="span" bold size="lg" className={styles.label}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

as="span" allows us to use the built in message prop on the input to display any validation errors

{children}
</Text>
);
};

const CreateConnectorModal: React.FC<CreateConnectorModalProps> = ({ onClose, onSubmit, errorMessage }) => {
const { formatMessage } = useIntl();

return (
Expand All @@ -116,18 +120,18 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag
dockerRepository: "",
}}
validateOnBlur
validateOnChange
validateOnChange={false}
teallarson marked this conversation as resolved.
Show resolved Hide resolved
validationSchema={validationSchema}
onSubmit={async (values, { setSubmitting }) => {
await onSubmit(values);
setSubmitting(false);
}}
>
{({ isSubmitting, dirty, isValid }) => (
{({ isSubmitting, dirty }) => (
<Form>
<FieldContainer>
<Field name="name">
{({ field }: FieldProps<string>) => (
{({ field, meta }: FieldProps<string>) => (
<LabeledInput
{...field}
type="text"
Expand All @@ -139,32 +143,40 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag
<FormattedMessage id="admin.connectorName" />
</Label>
}
error={meta.touched && !!meta.error}
message={meta.touched && meta.error && <FormattedMessage id={meta.error} />}
/>
)}
</Field>
</FieldContainer>
<FieldContainer>
<Field name="dockerRepository">
{({ field }: FieldProps<string>) => (
{({ field, meta }: FieldProps<string>) => (
<LabeledInput
{...field}
type="text"
autoComplete="off"
placeholder={formatMessage({
id: "admin.dockerRepository.placeholder",
id: `${
isCloudApp() ? "admin.dockerFullImageName.placeholder" : "admin.dockerRepository.placeholder"
josephkmh marked this conversation as resolved.
Show resolved Hide resolved
}`,
})}
label={
<Label>
<FormattedMessage id="admin.dockerRepository" />
<FormattedMessage
id={isCloudApp() ? "admin.dockerFullImageName" : "admin.dockerRepository"}
/>
</Label>
}
error={meta.touched && !!meta.error}
message={meta.touched && meta.error && <FormattedMessage id={meta.error} />}
/>
)}
</Field>
</FieldContainer>
<FieldContainer>
<Field name="dockerImageTag">
{({ field }: FieldProps<string>) => (
{({ field, meta }: FieldProps<string>) => (
<LabeledInput
{...field}
type="text"
Expand All @@ -177,13 +189,15 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag
<FormattedMessage id="admin.dockerImageTag" />
</Label>
}
error={!!meta.error && meta.touched}
message={meta.touched && meta.error && formatMessage({ id: meta.error })}
teallarson marked this conversation as resolved.
Show resolved Hide resolved
/>
)}
</Field>
</FieldContainer>
<FieldContainer>
<Field name="documentationUrl">
{({ field }: FieldProps<string>) => (
{({ field, meta }: FieldProps<string>) => (
<LabeledInput
{...field}
type="text"
Expand All @@ -196,6 +210,8 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag
<FormattedMessage id="admin.documentationUrl" />
</Label>
}
error={meta.touched && !!meta.error}
message={meta.touched && meta.error && formatMessage({ id: meta.error })}
/>
)}
</Field>
Expand All @@ -219,7 +235,7 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag
>
<FormattedMessage id="form.cancel" />
</Button>
<Button type="submit" disabled={isSubmitting || !dirty || !isValid} isLoading={isSubmitting}>
<Button type="submit" disabled={isSubmitting || !dirty} isLoading={isSubmitting}>
teallarson marked this conversation as resolved.
Show resolved Hide resolved
<FormattedMessage id="form.add" />
</Button>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ const Link = styled.a`
`;

const ImageCell: React.FC<ImageCellProps> = ({ imageName, link }) => {
if (!link || !link.length) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the cell in the tables on /settings/sources and /settings/destinations with the docker repository image name and the docs link.

return <>{imageName}</>;
josephkmh marked this conversation as resolved.
Show resolved Hide resolved
}

return (
<Link href={link} target="_blank">
{imageName}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ const LazyDocumentationPanel = lazy(() =>

export const ConnectorDocumentationLayout: React.FC<React.PropsWithChildren<unknown>> = ({ children }) => {
const { formatMessage } = useIntl();
const { documentationPanelOpen } = useDocumentationPanelContext();
const { documentationPanelOpen, documentationUrl } = useDocumentationPanelContext();
const screenWidth = useWindowSize().width;
const showDocumentationPanel = screenWidth > 500 && documentationPanelOpen;
const showDocumentationPanel =
screenWidth > 500 && documentationPanelOpen && documentationUrl.includes("docs.airbyte.com");
teallarson marked this conversation as resolved.
Show resolved Hide resolved

const documentationPanel = (
<Suspense fallback={<LoadingPage />}>
Expand Down