31Vector31 / react-training

Hooks, Axios, HOC, Redux, Context, Reselect, Router.
0 stars 0 forks source link

review field #11

Closed shevchuknine closed 2 years ago

shevchuknine commented 2 years ago

https://github.com/31Vector31/react-training/blob/1b8b6fda3924d93cef7ff560cc869221e982a7e3/src/Field/Container.js#L30 эта обертка в useCallback не имеет смысла, т.к. ты все-равно каждый раз возвращаешь новую функцию, когда используешь ее https://github.com/31Vector31/react-training/blob/1b8b6fda3924d93cef7ff560cc869221e982a7e3/src/Field/Container.js#L34 isSelected должно быть внутренним полем для дропдауна, снаружи его использование не обосновано. колбек должен принимать уже конкретное значение, а не обрабатывать объект события. зачем это делать снаружи, если обработку объекта можно реализовать внутри дропдауна. т.е. дропдаун принимает options и value, а все остальное - только реализация дропдауна внутри. наружу через onChange он так же отдает только одно значение. тот же пдход применим и к мультиселекту https://github.com/31Vector31/react-training/blob/1b8b6fda3924d93cef7ff560cc869221e982a7e3/src/Field/Container.js#L56 деструктуризацию можно провести в аргументе функции _ https://github.com/31Vector31/react-training/blob/1b8b6fda3924d93cef7ff560cc869221e982a7e3/src/Field/Field.js#L7-L12 лучше сначала собрать массив валидационных результатов и только потом устанавливать состояние

shevchuknine commented 2 years ago

https://github.com/31Vector31/react-training/blob/a76e290e276b2e6d0129845112543b936dbd3ddf/src/Field/Field.js#L9 имутабельность в приоритете, используй .reduce https://github.com/31Vector31/react-training/blob/a76e290e276b2e6d0129845112543b936dbd3ddf/src/Field/Container.js#L41-L48 у тебя дублируется логика, значит имеет смысл выносить этот компонент в отдельный и реализовывать переиспользуемую логику в нем https://github.com/31Vector31/react-training/blob/a76e290e276b2e6d0129845112543b936dbd3ddf/src/Field/Container.js#L76 Dropdown и Multiselect уже и так функции, которые принимают пропсы и распределяют их в нужное место. незачем оборачивать их в дополнительную инлайн-обертку

shevchuknine commented 2 years ago

https://github.com/31Vector31/react-training/blob/d85117f24dbad17d8090f0051f06bd026788571c/src/Field/Container.js#L83 речь не про html обертку, а про инлайн-компонент (({value, ...}) => <Dropdown value={value} .../>), который ты передаешь в component. по сути, ты просто делаешь еще одну функцию, в которой просто передаешь дальше получаемые пропсы. Dropdown точно такая же функция, т.е. ты можешь передать его прямиком в component: component={Dropdown}. перестанет работать error проп, т.к. он прилетает как invalid - его нужно будет подружить с компонентами. то же самое с остальными компонентами на этом уровне