fix: [M3-8588] - Fix value selection issue in ImageSelect
#11007
Conversation
carrillo-erik
requested review from
bnussman-akamai and
hana-akamai
and removed request for
a team
|
Coverage Report: ✅ |
Co-authored-by: Hana Xu <115299789+hana-linode@users.noreply.github.com>
…into fix/M3-8588
| @@ -518,6 +518,6 @@ export const UpdateLinodeDiskSchema = object({ | |||
|
|
|||
| export const CreateLinodeDiskFromImageSchema = CreateLinodeDiskSchema.clone().shape( | |||
| { | |||
| image: string().required('An image is required.'), | |||
| image: string().required('An image is required.').nullable(), | |||
There was a problem hiding this comment.
This change is done to remove the validation error message after a user has selected a value from the ImageSelect and then clears the value (input).
There was a problem hiding this comment.
Yup's .typeError might be more fitting here than nullable
There was a problem hiding this comment.
I wonder if we could use ImageSelectv2 here instead. This ImageSelect has some serious type safety issues and hackyness
There was a problem hiding this comment.
@bnussman-akamai I agree with your concerns about the type safety issues and the hackyness feeling. I gave ImageSelectv2 a try and it appears that with some minor changes it should work within the CreateDiskDrawer.
The one issue I noticed is the way the options are grouped. In the current ImageSelect, one the three categories is the recommended 64-bit distributions. In the ImageSelectv2, they are grouped by vendor. Would require some conditional logic to determine with grouping function to use.
ImageSelect
ImageSelectv2
There was a problem hiding this comment.
Ah good point. That would require some UX / product consideration
There was a problem hiding this comment.
@bnussman-akamai To keep the work within the scope of this ticket, I'll create a separate ticket for UX to consider the grouping logic and see if it would make sense to further replace the original ImageSelect with the ImageSelectv2 component. Could please review and approve this for now. Thanks.
| @@ -518,6 +518,6 @@ export const UpdateLinodeDiskSchema = object({ | |||
|
|
|||
| export const CreateLinodeDiskFromImageSchema = CreateLinodeDiskSchema.clone().shape( | |||
| { | |||
| image: string().required('An image is required.'), | |||
| image: string().required('An image is required.').nullable(), | |||
There was a problem hiding this comment.
Yup's .typeError might be more fitting here than nullable
bnussman-akamai
added
Approved
Cloud Manager E2E
|
||||||||||||||||||||||||||||
| Project |
Cloud Manager E2E
|
| Branch Review |
develop
|
| Run status |
|
| Run duration | 23m 43s |
| Commit |
|
| Committer | carrillo-erik |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
2
|
Skipped
|
0
|
Passing
|
408
|
| View all changes introduced in this branch ↗︎ | |
Description 📝
This PR fixes the bug in the
ImageSelectcomponent which resulted in the selectedvaluegetting cleared after the input lost focus.Changes 🔄
List any change relevant to the reviewer.
valueisundefinedinitially.default exportdeclarations.Target release date 🗓️
10/16/2024
Preview 📷
BEFORE.mov
AFTER.mov
How to test 🧪
Prerequisites
(How to setup test environment)
Storagetab for your chosen Linode.Disksin order to enable theAdd A Diskoption.Add A Diskbutton to open theCreate Diskdrawer.Create from Imageradio option.Verification steps
(How to verify changes)
Imagedropdown menu.Label,Root Password,Size).Imagedropdown component remains after you have focused out of the component and focused into another field in the drawer.Add A Diskfeature works as expected.As an Author I have considered 🤔
Check all that apply