[Address Book v2] - Prevent name validation in AB reducer#2356
Conversation
|
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
[warning] @typescript-eslint/explicit-module-boundary-types
Report generated by eslint-plus-action |
| import { isFeatureEnabled } from 'src/config' | ||
| import { FEATURES } from 'src/config/networks/network.d' | ||
| import { isValidAddress } from 'src/utils/isValidAddress' | ||
| import { ADDRESS_BOOK_INVALID_NAMES, isValidAddressBookName } from '../../logic/addressBook/utils' |
| @@ -50,7 +45,6 @@ export default handleActions<AppReduxState['addressBook'], Payloads>( | |||
|
|
|||
| addressBookEntries | |||
| // exclude those entries with invalid name | |||
There was a problem hiding this comment.
This comment was related to the .filter
| text="Owner Name" | ||
| type="text" | ||
| validate={composeValidators(required, minMaxLength(1, 50))} | ||
| validate={composeValidators(required, minMaxLength(1, 50), validAddressBookName)} |
There was a problem hiding this comment.
This will force to set a name to all owners when loading a Safe
There was a problem hiding this comment.
oops! thanks, just removed it.
| text="Owner Name" | ||
| type="text" | ||
| validate={composeValidators(required, minMaxLength(1, 50))} | ||
| validate={composeValidators(required, minMaxLength(1, 50), validAddressBookName)} |
There was a problem hiding this comment.
This will force to set a name to all owners when creating a Safe
| text="Safe name" | ||
| type="text" | ||
| validate={composeValidators(required, minMaxLength(1, 50))} | ||
| validate={composeValidators(required, minMaxLength(1, 50), validAddressBookName)} |
There was a problem hiding this comment.
We could make minMaxLength(1, 50) part of validAddressBookName.
| @@ -94,7 +94,7 @@ export const CreateEditEntryModal = ({ | |||
| testId={CREATE_ENTRY_INPUT_NAME_ID} | |||
| text="Name" | |||
There was a problem hiding this comment.
I find some places where an input is mandatory but we are not setting the *, while this character is tipically used for indicating that. I would like to raise this here because it's missing so either we should add it also here as it's a mandatory field or remove it from every place were we have it, because I don't even remember an input that could accept empty values in the whole app
There was a problem hiding this comment.
@katspaugh would also like to read your input here
There was a problem hiding this comment.
Personally not a fan of literally typing * in the label.
Ideally the component should indicate somehow that it's required if the prop required is true.
But yeah, for consistency with the other inputs it might make sense to add it here.
Co-authored-by: katspaugh <katspaugh@users.noreply.github.com>
fernandomg
force-pushed
the
fix/prevent-name-validation-in-reducer
branch
from
1fe2697 to
d4997f3
Compare
3 participants
What it solves
Moves the name validation from the reducer to the UI (the error will appear in the form field)
How this PR fixes it
By creating a validator
validAddressBookNamefor the name form fieldHow to test it
verify that the error will appear on any form field for names that will be stored in the address book.