-
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
🪟🧹 Connector builder: Fast fields performance improvements #20957
Changes from all commits
d2ed762
f474a1c
5b0521f
af659fd
1cd3bbb
807c436
6c76e0f
05c3107
eee4303
7d6f977
0fe11f0
34d4814
67d62c5
afea882
c7e7475
cc7f65f
a1c6cbd
9be8337
37cee8f
5686241
7b73364
7058d26
dd7e82a
6d4c7d3
d624792
36cf941
b7a6322
72b602a
5e47a4a
6c71028
809625f
10446eb
4a7a885
2024b60
ee30092
d91df38
93fe558
cbc023a
ec54d38
47152ea
6ec0e10
1aa7829
7b90088
fbefbdd
6b3e5e2
9aa3f80
c765003
5cd77ec
5dfbc22
a6aa741
95717d2
40d162d
b9384ee
582a5f2
18d6ce8
60c1f1c
9cb595d
fe74a45
5c00ad0
bd0dffc
1ff8c13
c6ee526
05bbd6f
fff4703
94ba866
97bb0a7
633b9b2
e47517f
e931956
219fcc1
8cb6873
6889f66
7b60406
d5e56eb
35a74ba
02c8096
526adfc
7d49233
c5da764
6c0bf9d
477977f
200cfe7
c7fd310
e0e3796
e23a6d8
610e668
e68ff49
9ccf294
ae9a229
a9e1d09
e4f7bba
16c71ae
54c070d
2e20fa6
65199d0
6b71161
81c12b2
b447b24
92722e0
ef86690
718c233
26acc32
6a8be0b
e166b03
4b4eb7e
a4d35be
54d744b
f5818e6
5dc707b
742538e
b2b03bf
4ce7811
0d5fcc5
98a33cf
7e08ea7
80ec5c6
8c33c36
f996b6d
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 |
---|---|---|
|
@@ -2,6 +2,7 @@ import { faPlus, faUser } from "@fortawesome/free-solid-svg-icons"; | |
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; | ||
import { useField } from "formik"; | ||
import { useMemo, useState } from "react"; | ||
import React from "react"; | ||
import { FormattedMessage, useIntl } from "react-intl"; | ||
|
||
import { ListBox, ListBoxControlButtonProps, Option } from "components/ui/ListBox"; | ||
|
@@ -26,20 +27,18 @@ export const BuilderFieldWithInputs: React.FC<BuilderFieldProps> = (props) => { | |
); | ||
}; | ||
|
||
export const UserInputHelper = ({ | ||
setValue, | ||
currentValue, | ||
}: { | ||
interface UserInputHelperProps { | ||
setValue: (value: string) => void; | ||
currentValue: string; | ||
}) => { | ||
} | ||
|
||
export const UserInputHelper = (props: UserInputHelperProps) => { | ||
const { formatMessage } = useIntl(); | ||
const [modalOpen, setModalOpen] = useState(false); | ||
const { builderFormValues } = useConnectorBuilderFormState(); | ||
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. We can't get rid of subscribing to the builder form state, but we can memoize building the actual options list and memoize the actual listbox component based on that. |
||
const listOptions = useMemo(() => { | ||
const options: Array<Option<string | undefined>> = [ | ||
...builderFormValues.inputs, | ||
...getInferredInputs(builderFormValues), | ||
...getInferredInputs(builderFormValues.global, builderFormValues.inferredInputOverrides), | ||
].map((input) => ({ | ||
label: input.definition.title || input.key, | ||
value: input.key, | ||
|
@@ -50,45 +49,56 @@ export const UserInputHelper = ({ | |
icon: <FontAwesomeIcon icon={faPlus} />, | ||
}); | ||
return options; | ||
}, [builderFormValues, formatMessage]); | ||
return ( | ||
<> | ||
<ListBox<string | undefined> | ||
buttonClassName={styles.button} | ||
optionClassName={styles.option} | ||
className={styles.container} | ||
selectedOptionClassName={styles.selectedOption} | ||
controlButton={UserInputHelperControlButton} | ||
selectedValue={undefined} | ||
onSelect={(selectedValue) => { | ||
if (selectedValue) { | ||
setValue(`${currentValue || ""}{{ config['${selectedValue}'] }}`); | ||
} else { | ||
// This hack is necessary because listbox will put the focus back when the option list gets hidden, which conflicts with the auto-focus setting of the modal. | ||
// As it's not possible to prevent listbox from forcing the focus back on the button component, this will wait until the focus went to the button, then opens the modal | ||
// so it can move it to the first input | ||
setTimeout(() => { | ||
setModalOpen(true); | ||
}, 50); | ||
} | ||
}} | ||
options={listOptions} | ||
/> | ||
{modalOpen && ( | ||
<InputForm | ||
inputInEditing={newInputInEditing()} | ||
onClose={(newInput) => { | ||
setModalOpen(false); | ||
if (!newInput) { | ||
return; | ||
}, [builderFormValues.global, builderFormValues.inferredInputOverrides, builderFormValues.inputs, formatMessage]); | ||
return <InnerUserInputHelper {...props} listOptions={listOptions} />; | ||
}; | ||
|
||
const InnerUserInputHelper = React.memo( | ||
({ | ||
setValue, | ||
currentValue, | ||
listOptions, | ||
}: UserInputHelperProps & { listOptions: Array<Option<string | undefined>> }) => { | ||
const [modalOpen, setModalOpen] = useState(false); | ||
return ( | ||
<> | ||
<ListBox<string | undefined> | ||
buttonClassName={styles.button} | ||
optionClassName={styles.option} | ||
className={styles.container} | ||
selectedOptionClassName={styles.selectedOption} | ||
controlButton={UserInputHelperControlButton} | ||
selectedValue={undefined} | ||
onSelect={(selectedValue) => { | ||
if (selectedValue) { | ||
setValue(`${currentValue || ""}{{ config['${selectedValue}'] }}`); | ||
} else { | ||
// This hack is necessary because listbox will put the focus back when the option list gets hidden, which conflicts with the auto-focus setting of the modal. | ||
// As it's not possible to prevent listbox from forcing the focus back on the button component, this will wait until the focus went to the button, then opens the modal | ||
// so it can move it to the first input | ||
setTimeout(() => { | ||
setModalOpen(true); | ||
}, 50); | ||
} | ||
setValue(`${currentValue}{{ config['${newInput.key}'] }}`); | ||
}} | ||
options={listOptions} | ||
/> | ||
)} | ||
</> | ||
); | ||
}; | ||
{modalOpen && ( | ||
<InputForm | ||
inputInEditing={newInputInEditing()} | ||
onClose={(newInput) => { | ||
setModalOpen(false); | ||
if (!newInput) { | ||
return; | ||
} | ||
setValue(`${currentValue}{{ config['${newInput.key}'] }}`); | ||
}} | ||
/> | ||
)} | ||
</> | ||
); | ||
} | ||
); | ||
|
||
const UserInputHelperControlButton: React.FC<ListBoxControlButtonProps<string | undefined>> = () => { | ||
return ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { useField } from "formik"; | ||
import { FastField, FastFieldProps } from "formik"; | ||
import React from "react"; | ||
|
||
import GroupControls from "components/GroupControls"; | ||
|
@@ -25,10 +25,14 @@ interface BuilderOneOfProps { | |
tooltip: string; | ||
} | ||
|
||
export const BuilderOneOf: React.FC<BuilderOneOfProps> = ({ options, path, label, tooltip }) => { | ||
const [, , oneOfPathHelpers] = useField(path); | ||
const typePath = `${path}.type`; | ||
const [typePathField] = useField(typePath); | ||
const InnerBuilderOneOf: React.FC<BuilderOneOfProps & FastFieldProps<string>> = ({ | ||
options, | ||
label, | ||
tooltip, | ||
field: typePathField, | ||
path, | ||
form, | ||
}) => { | ||
const value = typePathField.value; | ||
|
||
const selectedOption = options.find((option) => option.typeValue === value); | ||
|
@@ -48,7 +52,7 @@ export const BuilderOneOf: React.FC<BuilderOneOfProps> = ({ options, path, label | |
return; | ||
} | ||
// clear all values for this oneOf and set selected option and default values | ||
oneOfPathHelpers.setValue({ | ||
form.setFieldValue(path, { | ||
type: selectedOption.value, | ||
...selectedOption.default, | ||
}); | ||
|
@@ -60,3 +64,10 @@ export const BuilderOneOf: React.FC<BuilderOneOfProps> = ({ options, path, label | |
</GroupControls> | ||
); | ||
}; | ||
export const BuilderOneOf: React.FC<BuilderOneOfProps> = (props) => { | ||
return ( | ||
<FastField name={`${props.path}.type`}> | ||
{(fastFieldProps: FastFieldProps<string>) => <InnerBuilderOneOf {...props} {...fastFieldProps} />} | ||
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. Wrap the oneOf block in its own fastfield 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. My understanding here is that it is okay to have the Is this correct? |
||
</FastField> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,7 +111,9 @@ export const BuilderSidebar: React.FC<BuilderSidebarProps> = React.memo(({ class | |
<FontAwesomeIcon icon={faUser} /> | ||
<FormattedMessage | ||
id="connectorBuilder.userInputs" | ||
values={{ number: values.inputs.length + getInferredInputs(values).length }} | ||
values={{ | ||
number: values.inputs.length + getInferredInputs(values.global, values.inferredInputOverrides).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. Any reason the |
||
}} | ||
/> | ||
</ViewSelectButton> | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import { useField } from "formik"; | ||
import React, { useRef } from "react"; | ||
import { FormattedMessage } from "react-intl"; | ||
|
||
import GroupControls from "components/GroupControls"; | ||
|
@@ -60,29 +61,53 @@ interface KeyValueListFieldProps { | |
export const KeyValueListField: React.FC<KeyValueListFieldProps> = ({ path, label, tooltip }) => { | ||
const [{ value: keyValueList }, , { setValue: setKeyValueList }] = useField<Array<[string, string]>>(path); | ||
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. Can't get rid of the "useField" here, but we can memoize everything happening within the component if the key value list didn't actually change 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. Why can't we use a FastField here? |
||
|
||
// need to wrap the setter into a ref because it will be a new function on every formik state update | ||
const setKeyValueListRef = useRef(setKeyValueList); | ||
setKeyValueListRef.current = setKeyValueList; | ||
|
||
return ( | ||
<GroupControls | ||
label={<ControlLabels label={label} infoTooltipContent={tooltip} />} | ||
control={ | ||
<Button type="button" variant="secondary" onClick={() => setKeyValueList([...keyValueList, ["", ""]])}> | ||
<FormattedMessage id="connectorBuilder.addKeyValue" /> | ||
</Button> | ||
} | ||
> | ||
{keyValueList.map((keyValue, keyValueIndex) => ( | ||
<KeyValueInput | ||
key={keyValueIndex} | ||
keyValue={keyValue} | ||
onChange={(newKeyValue) => { | ||
const updatedList = keyValueList.map((entry, index) => (index === keyValueIndex ? newKeyValue : entry)); | ||
setKeyValueList(updatedList); | ||
}} | ||
onRemove={() => { | ||
const updatedList = keyValueList.filter((_, index) => index !== keyValueIndex); | ||
setKeyValueList(updatedList); | ||
}} | ||
/> | ||
))} | ||
</GroupControls> | ||
<KeyValueList label={label} tooltip={tooltip} keyValueList={keyValueList} setKeyValueList={setKeyValueListRef} /> | ||
); | ||
}; | ||
|
||
const KeyValueList = React.memo( | ||
({ | ||
keyValueList, | ||
setKeyValueList, | ||
label, | ||
tooltip, | ||
}: Omit<KeyValueListFieldProps, "path"> & { | ||
keyValueList: Array<[string, string]>; | ||
setKeyValueList: React.MutableRefObject<(val: Array<[string, string]>) => void>; | ||
}) => { | ||
return ( | ||
<GroupControls | ||
label={<ControlLabels label={label} infoTooltipContent={tooltip} />} | ||
control={ | ||
<Button | ||
type="button" | ||
variant="secondary" | ||
onClick={() => setKeyValueList.current([...keyValueList, ["", ""]])} | ||
> | ||
<FormattedMessage id="connectorBuilder.addKeyValue" /> | ||
</Button> | ||
} | ||
> | ||
{keyValueList.map((keyValue, keyValueIndex) => ( | ||
<KeyValueInput | ||
key={keyValueIndex} | ||
keyValue={keyValue} | ||
onChange={(newKeyValue) => { | ||
const updatedList = keyValueList.map((entry, index) => (index === keyValueIndex ? newKeyValue : entry)); | ||
setKeyValueList.current(updatedList); | ||
}} | ||
onRemove={() => { | ||
const updatedList = keyValueList.filter((_, index) => index !== keyValueIndex); | ||
setKeyValueList.current(updatedList); | ||
}} | ||
/> | ||
))} | ||
</GroupControls> | ||
); | ||
} | ||
); |
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.
Wrap each builder field in a fast field instead of using
useField