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

🪟🔧 Remove top level state for unfinished flows in connector form #20135

Merged
merged 33 commits into from
Dec 13, 2022
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
807c436
refactor loading state
Dec 6, 2022
6c76e0f
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 6, 2022
05c3107
move loading state up
Dec 6, 2022
eee4303
remove isLoading references
Dec 6, 2022
7d6f977
remove unused props and make fetch connector error work
Dec 6, 2022
0fe11f0
remove special component for name
Dec 6, 2022
34d4814
remove top level state for unifinished flows
Dec 6, 2022
67d62c5
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 6, 2022
afea882
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 6, 2022
c7e7475
Merge branch 'master' into flash1293/connector-form-loading-state
Dec 6, 2022
9be8337
Update airbyte-webapp/src/views/Connector/ConnectorCard/ConnectorCard…
Dec 6, 2022
37cee8f
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 6, 2022
5686241
Merge branch 'flash1293/connector-form-loading-state' of github.com:a…
Dec 6, 2022
7b73364
remove undefined option for selected id
Dec 6, 2022
7058d26
Merge branch 'flash1293/connector-form-loading-state' into flash1293/…
Dec 6, 2022
dd7e82a
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 6, 2022
6d4c7d3
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 7, 2022
d624792
Merge branch 'flash1293/connector-form-loading-state' into flash1293/…
Dec 7, 2022
36cf941
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 7, 2022
b7a6322
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 7, 2022
72b602a
Merge branch 'flash1293/connector-form-loading-state' into flash1293/…
Dec 7, 2022
5e47a4a
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 7, 2022
6c71028
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 8, 2022
809625f
remove unused prop
Dec 8, 2022
10446eb
Merge branch 'flash1293/connector-form-loading-state' into flash1293/…
Dec 8, 2022
4a7a885
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 8, 2022
93fe558
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 9, 2022
cbc023a
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 9, 2022
9aa3f80
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 12, 2022
5cd77ec
pass error to secure inputs
Dec 12, 2022
5dfbc22
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 13, 2022
a6aa741
simplify and improve styling
Dec 13, 2022
95717d2
align top
Dec 13, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import styled from "styled-components";

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

import { useConnectorForm } from "../connectorFormContext";
import styles from "./EditControls.module.scss";
import { TestingConnectionError } from "./TestingConnectionError";
import { TestingConnectionSpinner } from "./TestingConnectionSpinner";
Expand Down Expand Up @@ -42,8 +41,6 @@ const EditControls: React.FC<IProps> = ({
errorMessage,
onCancelTesting,
}) => {
const { unfinishedFlows } = useConnectorForm();

if (isSubmitting) {
return <TestingConnectionSpinner isCancellable={isTestConnectionInProgress} onCancelTesting={onCancelTesting} />;
}
Expand All @@ -63,7 +60,7 @@ const EditControls: React.FC<IProps> = ({
{renderStatusMessage()}
<Controls>
<div className={styles.buttonsContainer}>
<Button type="submit" disabled={isSubmitting || !dirty || Object.keys(unfinishedFlows).length > 0}>
<Button type="submit" disabled={isSubmitting || !dirty}>
<FormattedMessage id="form.saveChangesAndTest" />
</Button>
<Button
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,23 @@ import { DatePicker } from "components/ui/DatePicker";
import { DropDown } from "components/ui/DropDown";
import { Input } from "components/ui/Input";
import { Multiselect } from "components/ui/Multiselect";
import { SecretTextArea } from "components/ui/SecretTextArea";
import { TagInput } from "components/ui/TagInput/TagInput";
import { TextArea } from "components/ui/TextArea";

import { FormBaseItem } from "core/form/types";
import { useExperiment } from "hooks/services/Experiment";
import { isDefined } from "utils/common";

import ConfirmationControl from "./ConfirmationControl";
import SecretConfirmationControl from "./SecretConfirmationControl";

interface ControlProps {
property: FormBaseItem;
name: string;
unfinishedFlows: Record<string, { startValue: string }>;
addUnfinishedFlow: (key: string, info?: Record<string, unknown>) => void;
removeUnfinishedFlow: (key: string) => void;
disabled?: boolean;
error?: boolean;
}

export const Control: React.FC<ControlProps> = ({
property,
name,
addUnfinishedFlow,
removeUnfinishedFlow,
unfinishedFlows,
disabled,
error,
}) => {
export const Control: React.FC<ControlProps> = ({ property, name, disabled, error }) => {
const [field, meta, helpers] = useField(name);
const useDatepickerExperiment = useExperiment("connector.form.useDatepicker", true);

Expand Down Expand Up @@ -107,46 +95,14 @@ export const Control: React.FC<ControlProps> = ({
} else if (property.multiline && !property.isSecret) {
return <TextArea {...field} autoComplete="off" value={value ?? ""} rows={3} disabled={disabled} error={error} />;
} else if (property.isSecret) {
const unfinishedSecret = unfinishedFlows[name];
const isEditInProgress = !!unfinishedSecret;
const isFormInEditMode = isDefined(meta.initialValue);
return (
<ConfirmationControl
component={
property.multiline && (isEditInProgress || !isFormInEditMode) ? (
<SecretTextArea
{...field}
autoComplete="off"
value={value ?? ""}
rows={3}
disabled={disabled}
error={error}
/>
) : (
<Input
{...field}
autoComplete="off"
value={value ?? ""}
type="password"
disabled={disabled}
error={error}
/>
)
}
<SecretConfirmationControl
name={name}
multiline={Boolean(property.multiline)}
showButtons={isFormInEditMode}
isEditInProgress={isEditInProgress}
onDone={() => removeUnfinishedFlow(name)}
onStart={() => {
addUnfinishedFlow(name, { startValue: field.value });
helpers.setValue("");
}}
onCancel={() => {
removeUnfinishedFlow(name);
if (unfinishedSecret && unfinishedSecret.hasOwnProperty("startValue")) {
helpers.setValue(unfinishedSecret.startValue);
}
}}
disabled={disabled}
error={error}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { useField, useFormikContext } from "formik";
import React, { useEffect, useRef, useState } from "react";
import { FormattedMessage } from "react-intl";
import styled from "styled-components";

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

const ComponentContainer = styled.div`
display: flex;
flex-direction: row;
align-items: center;
`;

interface SecretConfirmationControlProps {
showButtons?: boolean;
name: string;
multiline: boolean;
disabled?: boolean;
error?: boolean;
}

const SecretConfirmationControl: React.FC<SecretConfirmationControlProps> = ({
showButtons,
disabled,
multiline,
name,
error,
}) => {
const [field, , helpers] = useField(name);
const [previousValue, setPreviousValue] = useState<unknown>(undefined);
const isEditInProgress = Boolean(previousValue);
const controlRef = useRef<HTMLElement>(null);

const { dirty, touched } = useFormikContext();

const component =
multiline && (isEditInProgress || !showButtons) ? (
<SecretTextArea
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed a couple small styling differences with the text area component which I put up a small PR here to fix: #20397

{...field}
autoComplete="off"
value={field.value ?? ""}
rows={3}
disabled={disabled}
error={error}
/>
) : (
<Input
{...field}
autoComplete="off"
value={field.value ?? ""}
type="password"
disabled={disabled}
error={error}
Copy link
Contributor

Choose a reason for hiding this comment

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

Something I noticed while testing: the secret input border is not set to red when it has an error
Screen Shot 2022-12-12 at 11 58 57 AM

I think this is because the error prop is not being passed to this component from Control.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, seems like it's like that on master too. Should be fixed:
Screenshot 2022-12-12 at 21 24 38

/>
);

useEffect(() => {
if (!dirty && !touched && previousValue) {
setPreviousValue(undefined);
}
}, [dirty, helpers, previousValue, touched]);
Comment on lines +57 to +61
Copy link
Contributor

Choose a reason for hiding this comment

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

Listen to the global formik state - if the dirty flag flips to false but there is a "previous value", reset that as well

Just to confirm my understanding: this code is basically ensuring that if the formik form is reset back to the initial state, then the previousValue state will be cleared out? An in practice, this would happen if a user clicks the Cancel button at the bottom of the ConnectorForm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, if the form-level "Cancel" button is hit, all open secret controls are "closed" as well.


if (!showButtons) {
return <>{component}</>;
}

const handleStartEdit = () => {
if (controlRef && controlRef.current) {
controlRef.current?.removeAttribute?.("disabled");
controlRef.current?.focus?.();
}
setPreviousValue(field.value);
helpers.setValue("");
};

const onDone = () => {
setPreviousValue(undefined);
};

const onCancel = () => {
if (previousValue) {
helpers.setValue(previousValue);
}
setPreviousValue(undefined);
};

return (
<ComponentContainer>
{React.cloneElement(component, {
ref: controlRef,
autoFocus: isEditInProgress,
disabled: !isEditInProgress || disabled,
})}
{isEditInProgress ? (
<>
<Button size="xs" onClick={onDone} type="button" disabled={disabled}>
<FormattedMessage id="form.done" />
</Button>
<Button size="xs" onClick={onCancel} type="button" variant="secondary" disabled={disabled}>
<FormattedMessage id="form.cancel" />
</Button>
</>
) : (
<Button size="xs" onClick={handleStartEdit} type="button" disabled={disabled}>
<FormattedMessage id="form.edit" />
</Button>
)}
</ComponentContainer>
);
};

export default SecretConfirmationControl;
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const PropertySection: React.FC<PropertySectionProps> = ({ property, path, disab
const propertyPath = path ?? property.path;
const formikBag = useField(propertyPath);
const [field, meta] = formikBag;
const { addUnfinishedFlow, removeUnfinishedFlow, unfinishedFlows, widgetsInfo } = useConnectorForm();
const { widgetsInfo } = useConnectorForm();

const overriddenComponent = widgetsInfo[propertyPath]?.component;
if (overriddenComponent) {
Expand Down Expand Up @@ -59,15 +59,7 @@ const PropertySection: React.FC<PropertySectionProps> = ({ property, path, disab

return (
<PropertyLabel className={styles.defaultLabel} property={property} label={labelText}>
<Control
property={property}
name={propertyPath}
addUnfinishedFlow={addUnfinishedFlow}
removeUnfinishedFlow={removeUnfinishedFlow}
unfinishedFlows={unfinishedFlows}
disabled={disabled}
error={hasError}
/>
<Control property={property} name={propertyPath} disabled={disabled} error={hasError} />
{hasError && <PropertyError>{errorMessage}</PropertyError>}
</PropertyLabel>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ interface ConnectorFormContext {
getValues: <T = unknown>(values: ConnectorFormValues<T>) => ConnectorFormValues<T>;
widgetsInfo: WidgetConfigMap;
setUiWidgetsInfo: (path: string, value: Record<string, unknown>) => void;
unfinishedFlows: Record<string, { startValue: string; id: number | string }>;
addUnfinishedFlow: (key: string, info?: Record<string, unknown>) => void;
removeUnfinishedFlow: (key: string) => void;
resetConnectorForm: () => void;
selectedConnectorDefinition: ConnectorDefinition;
selectedConnectorDefinitionSpecification: ConnectorDefinitionSpecification;
Expand Down Expand Up @@ -62,7 +59,6 @@ export const ConnectorFormContextProvider: React.FC<React.PropsWithChildren<Conn
const { resetForm } = useFormikContext<ConnectorFormValues>();

const ctx = useMemo<ConnectorFormContext>(() => {
const unfinishedFlows = widgetsInfo["_common.unfinishedFlows"] ?? {};
const context: ConnectorFormContext = {
widgetsInfo,
getValues,
Expand All @@ -73,17 +69,6 @@ export const ConnectorFormContextProvider: React.FC<React.PropsWithChildren<Conn
validationSchema,
isEditMode,
connectorId,
unfinishedFlows,
addUnfinishedFlow: (path, info) =>
setUiWidgetsInfo("_common.unfinishedFlows", {
...unfinishedFlows,
[path]: info ?? {},
}),
removeUnfinishedFlow: (path: string) =>
setUiWidgetsInfo(
"_common.unfinishedFlows",
Object.fromEntries(Object.entries(unfinishedFlows).filter(([key]) => key !== path))
),
resetConnectorForm: () => {
resetForm();
resetUiWidgetsInfo();
Expand Down