-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Changes from all commits
f91abde
8741b08
b53dc4c
6502399
8ad2d21
8c7b8cc
79708a1
2a128c0
ace147c
bccd9e0
65368a3
3f1e749
321b68f
daa42c5
f9fb974
2e70d0b
fe9240f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", | ||
"admin.dockerImageTag": "Docker image tag", | ||
"admin.dockerImageTag.placeholder": "12.3", | ||
"admin.documentationUrl": "Connector Documentation URL", | ||
"admin.documentationUrl": "Connector documentation URL", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.", | ||
|
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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: { | ||
|
@@ -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; | ||
`; | ||
|
@@ -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(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{children} | ||
</Text> | ||
); | ||
}; | ||
|
||
const CreateConnectorModal: React.FC<CreateConnectorModalProps> = ({ onClose, onSubmit, errorMessage }) => { | ||
const { formatMessage } = useIntl(); | ||
|
||
return ( | ||
|
@@ -115,19 +119,17 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag | |
dockerImageTag: "", | ||
dockerRepository: "", | ||
}} | ||
validateOnBlur | ||
validateOnChange | ||
validationSchema={validationSchema} | ||
onSubmit={async (values, { setSubmitting }) => { | ||
await onSubmit(values); | ||
setSubmitting(false); | ||
}} | ||
> | ||
{({ isSubmitting, dirty, isValid }) => ( | ||
{({ isSubmitting, isValid, dirty }) => ( | ||
<Form> | ||
<FieldContainer> | ||
<Field name="name"> | ||
{({ field }: FieldProps<string>) => ( | ||
{({ field, meta }: FieldProps<string>) => ( | ||
<LabeledInput | ||
{...field} | ||
type="text" | ||
|
@@ -139,13 +141,21 @@ 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} /> | ||
) : ( | ||
<FormattedMessage id="form.empty.error" /> | ||
) | ||
} | ||
/> | ||
)} | ||
</Field> | ||
</FieldContainer> | ||
<FieldContainer> | ||
<Field name="dockerRepository"> | ||
{({ field }: FieldProps<string>) => ( | ||
{({ field, meta }: FieldProps<string>) => ( | ||
<LabeledInput | ||
{...field} | ||
type="text" | ||
|
@@ -155,16 +165,26 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag | |
})} | ||
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} /> | ||
) : ( | ||
<FormattedMessage id="form.empty.error" /> | ||
) | ||
} | ||
/> | ||
)} | ||
</Field> | ||
</FieldContainer> | ||
<FieldContainer> | ||
<Field name="dockerImageTag"> | ||
{({ field }: FieldProps<string>) => ( | ||
{({ field, meta }: FieldProps<string>) => ( | ||
<LabeledInput | ||
{...field} | ||
type="text" | ||
|
@@ -177,13 +197,21 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag | |
<FormattedMessage id="admin.dockerImageTag" /> | ||
</Label> | ||
} | ||
error={!!meta.error && meta.touched} | ||
message={ | ||
meta.touched && meta.error ? ( | ||
<FormattedMessage id={meta.error} /> | ||
) : ( | ||
<FormattedMessage id="form.empty.error" /> | ||
) | ||
} | ||
/> | ||
)} | ||
</Field> | ||
</FieldContainer> | ||
<FieldContainer> | ||
<Field name="documentationUrl"> | ||
{({ field }: FieldProps<string>) => ( | ||
{({ field, meta }: FieldProps<string>) => ( | ||
<LabeledInput | ||
{...field} | ||
type="text" | ||
|
@@ -196,6 +224,8 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag | |
<FormattedMessage id="admin.documentationUrl" /> | ||
</Label> | ||
} | ||
error={meta.touched && !!meta.error} | ||
message={meta.error && <FormattedMessage id={meta.error} />} | ||
/> | ||
)} | ||
</Field> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
@use "scss/_variables"; | ||
@use "scss/_colors"; | ||
|
||
.imageNameText { | ||
margin-right: variables.$spacing-md; | ||
} | ||
|
||
.linkText { | ||
&:hover, | ||
&:active { | ||
color: colors.$blue; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,30 @@ | ||
import React from "react"; | ||
import styled from "styled-components"; | ||
import classNames from "classnames"; | ||
import React, { PropsWithChildren } from "react"; | ||
|
||
import { Text } from "components/ui/Text"; | ||
|
||
import styles from "./ImageCell.module.scss"; | ||
|
||
interface ImageCellProps { | ||
imageName: string; | ||
link: string | undefined; | ||
} | ||
|
||
const Link = styled.a` | ||
height: 17px; | ||
margin-right: 9px; | ||
color: ${({ theme }) => theme.darkPrimaryColor}; | ||
const ImageCellText: React.FC<PropsWithChildren<{ className?: string }>> = ({ children, className }) => ( | ||
<Text size="sm" className={classNames(styles.imageNameText, className)}> | ||
{children} | ||
</Text> | ||
); | ||
|
||
&:hover, | ||
&:active { | ||
color: ${({ theme }) => theme.primaryColor}; | ||
const ImageCell: React.FC<ImageCellProps> = ({ imageName, link }) => { | ||
if (!link || !link.length) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 <ImageCellText>{imageName}</ImageCellText>; | ||
} | ||
`; | ||
|
||
const ImageCell: React.FC<ImageCellProps> = ({ imageName, link }) => { | ||
return ( | ||
<Link href={link} target="_blank"> | ||
{imageName} | ||
</Link> | ||
<a href={link} target="_blank" rel="noreferrer" className={styles.linkText}> | ||
<ImageCellText className={styles.linkText}>{imageName}</ImageCellText> | ||
</a> | ||
); | ||
}; | ||
|
||
|
There was a problem hiding this comment.
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 likeus-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
toDocker image name
to align withDocker image tag
WDYT? Cc @xiaohansong @sophia-wileyThere was a problem hiding this comment.
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!
There was a problem hiding this comment.
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!