Add TLS Cert input #1784
Add TLS Cert input #1784
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| tabIndex={0} | ||
| aria-rowindex={index + 1} | ||
| aria-label={`Name: ${item.name}, Description: ${item.description}`} | ||
| key={item.name} |
There was a problem hiding this comment.
Currently we aren't checking for uniqueness around name. Do we want to do so? If not, we should use something like ${item.name}-${index} to ensure the keys do not clash.
There was a problem hiding this comment.
Yeah, we probably should enforce uniqueness. I guess we’d check that on submit in the modal? Refuse to submit if there’s a duplicate.
|
Validated working against dogfood earlier today. |
| return ( | ||
| <TextField | ||
| validate={(name) => | ||
| allNames.includes(name) ? 'Certificate with this name already exists' : true |
There was a problem hiding this comment.
I did attempt to get this going in the onSubmit of AddCertModal but for whatever reason nothing I put in there was firing. Guessing react-hook-form is intercepting the default onSubmit.
But the benefit of this approach is we don't need to mess around with setError or anything like that. I did consider memoizing allNames but figured it might be overboard for the number of certs we'll have here.
Also the key is guaranteed to be unique because if a name is not, it will never be added.
libs/ui/lib/file-input/FileInput.tsx
Outdated
| /* Validating this onBlur causes the required | ||
| error message to appear on click. Instead we can | ||
| validate on form submit by overriding onBlur */ | ||
| onBlur={() => {}} |
There was a problem hiding this comment.
Any thoughts on this @david-crespo? We might want to validate onChange if we have anything like validating filenames or sizes – but it might give you an error when you clear the file, which we may or may not want. Presently we are only really validating required or not.
This fixes the issue where the error appears as soon as you click on the file input.
There was a problem hiding this comment.
As discussed in chat, I think validate on blur only if dirty (#1289) would be our ideal behavior, though I don't think RHF supports it yet. I will look into patching or PRing it so we can test that out. On the other hand, I don't think that would solve #1786 because you could still trigger by typing something invalid in the name field and then trying to change the radio input.
There was a problem hiding this comment.
Yeah – though this specifically is targeting the error appearing when you click on the file input. It seems like the onBlur is triggered whenever the file input opens.
|
Merging main with #1787 fixed the issues with the file input error on blur, plus the one (same as #1786) where if you scroll down and click the add cert button without typing in the name field, you'd get a validation error on name due to the blur that prevents your click on the button from going through (probably because it gets scrolled out of the way, ugh). I noticed name was not being validated as a required field, fixing that. |
| @@ -53,7 +53,6 @@ export interface TextFieldProps< | |||
| description?: string | |||
| placeholder?: string | |||
| units?: string | |||
| // TODO: think about this doozy of a type | |||
There was a problem hiding this comment.
one year of thinking is enough
| } else if (!/^[a-z0-9-]+$/.test(value)) { | ||
| return 'Can only contain lower-case letters, numbers, and dashes' | ||
| } | ||
| } |
There was a problem hiding this comment.
this wasn't used. apparently a leftover from when we switched from Formik to RHF
There was a problem hiding this comment.
This looks good to me. I want to give the code and the E2Es another once-over later and then merge.
Something we should follow up on: displaying the certs on the silo detail page where we currently list IdPs. I'm a little confused about the API for this, will need to look into it. I don't see a way to list these certs.
… getByRole where possible
oxidecomputer/console@0cc1e03...9940fd0 * [9940fd0d](oxidecomputer/console@9940fd0d) oxidecomputer/console#1790 * [d07ca142](oxidecomputer/console@d07ca142) oxidecomputer/console#1784 * [0a32ccf8](oxidecomputer/console@0a32ccf8) oxidecomputer/console#1787 * [e74d5aec](oxidecomputer/console@e74d5aec) oxidecomputer/console#1772 * [a68c7c86](oxidecomputer/console@a68c7c86) oxidecomputer/console#1779 * [b2b3a748](oxidecomputer/console@b2b3a748) oxidecomputer/console#1774 * [0f015505](oxidecomputer/console@0f015505) bump playwright for UI mode improvements * [1ccff371](oxidecomputer/console@1ccff371) oxidecomputer/console#1582
oxidecomputer/console@0cc1e03...9940fd0 * [9940fd0d](oxidecomputer/console@9940fd0d) oxidecomputer/console#1790 * [d07ca142](oxidecomputer/console@d07ca142) oxidecomputer/console#1784 * [0a32ccf8](oxidecomputer/console@0a32ccf8) oxidecomputer/console#1787 * [e74d5aec](oxidecomputer/console@e74d5aec) oxidecomputer/console#1772 * [a68c7c86](oxidecomputer/console@a68c7c86) oxidecomputer/console#1779 * [b2b3a748](oxidecomputer/console@b2b3a748) oxidecomputer/console#1774 * [0f015505](oxidecomputer/console@0f015505) bump playwright for UI mode improvements * [1ccff371](oxidecomputer/console@1ccff371) oxidecomputer/console#1582
Fixes #1736
Uses the mini table and modal create pattern to add TLS certificates. I've omitted
serviceas there's only one option currentlyexternal_api.As specced in the API,
keyandcertare plain text — addedreadBlobAsTextto read that from the File object.Running into a couple of nits around the required validation. If you try to click cancel after opening the modal, the first click will be interrupted by the error on the "Name" field. Similarly, when you click on the file upload you immediately get the required error message. Should probably handle in a separate PR.