Home work 3#25
Conversation
…ten in different components
| } | ||
|
|
||
| if (!inputState.surname) { | ||
| tempErrorState = { |
There was a problem hiding this comment.
И обрати внимание, что проверка пустое ли поле у тебя везде одинакова и не зависит от имени поля или чего-то еще. А это очевидное дублирование кода, значит нужно вынести в отдельную функцию
На самом деле, вся валидация для каждого конкретного поля тоже дублирование, первый и последний шаг абсолютно идентичны, а середину, которая зависит от имени поля переписать так, чтобы сама проверка и текст ошибки выдавались в зависимости от имени проверяемого поля
There was a problem hiding this comment.
Спасибо! Поправил
| let tempErrorState = {...errorState}; | ||
| event.preventDefault(); | ||
| let isValid = true; | ||
| if (!inputState.name) { |
There was a problem hiding this comment.
Валидацию можно бы вынести в отдельную функцию/ объект с ключами по имени и вэлью в виде функций валидаций:
validate(name) вместо 130 строк кода.
Для переиспользуемого компонента (а в этом их предназначение) лучше логику, которая может меняться в зависимости от потребностей, выносить куда-то и задавать ее потом оттуда, прокидывая в компонент или импортируя. Тогда сам компонент как бы чистенький и готов делать, что прикажут по логике, в то же время рисуя одно тот же интерфейс
There was a problem hiding this comment.
Спасибо! переписал функции валидации
| <div className={styles.labelWrapper}> | ||
| <label className={styles.label} htmlFor={props.name}> | ||
| {props.label} | ||
| <textarea className={styles.areaInput} id={props.name} name={props.name} placeholder={props.placeholder} rows={7} wrap='hard' onChange={props.onChange} /> |
There was a problem hiding this comment.
Дело вкуса, но можно деструктуризировать пропсы, чтобы избежать props. каждый раз
There was a problem hiding this comment.
Спасибо! В следующих задания уже использовал деструктуризацию
| } | ||
| return ( | ||
| <div className={styles.formWrapper}> | ||
| {inputState ? <ResultForm result={inputState} /> : <Form onSave={(form) => onSave(form)}/>} |
There was a problem hiding this comment.
Не замечание, но мысль:
если один компонент, то ок через тернарник, но если будет что-то побольше, то, на мой взгляд, условие удобнее прописать через логическое И. Да, нужно будет написать два раза:
{inputState && }
{! inputState && }
но в нек-х случаях это может быть читабельнее, когда много JSX'а
There was a problem hiding this comment.
Спасибо! Согласен, но в данном случае решил воспользоваться тернарником, так как только 2 компоненты
| } | ||
|
|
||
| const submit = function(event) { | ||
| let tempErrorState = {...errorState}; |
There was a problem hiding this comment.
Кажется, можно было бы обойтись без временного состояния ошибок, поскольку
Если на сабмит твои валидации записывают ошибки сразу в errorState , то компонент понимает, что нужно перерендериться, ведь стейт изменился
А вот эта часть кода удалит текст ошибки если она не актуальна(только errorState вместо tempErrorState):
else {
tempErrorState = {
...tempErrorState,
nameError: '',
};
и компонент опять же отрисует уже новое состояние
Вообще все, что меняется и должно быть тут же отрисовано: через стейт или через стор(который как пропс приходит), иначе рискуешь видеть старое значение, пока не компонент не перерендерится по каким-то другим причинам
Поэтому, взаимодействие с доп. объектом, не стейтом, кажется мне неоправданным
There was a problem hiding this comment.
Спасибо! Так как валидация в одной функции, если не использовать временный объект, то мы не сможем иметь актуальное значение стейта при каждой валидации поля
|
На мой взгяд, все гуд Позови Павла, пожалуйста, когда сочтешь нужным |
poulkud
left a comment
poulkud
left a comment
There was a problem hiding this comment.
апрув, но глянь комменты
|
@mariia-kiliushina @poulkud спасибо за замечания, все поправил |
3 participants
class components changed to functional. Form and result form are written in different components