-
Notifications
You must be signed in to change notification settings - Fork 236
feat(connect-form): Add tls tab form options COMPASS-5232 #2687
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
Conversation
packages/connect-form/src/components/advanced-options-tabs/general-tab/host-input.tsx
Show resolved
Hide resolved
{label} | ||
</span> | ||
</div> | ||
{optional && ( |
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.
suggestion: in the leafygreen UI, the optional text is always right next in the field and not label. should we align that?
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.
@Sgrinfy what do you think? Should we make this a follow up too?
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.
I'm looking at the optional field for "Client Key Password", and the optional label is in the field like it should be. @mabaasit what should we align?
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.
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.
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.
exactly. Its used in import to/export from collection modals as well.
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.
Is that recent screenshot what it should be updated to @Sgrinfy ?
I'd like to merge this and update the styles in a follow up I'm thinking
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.
@Anemy No, let's leave it like it is currently implemented. This select file component it's used in other places too in Compass and I don't want to have slightly different styles. We can revaluate in future how to improve it.
COMPASS-5232
Adds TLS options to the new connect form.
Updates the
FileInput
compass-components component to allow showing files on following lines and removing files (see video).Some decisions we can discuss/improve/change:
tls
options. If the user wants to use deprecatedssl
options it's still possible in the connection string but we don't currently reflect it in the form..pem
input, instead of 2 for a.crt
and.key
ssl
equivalents the defaults or exposed?env USE_NEW_CONNECT_FORM="true" npm start
Here's how it looks, and connecting to the devtools docker tls env:
options.1.mp4
editing.tls.options.mp4
Videos not totally up to date - the width changing glitch when switching tabs is fixed in a recent commit.