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

🪟🎨 Connector form: Improve logs look and feel #20951

Merged
merged 10 commits into from
Jan 9, 2023
1 change: 1 addition & 0 deletions airbyte-webapp/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@
"connector.optionsHeader": "Options",
"connector.exampleValues": "Example {count, plural, one {value} other {values}}",
"connector.oauthCredentialsMissing": "OAuth login is temporarily unavailable for this connector. Please try again later.",
"connector.testLogs": "Connection test logs",

"credits.credits": "Credits",
"credits.whatAreCredits": "What are credits?",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
@use "../../../scss/variables" as vars;
@use "scss/variables";

.cardForm {
padding: 22px 27px 23px 24px;
}

.connectorSelectControl {
margin-bottom: vars.$spacing-xl;
margin-bottom: variables.$spacing-xl;
}

.loaderContainer {
display: flex;
justify-content: center;
align-items: center;
padding: vars.$spacing-2xl 0;
padding: variables.$spacing-2xl 0;
}

.loadingMessage {
margin-top: vars.$spacing-md;
margin-left: vars.$spacing-lg;
margin-top: variables.$spacing-md;
margin-left: variables.$spacing-lg;
}

.connectionTestLogs {
margin-top: variables.$spacing-lg;
}
38 changes: 31 additions & 7 deletions airbyte-webapp/src/views/Connector/ConnectorCard/ConnectorCard.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { faChevronDown, faChevronRight } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import React, { useEffect, useMemo, useState } from "react";
import { FormattedMessage } from "react-intl";

import { JobItem } from "components/JobItem/JobItem";
import JobLogs from "components/JobItem/components/JobLogs";
import { Button } from "components/ui/Button";
import { Card } from "components/ui/Card";
import { Spinner } from "components/ui/Spinner";

Expand All @@ -14,9 +17,13 @@ import {
} from "core/domain/connector";
import { DestinationRead, SourceRead, SynchronousJobRead } from "core/request/AirbyteClient";
import { LogsRequestError } from "core/request/LogsRequestError";
import { useAdvancedModeSetting } from "hooks/services/useAdvancedModeSetting";
import { generateMessageFromError } from "utils/errorStatusMessage";
import { ConnectorCardValues, ConnectorForm, ConnectorFormValues } from "views/Connector/ConnectorForm";
import {
ConnectorCardValues,
ConnectorForm,
ConnectorFormProps,
ConnectorFormValues,
} from "views/Connector/ConnectorForm";

import { useDocumentationPanelContext } from "../ConnectorDocumentationLayout/DocumentationPanelContext";
import { ConnectorDefinitionTypeControl } from "../ConnectorForm/components/Controls/ConnectorServiceTypeControl";
Expand Down Expand Up @@ -81,7 +88,7 @@ export const ConnectorCard: React.FC<ConnectorCardCreateProps | ConnectorCardEdi
const [saved, setSaved] = useState(false);
const [errorStatusRequest, setErrorStatusRequest] = useState<Error | null>(null);
const [isFormSubmitting, setIsFormSubmitting] = useState(false);
const [advancedMode] = useAdvancedModeSetting();
const [logsVisible, setLogsVisible] = useState(false);

const { setDocumentationUrl, setDocumentationPanelOpen } = useDocumentationPanelContext();
const {
Expand Down Expand Up @@ -132,6 +139,11 @@ export const ConnectorCard: React.FC<ConnectorCardCreateProps | ConnectorCardEdi
setDocumentationUrl,
]);

const handleTestConnector: ConnectorFormProps["testConnector"] = (v) => {
setErrorStatusRequest(null);
return testConnector(v);
};

const onHandleSubmit = async (values: ConnectorFormValues) => {
if (!selectedConnectorDefinition) {
return;
Expand Down Expand Up @@ -215,16 +227,28 @@ export const ConnectorCard: React.FC<ConnectorCardCreateProps | ConnectorCardEdi
isTestConnectionInProgress={isTestConnectionInProgress}
connectionTestSuccess={connectionTestSuccess}
onStopTesting={onStopTesting}
testConnector={testConnector}
testConnector={handleTestConnector}
onSubmit={onHandleSubmit}
formValues={formValues}
errorMessage={error && generateMessageFromError(error)}
successMessage={saved && props.isEditMode && <FormattedMessage id="form.changesSaved" />}
connectorId={isEditMode ? getConnectorId(props.connector) : undefined}
/>
)}
{/* Show the job log only if advanced mode is turned on or the actual job failed (not the check inside the job) */}
{job && (advancedMode || !job.succeeded) && <JobItem job={job} />}
{job && (
<div className={styles.connectionTestLogs}>
<Button
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels like it may make more sense to use an expandable section here rather than a Button, similar to how the BuilderOptional component looks.

Since this button is just showing/hiding additional info, using the faAngleRight / faAngleDown icon next to some text that just says Connection test logs feels like a more natural UX. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, changed the button to look like this:
Screenshot 2023-01-06 at 13 32 25
Screenshot 2023-01-06 at 13 32 28

variant="clear"
icon={<FontAwesomeIcon icon={logsVisible ? faChevronDown : faChevronRight} />}
onClick={() => {
setLogsVisible(!logsVisible);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
setLogsVisible(!logsVisible);
setLogsVisible((prevLogsVisible) => !prevLogsVisible);

}}
>
<FormattedMessage id="connector.testLogs" />
</Button>
{logsVisible && <JobLogs job={job} jobIsFailed />}
</div>
)}
</div>
</div>
</Card>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
@use "scss/variables";

.controlsContainer {
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: variables.$spacing-lg;
}

.buttonsContainer {
display: flex;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from "react";
import { FormattedMessage } from "react-intl";
import styled from "styled-components";

import { Button } from "components/ui/Button";

Expand All @@ -9,13 +8,6 @@ import { TestingConnectionError } from "./TestingConnectionError";
import { TestingConnectionSpinner } from "./TestingConnectionSpinner";
import TestingConnectionSuccess from "./TestingConnectionSuccess";

const Controls = styled.div`
margin-top: 34px;
display: flex;
justify-content: space-between;
align-items: center;
`;

interface IProps {
formType: "source" | "destination";
isSubmitting: boolean;
Expand Down Expand Up @@ -57,8 +49,7 @@ const EditControls: React.FC<IProps> = ({

return (
<>
{renderStatusMessage()}
<Controls>
<div className={styles.controlsContainer}>
<div className={styles.buttonsContainer}>
<Button type="submit" disabled={isSubmitting || !dirty}>
<FormattedMessage id="form.saveChangesAndTest" />
Expand All @@ -78,7 +69,8 @@ const EditControls: React.FC<IProps> = ({
<FormattedMessage id={`form.${formType}Retest`} />
</Button>
)}
</Controls>
</div>
{renderStatusMessage()}
</>
);
};
Expand Down