31Vector31 / react-training

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

review online-store #10

Closed shevchuknine closed 2 years ago

shevchuknine commented 2 years ago

в адресной строке я могу установить любое невалидное значение и оно будет применено, нужна валидация получаемых данных https://github.com/31Vector31/react-training/blob/1d67d5c1c750af6eca57af8253f92f4dfa3c8218/src/OnlineStore/Catalog/Catalog.js#L38-L39 выглядит так, будто можно обойтись только useSearchParams, useState его просто дублирует https://github.com/31Vector31/react-training/blob/1d67d5c1c750af6eca57af8253f92f4dfa3c8218/src/OnlineStore/Catalog/Catalog.js#L44 фильтрацию продуктов на основании полученных фильтров имеет смысл скрыть в отдельный компонент, она мешается на этом уровне. сортировка так же https://github.com/31Vector31/react-training/blob/1d67d5c1c750af6eca57af8253f92f4dfa3c8218/src/OnlineStore/Catalog/Catalog.js#L47 если после проверки if ты возвращаешь буевое значение, значит проверку можно сократить до возврата условной части из выражения if https://github.com/31Vector31/react-training/blob/1d67d5c1c750af6eca57af8253f92f4dfa3c8218/src/OnlineStore/Catalog/Catalog.js#L67 key самостоятельно будет приведен к строке _ https://github.com/31Vector31/react-training/blob/1d67d5c1c750af6eca57af8253f92f4dfa3c8218/src/OnlineStore/CatalogFilters/CatalogFilters.js#L12-L13 как быть, если производителей будет динамическое кол-во? должна быть реализация через массив выбранных значений

shevchuknine commented 2 years ago

я все еще могу сделать priceTo меньше. чем priceFrom https://github.com/31Vector31/react-training/blob/5295054601036789790f579ed43a610cb5e5d8bd/src/OnlineStore/Catalog/Catalog.js#L46 это выглядит как функциональность вычисления params, незачем сразу же обновлять невалидные значения фильтов в адресной строке. это можно сделать после какого-нибудь изменения. имеет смысл научить каждый фильтровый компонент отдельно валидировать принимаемые значения, иначе у тебя получается размазанная логика по всему приложению https://github.com/31Vector31/react-training/blob/5295054601036789790f579ed43a610cb5e5d8bd/src/OnlineStore/Catalog/Catalog.js#L45 пустая строчка точно валидное значение для сортировки? https://github.com/31Vector31/react-training/blob/5295054601036789790f579ed43a610cb5e5d8bd/src/OnlineStore/CatalogFilters/CatalogFilters.js#L12 используй allBrands как список доступных опций, а brands как выбранные - будет сильно проще

shevchuknine commented 2 years ago

установка начальных значений брендов не работает при загрузке приложения с параметрами в случае невалидных значений сортировки, при последующих изменениях фильтров, она исчезает из адресной строки, но невалидные значения цены (endPrice > startPrice) - нет https://github.com/31Vector31/react-training/blob/1b8b6fda3924d93cef7ff560cc869221e982a7e3/src/OnlineStore/CatalogFilters/CatalogFilters.js#L25-L29 я не до конца понимаю предназначение этой логики - это валидация входных параметров? если да, я тебе описывал в прошлых замечаниях, что это стоит поместить в вычисление params. изменять поисковую строку, при невалидных значениях, сразу же после загрузки приложения незачем. обнуление поискового параметра через пустой массив - костыль и я не уверен, что он работает https://github.com/31Vector31/react-training/blob/1b8b6fda3924d93cef7ff560cc869221e982a7e3/src/OnlineStore/CatalogFilters/CatalogFilters.js#L45 функциональность с minDistance ДОлжна быть скрыта внутри Slider-а. если я захочу сделать еще один точно такой же слайдер, мне понадобится копировать весь handkeChange с логикой по minDistance https://github.com/31Vector31/react-training/blob/1b8b6fda3924d93cef7ff560cc869221e982a7e3/src/OnlineStore/CatalogFilters/CatalogFilters.js#L46-L51 в этом блоке изменяется состояние setPrice, но не вызывается addSearch https://github.com/31Vector31/react-training/blob/1b8b6fda3924d93cef7ff560cc869221e982a7e3/src/OnlineStore/Catalog/Catalog.js#L22 brands нужно объявлять внутри фильтров

shevchuknine commented 2 years ago

https://github.com/31Vector31/react-training/blob/97ed77dcbe5fa9dd1b92abd6c2db136a04f43158/src/OnlineStore/Catalog/Catalog.js#L39 фильтры изначально получают занчения из адресной строки, которые могут быть невалидными, потом ты их валидируешь и снова устанавливаешь в фильтры. т.е. filters не дает гарантий, что там хранятся валидные значения - это нужно исключить _ https://github.com/31Vector31/react-training/blob/97ed77dcbe5fa9dd1b92abd6c2db136a04f43158/src/OnlineStore/CatalogFilters/CatalogFilters.js#L35 эта логика уже выполняется в предыдущем хуке, нужно переиспользовать

shevchuknine commented 2 years ago

перенос вычислений внутрь фильтров удобства сильно не добавил. чтобы избавиться от лишних пропсов, которые передаются в фильтровые компоненты, можно в каждом из фильтров вынести парсинг фильтров в отдельную функцию, импортировать и применять ее в ленивом вычислении фильтров в главном компоненте. такое решение выглядит более рациональным