🪟🔧 Remove connector form name override #20132
Conversation
…or-form-loading-state
flash1293
added
area/frontend
team/extensibility
area/frontend
…or-form-name-override
flash1293
changed the base branch from
master
to
flash1293/connector-form-loading-state
flash1293
changed the base branch from
flash1293/connector-form-loading-state
to
master
….module.scss Co-authored-by: Tim Roes <tim@airbyte.io>
…or-form-loading-state
…irbytehq/airbyte into flash1293/connector-form-loading-state
…connector-form-name-override
…or-form-loading-state
…connector-form-name-override
…or-form-loading-state
…connector-form-name-override
…-form-loading-state
…connector-form-name-override
…-form-name-override
| const jsonSchema: JSONSchema7 = useMemo( | ||
| () => ({ | ||
| type: "object", | ||
| properties: { | ||
| name: { | ||
| type: "string", | ||
| title: formatMessage({ id: `form.${formType}Name` }), | ||
| description: formatMessage({ id: `form.${formType}Name.message` }), | ||
| }, | ||
| connectionConfiguration: | ||
| selectedConnectorDefinitionSpecification.connectionSpecification as JSONSchema7Definition, | ||
| }, | ||
| required: ["name"], | ||
| }), | ||
| [formType, formatMessage, selectedConnectorDefinitionSpecification.connectionSpecification] | ||
| ); |
There was a problem hiding this comment.
One potential future issue with putting this in the useBuildForm hook is that when we want to use the connector form in the Connector Builder for the config menu, we don't want to have this "connector name" field in there.
Can definitely be addressed in a later PR but I wanted to call it out since I thought of it
There was a problem hiding this comment.
A couple of small suggestions, but overall this looks like a good incremental improvement to me.
I don't think I'll need to take another look after you address my comments bc they are pretty straightforward, so approving now.
Tested locally and didn't notice any regressions from master, though I did find a bug in master which I captured in an issue here: #20332
|
|
||
| // As schema is dynamic, it is possible, that new updated values, will differ from one stored. | ||
| const mergedState = useMemo( | ||
| () => | ||
| merge( | ||
| buildPathInitialState(Array.isArray(formFields) ? formFields : [formFields], formValues), | ||
| merge(overriddenWidgetState, uiOverrides) | ||
| merge(overriddenWidgetState) |
There was a problem hiding this comment.
| merge(overriddenWidgetState) | |
| overriddenWidgetState |
This merge() call is now unnecessary
| ): BuildUiWidgetsContextHook => { | ||
| const [overriddenWidgetState, setUiWidgetsInfo] = useState<WidgetConfigMap>(uiOverrides ?? {}); | ||
| const [overriddenWidgetState, setUiWidgetsInfo] = useState<WidgetConfigMap>({}); |
There was a problem hiding this comment.
Since there is no more concept of "ui overrides" here, I think we can just name this something like uiWidgetsInfo instead of overriddenWidgetState
There was a problem hiding this comment.
I will take care of this in a follow-up PR
Part of #14250
Based on #20127 (wait until merged)
What
This PR removes the special component for the connector name and replaces it with regular schema definition. It also moves this logic into the
useBuildFormhook.How
As the regular form builder can handle name fields just fine, it's simple to do the name and description resolution in the schema modification instead of handling this via a special component. This also allows to remove the "uiOverride" concept from the uiWidget state. In a follow-up PR this will be removed completely.
🚨 User Impact 🚨
No user-visible changes