🪟 🧹 Custom connectors in Cloud UI updates #21034

teallarson · 2023-01-04T19:12:42Z

What

closes airbytehq/airbyte-cloud#3753

Connector doc URL should not be required for custom connectors on cloud
More useful placeholder text
Update field label for docker repository name --> docker full image name
Check if docs URL is a valid URL for custom connectors
Denote which fields are required in the form
Docs side panel shouldn't show for custom connectors

How

Conditional yup schema, label, and placeholder in the modal form based on whether we are in Cloud mode.
Also added some better visual feedback for validation errors (unblocking submit button as soon as the form is dirty, adding input highlighting and messaging for errors)

Recommended reading order

<CreateConnectorModal />
everything else

teallarson · 2023-01-04T19:15:05Z

airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/CreateConnectorModal.tsx

@@ -117,7 +122,7 @@ const CreateConnectorModal: React.FC<IProps> = ({ onClose, onSubmit, errorMessag
          }}
          validateOnBlur
          validateOnChange
-          validationSchema={validationSchema}
+          validationSchema={isCloudApp() === false ? standardValidationSchema : customConnectorValidationSchema}


In general, I would use the FeatureService over isCloudApp(), but the AllowCustomImageUpload doesn't tell us if a user is on Cloud specifically. Open to a better check if someone has one!

teallarson · 2023-01-04T21:34:08Z

airbyte-webapp/src/locales/en.json

  "admin.dockerImageTag": "Docker image tag",
  "admin.dockerImageTag.placeholder": "12.3",
-  "admin.documentationUrl": "Connector Documentation URL",
+  "admin.documentationUrl": "Connector documentation URL",


casing was inconsistent with other labels

teallarson · 2023-01-05T19:39:09Z

airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/ImageCell.tsx

@@ -18,6 +18,10 @@ const Link = styled.a`
 `;

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


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

...yte-webapp/src/views/Connector/ConnectorDocumentationLayout/ConnectorDocumentationLayout.tsx

teallarson · 2023-01-05T20:11:56Z

airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/CreateConnectorModal.tsx

-  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(),


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.

teallarson · 2023-01-05T20:33:49Z

Note: I have not been able to manually test the documentation panel behavior for a custom connector on cloud, however I have verified using OSS, including the behavior for a connector with no documentation URL and an external documentation URL.

Custom connectors on cloud dev environments is blocked by airbytehq/airbyte-cloud/pull/3952

There is a workaround but I feel confident having seen the behavior in OSS.

teallarson · 2023-01-05T20:36:13Z

airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/CreateConnectorModal.tsx

-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}>


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

josephkmh

Looking good! Just left a few minor comments. I tested it locally and the form UX is much nicer ✨

josephkmh · 2023-01-09T09:55:11Z

airbyte-webapp/src/locales/en.json

  "admin.dockerRepository": "Docker repository name",
+  "admin.dockerFullImageName": "Docker full image name",
  "admin.dockerRepository.placeholder": "airbytehq/postgres",
+  "admin.dockerFullImageName.placeholder": "customer-name/customer-image",


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

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

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

airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/CreateConnectorModal.tsx

airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/ImageCell.tsx

...yte-webapp/src/views/Connector/ConnectorDocumentationLayout/ConnectorDocumentationLayout.tsx

…t/ConnectorDocumentationLayout.tsx Co-authored-by: Joey Marshment-Howell <josephkmh@users.noreply.github.com>

…e while I'm at it

teallarson · 2023-01-09T14:52:05Z

Ready for re-review

josephkmh

Looks good!

* check if doc url is a valid url in cloud, but don't require it * field label and placeholder * cleanup * match casing for other form labels * cleanup imports * hide documentation panel if URL is empty * only show documentation panel if url is from our docs * make docs url optional, don't put a link in conncectorsview if empty * Update airbyte-webapp/src/views/Connector/ConnectorDocumentationLayout/ConnectorDocumentationLayout.tsx Co-authored-by: Joey Marshment-Howell <josephkmh@users.noreply.github.com> * use <FormattedMessage />, cleanup from Joey's commit in review comment * Use <Text/> component in ImageCell, get styled components out of there while I'm at it * add "required" labels * show validation error regardless of `touched` for documentationUrl * use old placeholder instead Co-authored-by: Joey Marshment-Howell <josephkmh@users.noreply.github.com>

check if doc url is a valid url in cloud, but don't require it

f91abde

octavia-squidington-iv added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Jan 4, 2023

teallarson commented Jan 4, 2023

View reviewed changes

teallarson added 3 commits January 4, 2023 14:27

field label and placeholder

8741b08

cleanup

b53dc4c

match casing for other form labels

6502399

teallarson commented Jan 4, 2023

View reviewed changes

teallarson and others added 5 commits January 4, 2023 16:44

Merge branch 'master' into teal/custom-connectors-ui-cleanup

8ad2d21

cleanup imports

8c7b8cc

hide documentation panel if URL is empty

79708a1

only show documentation panel if url is from our docs

2a128c0

make docs url optional, don't put a link in conncectorsview if empty

ace147c

teallarson commented Jan 5, 2023

View reviewed changes

...yte-webapp/src/views/Connector/ConnectorDocumentationLayout/ConnectorDocumentationLayout.tsx Outdated Show resolved Hide resolved

teallarson commented Jan 5, 2023

View reviewed changes

Merge branch 'master' into teal/custom-connectors-ui-cleanup

bccd9e0

teallarson marked this pull request as ready for review January 5, 2023 20:18

teallarson requested a review from a team as a code owner January 5, 2023 20:18

teallarson requested review from josephkmh and ambirdsall January 5, 2023 20:34

teallarson commented Jan 5, 2023

View reviewed changes

josephkmh requested changes Jan 9, 2023

View reviewed changes

Update airbyte-webapp/src/views/Connector/ConnectorDocumentationLayou…

65368a3

…t/ConnectorDocumentationLayout.tsx Co-authored-by: Joey Marshment-Howell <josephkmh@users.noreply.github.com>

octavia-squidington-iv removed the area/platform issues related to the platform label Jan 9, 2023

teallarson added 3 commits January 9, 2023 09:19

use <FormattedMessage />, cleanup from Joey's commit in review comment

3f1e749

Use <Text/> component in ImageCell, get styled components out of ther…

321b68f

…e while I'm at it

add "required" labels

daa42c5

teallarson requested a review from josephkmh January 9, 2023 14:51

teallarson and others added 3 commits January 9, 2023 10:53

show validation error regardless of touched for documentationUrl

f9fb974

Merge branch 'master' into teal/custom-connectors-ui-cleanup

2e70d0b

use old placeholder instead

fe9240f

josephkmh approved these changes Jan 10, 2023

View reviewed changes

teallarson merged commit 0b27aa5 into master Jan 10, 2023

teallarson deleted the teal/custom-connectors-ui-cleanup branch January 10, 2023 20:34

octavia-squidington-iii mentioned this pull request Jan 12, 2023

Bump Airbyte version from 0.40.27 to 0.40.28 #21359

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🪟 🧹 Custom connectors in Cloud UI updates #21034

🪟 🧹 Custom connectors in Cloud UI updates #21034

teallarson commented Jan 4, 2023 •

edited

Loading

teallarson Jan 4, 2023

teallarson Jan 4, 2023

teallarson Jan 5, 2023

teallarson Jan 5, 2023

teallarson commented Jan 5, 2023

teallarson Jan 5, 2023

josephkmh left a comment •

edited

Loading

josephkmh Jan 9, 2023

teallarson Jan 9, 2023

sophia-wiley Jan 9, 2023

teallarson commented Jan 9, 2023 •

edited

Loading

josephkmh left a comment

🪟 🧹 Custom connectors in Cloud UI updates #21034

🪟 🧹 Custom connectors in Cloud UI updates #21034

Conversation

teallarson commented Jan 4, 2023 • edited Loading

What

How

Recommended reading order

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teallarson commented Jan 5, 2023

Choose a reason for hiding this comment

josephkmh left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teallarson commented Jan 9, 2023 • edited Loading

josephkmh left a comment

Choose a reason for hiding this comment

teallarson commented Jan 4, 2023 •

edited

Loading

josephkmh left a comment •

edited

Loading

teallarson commented Jan 9, 2023 •

edited

Loading