feat(connections): add general tab contents to connect form COMPASS-5231 #2620
Conversation
packages/connect-form/src/components/advanced-options-tabs/general/schema-input.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
In the screen capture it looks like directConnection unsets when moving back and forth to another connection, couldn't really find the cause, but could be related to the fact that one connection is SRV and the other is not.
Is there a way to make the state completely independent between connections?
packages/connect-form/src/components/advanced-options-tabs/general/schema-input.tsx
Outdated
Show resolved
Hide resolved
packages/connect-form/src/components/advanced-connection-options.tsx
Outdated
Show resolved
Hide resolved
packages/connect-form/src/components/advanced-options-tabs/general-tab.spec.tsx
Outdated
Show resolved
Hide resolved
packages/connect-form/src/components/advanced-options-tabs/general/schema-input.tsx
Outdated
Show resolved
Hide resolved
packages/connect-form/src/components/advanced-options-tabs/general/schema-input.tsx
Show resolved
Hide resolved
…invalid to set, set directConnection true on default, use color variable
There was a problem hiding this comment.
Overall it looks good to me, I really like using this new form and the fact that it now more or less just maps fields directly to driver options makes so much sense to me. It's also cool to see the string change when the fields are updated and vice versa. I did stumble on a few issues that I mentioned in the comments. Also left a few suggestions here and there for things that caught my eye, but they are totally not that important if you don't feel like accepting them.
packages/connect-form/src/components/advanced-options-tabs/general/direct-connection-input.tsx
Outdated
Show resolved
Hide resolved
packages/connect-form/src/components/advanced-options-tabs/general/host-input.tsx
Outdated
Show resolved
Hide resolved
packages/connect-form/src/components/advanced-options-tabs/general/host-input.tsx
Outdated
Show resolved
Hide resolved
packages/connect-form/src/components/connection-string-input.tsx
Outdated
Show resolved
Hide resolved
packages/connect-form/src/components/advanced-options-tabs/general/host-input.tsx
Show resolved
Hide resolved
| <Button variant={ButtonVariant.Primary} onClick={onConnectClicked}> | ||
| Connect | ||
| </Button> |
There was a problem hiding this comment.
Should this button be disabled / hidden when the URL is not valid? Currently it is clickable and when pressed you are being connected to the last valid url that was in the input
There was a problem hiding this comment.
Yes, but I think it probably makes sense to do that in a separate pr if that's cool with you. We can add some tests too. (This diff here is because we moved the original file into the components folder)
Anemy
deleted the
COMPASS-5231-add-general-tab-contents-to-connect-form
branch
3 participants
COMPASS-5231
This PR adds the general tab to the form. Since this PR has some of the first state related items for this form, we can look at how the state in the rest of the form will work. Right now a lot of the state updating happens near the components where they'll send up a completed connection string when they can successfully be updated which updates the rest of the form.
We could move around where some of these things happen now so if y'all have any ideas or thoughts please share. I'll leave a comment or two in places where these things happen. We could also break this pr down into a couple smaller ones if it helps for reviewing.
general.tab.new.connect.mp4