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.

Sign up for GitHub

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
+87 −38
Merged

🪟 🧹 Custom connectors in Cloud UI updates #21034

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f91abde
check if doc url is a valid url in cloud, but don't require it
teallarson Jan 4, 2023
8741b08
field label and placeholder
teallarson Jan 4, 2023
b53dc4c
cleanup
teallarson Jan 4, 2023
6502399
match casing for other form labels
teallarson Jan 4, 2023
8ad2d21
Merge branch 'master' into teal/custom-connectors-ui-cleanup
teallarson Jan 4, 2023
8c7b8cc
cleanup imports
teallarson Jan 5, 2023
79708a1
hide documentation panel if URL is empty
teallarson Jan 5, 2023
2a128c0
only show documentation panel if url is from our docs
teallarson Jan 5, 2023
ace147c
make docs url optional, don't put a link in conncectorsview if empty
teallarson Jan 5, 2023
bccd9e0
Merge branch 'master' into teal/custom-connectors-ui-cleanup
teallarson Jan 5, 2023
65368a3
Update airbyte-webapp/src/views/Connector/ConnectorDocumentationLayou…
teallarson Jan 9, 2023
3f1e749
use <FormattedMessage />, cleanup from Joey's commit in review comment
teallarson Jan 9, 2023
321b68f
Use <Text/> component in ImageCell, get styled components out of ther…
teallarson Jan 9, 2023
daa42c5
add "required" labels
teallarson Jan 9, 2023
f9fb974
show validation error regardless of `touched` for documentationUrl
teallarson Jan 9, 2023
2e70d0b
Merge branch 'master' into teal/custom-connectors-ui-cleanup
teallarson Jan 9, 2023
fe9240f
use old placeholder instead
teallarson Jan 9, 2023
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...p/src/pages/SettingsPage/pages/ConnectorsPage/components/CreateConnectorModal.module.scss
View file
Open in desktop
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
58 changes: 37 additions & 21 deletions ...te-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/CreateConnectorModal.tsx
View file
Open in desktop
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
4 changes: 4 additions & 0 deletions airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/ImageCell.tsx
View file
Open in desktop
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
5 changes: 3 additions & 2 deletions ...-webapp/src/views/Connector/ConnectorDocumentationLayout/ConnectorDocumentationLayout.tsx
View file
Open in desktop
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