-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Allow consumers to override useInputValidation state, use in AddUserEmailForm #16065
Conversation
Codecov Report
@@ Coverage Diff @@
## cloud-emails-redesign-15409 #16065 +/- ##
===============================================================
- Coverage 52.40% 52.39% -0.01%
===============================================================
Files 1636 1636
Lines 81664 81673 +9
Branches 7141 7144 +3
===============================================================
- Hits 42795 42793 -2
- Misses 35048 35058 +10
- Partials 3821 3822 +1
|
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 think it's fine to merge this and you should own the experience and making further changes if you think it would be better (you could file an issue on yourself and include it in our iteration plan)
value: overrideOptions?.value ?? '', | ||
}) | ||
|
||
if (overrideOptions?.value) { |
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.
Does this prevent overriding { value: '', validate: true }
?
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.
Good catch, this should be overrideOptions?.validate
, it doesn't matter what the value is
} | ||
|
||
eventLogger.log('NewUserEmailAddressAdded') | ||
overrideEmailState() |
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.
nit: I think requiring { value: '' }
to be passed would be a bit more readable.
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.
Thank you, TJ!
@felixfbecker I think that while
useInputValidation
's return value is starting to get ugly, most consumers don't need to override state. Should we prioritize giving consumers control, or is this change sufficient for our use cases?