Initilalize the validation value of the 'plan name' field to default instead of error #1421
Conversation
…o default instead of error Reference: https://issues.redhat.com/browse/MTV-1651 To avoid displaying an error on initialization for the 'plan name'empty field, when entering the wizard's step 2, leave the initialized validation value for this field as 'default. Signed-off-by: Sharon Gratch <[email protected]>
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1421 +/- ##
==========================================
- Coverage 36.81% 36.21% -0.60%
==========================================
Files 158 159 +1
Lines 2548 2579 +31
Branches 599 615 +16
==========================================
- Hits 938 934 -4
- Misses 1428 1463 +35
Partials 182 182 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Hi @sgratch
left one nitpick suggestion for you, and I have a question
I saw in the description the screen recordings, both in before and after when clicking next in the wizard you had to scroll up to view the field? is that because you're zoomed in or we have a focus issue here?
| Object.values(state?.validation || []).some((validation) => validation === 'error') || | ||
| state?.validation?.planName === 'default' |
There was a problem hiding this comment.
looking at this file, I see a lot of hard-coded values repeating, a nice nitpick would be starting to use constants in a separated constants.ts file as you did for title, it would help clean this file a bit
There was a problem hiding this comment.
@avivtur Yes, this was a code convention used up till now - using hard-coded string values in most cases.
We can definitely refactor it as was done already in other places in code, but this will require refactoring the Validation type to use enum. Since this Validation type has many references, I prefer taking it out to a follow up PR. WDYT?
There was a problem hiding this comment.
I updated the refactoring task ticket: https://issues.redhat.com/browse/MTV-1728
There was a problem hiding this comment.
A seprated PR is always a good idea :)
@avivtur This is a bug and thanks for raising it.
|
|
/lgtm |
sgratch
deleted the
set-plan-name-field-init-validation-value-to-default
branch
Reference: https://issues.redhat.com/browse/MTV-1651
To avoid displaying an error on initialization for the 'plan name' empty field, when entering the wizard's step 2, leave the initialized validation value for this field as 'default.
Screncast before fix
Screencast.from.2024-12-27.01-36-08.webm
Screncast after fix
Screencast.from.2024-12-27.01-44-19.webm