Open DenisovVladimir opened 1 year ago
DenisovVladimir
commented
1 year ago https://github.com/ShestakovAndrew/ReactStudy/blob/e8e39ec28884db1ff91e1b231eb5aa7e772cfaf2/todo-list-project/src/pages/todoList/model/state/prioritySlice.ts#L9-L11 Тоесть когда пользователь первый раз откроет страницу, у него будут исключительно таски с высоким приоритетом? Кажется, что если я не применил фильтр, то хочу видеть всё
DenisovVladimir
commented
1 year ago DenisovVladimir
commented
1 year ago https://github.com/ShestakovAndrew/ReactStudy/blob/e8e39ec28884db1ff91e1b231eb5aa7e772cfaf2/todo-list-project/src/pages/todoList/dropDown/DropDown.tsx#L1-L8 Ты сгруппировал импорты, но я совсем не понял лоигики группировки
DenisovVladimir
commented
1 year ago DenisovVladimir
commented
1 year ago https://github.com/ShestakovAndrew/ReactStudy/blob/e8e39ec28884db1ff91e1b231eb5aa7e772cfaf2/todo-list-project/src/pages/todoList/dropDown/DropDown.tsx#L28-L35 Почему просто не сравнить с todoPriorityFromStore, почему это вообще пропс? Зачем приоритет сетать через callback. У тебя сама по себе архитектура нарушается. Нажали на приоритет, если в сторе не такой же диспатчишь action. Или можешь просто диспатчить, на уровне редюсера всё равно надо разруливать кейс, когда ничего не поменялось, чтобы перерисовок не было.
DenisovVladimir
commented
1 year ago Вынеси из useEffect объявление. И почему outsideClick. У тебя потом проверка, что он outside. Название функции звучит так, как будто ты будешь обрабатывать исключительно клики снаружи компонента
DenisovVladimir
commented
1 year ago https://github.com/ShestakovAndrew/ReactStudy/blob/e8e39ec28884db1ff91e1b231eb5aa7e772cfaf2/todo-list-project/src/pages/todoList/dropDown/DropDown.tsx#L58 Название handleToggleShow тоже не соответсвут тому, что происходит. Toggle это уже переключений. По клику ты обрабатываешь клик, а не переключение состояния
DenisovVladimir
commented
1 year ago https://github.com/ShestakovAndrew/ReactStudy/blob/e8e39ec28884db1ff91e1b231eb5aa7e772cfaf2/todo-list-project/src/pages/todoList/dropDown/DropDown.tsx#L65 Тут аналогично. Функция - обработчик клика, а не смены. То что обработчик клика меняет состояние не отражает сути функции. Функция handleSetPriority звучит так, что она должна обработать смену статуса. Типо статус поменялся, она подписана на событие изменения статуса. И по этому событию например должно показать пользователю сообщение, что статус успешно изменён
DenisovVladimir
commented
1 year ago https://github.com/ShestakovAndrew/ReactStudy/blob/e8e39ec28884db1ff91e1b231eb5aa7e772cfaf2/todo-list-project/src/pages/todoList/TodoList.tsx#L3 Тоже не понял зачем реакт импортится, вроде нигде не используется
DenisovVladimir
commented
1 year ago https://github.com/ShestakovAndrew/ReactStudy/blob/e8e39ec28884db1ff91e1b231eb5aa7e772cfaf2/todo-list-project/src/pages/todoList/TodoList.tsx#L21 Я теперь всё меньше понимаю как это должно работать. В моём понимании, если не хочется селектить дважды, то селектится в странице приоритет и передаётся пропсом текущее состояние в dropDown. По клику, если в dropDown поменялся приоритет диспатчится action. Селектор увидит эти изменения и перерисует и страницу и dropdown. Что тут происходит я честно не понял
DenisovVladimir
commented
1 year ago https://github.com/ShestakovAndrew/ReactStudy/blob/e8e39ec28884db1ff91e1b231eb5aa7e772cfaf2/todo-list-project/src/pages/todoList/TodoList.tsx#L24-L30 Сортировку в компонете наверное не стоит делать никогда. Её стоит перенести в редюсер, во view должно быть как можно меньше логики.
DenisovVladimir
commented
1 year ago https://github.com/ShestakovAndrew/ReactStudy/blob/e8e39ec28884db1ff91e1b231eb5aa7e772cfaf2/todo-list-project/src/pages/todoList/model/actions/changeTodoPriority.ts#L6 Добавить проверку на изменение, чтобы не перерисовывать компонент, если ничего не изменилось
ShestakovAndrew
commented
1 year ago Странно выглядит, что в модели у тебя есть dir "state", а внутри у тебя slice. Хочется либо в разные файлы, либо подругому назвать папку
Store который принимает в себя редьюсеры обращается к директории page, посмотрел как делают другие, у них одна на верхнем уровне директория для store и внутри уже раскидано по каждой страницы. Я сделал так же. От директории model отказался.
ShestakovAndrew
commented
1 year ago Тоесть когда пользователь первый раз откроет страницу, у него будут исключительно таски с высоким приоритетом? Кажется, что если я не применил фильтр, то хочу видеть всё
При открытии в первый раз будет происходить сортировка сначала с высоким приоритетом (они будут вверху, остальные внизу). Сортировка ведь не подразумевает отображение только конкретных тасков с определённым приоритетом. Выставленный приоритет отображает таски от меньшего к большему: выбрал 2 значит 2 1 0, выбрал 1 значит 1 2 0, выбрал 0 значит 0 2 1.
ShestakovAndrew
commented
1 year ago ShestakovAndrew
commented
1 year ago Ты сгруппировал импорты, но я совсем не понял лоигики группировки
немного перемудрил) переделал во всех компонентах импорты, оптимизировав через ctrl+alt+O, пробелы убрал.
ShestakovAndrew
commented
1 year ago Почему просто не сравнить с todoPriorityFromStore, почему это вообще пропс? Зачем приоритет сетать через callback. У тебя сама по себе архитектура нарушается. Нажали на приоритет, если в сторе не такой же диспатчишь action. Или можешь просто диспатчить, на уровне редюсера всё равно надо разруливать кейс, когда ничего не поменялось, чтобы перерисовок не было.
Компонент принимает в себя необязательный флаг isChangeStoreTodoPriority, который говорит нужно ли изменять в сторе выбранный приоритет или нет. Сделал так потому что DropDown используется в 2 местах. В одном мне надо чтобы после изменения в сторе у меня менялся приоритет и компонент перерисовывался. В другом случае у меня ничего не должно перерисовываться так как я просто выбрал приоритет и закрыл менюшку (создание новой задачи)
ShestakovAndrew
commented
1 year ago Вынеси из useEffect объявление. И почему outsideClick. У тебя потом проверка, что он outside. Название функции звучит так, как будто ты будешь обрабатывать исключительно клики снаружи компонента
Вынес, переименовал просто в handleClick.
ShestakovAndrew
commented
1 year ago Название handleToggleShow тоже не соответсвут тому, что происходит. Toggle это уже переключений. По клику ты обрабатываешь клик, а не переключение состояния
Переименовал в handleDropdownButtonClick.
ShestakovAndrew
commented
1 year ago Тут аналогично. Функция - обработчик клика, а не смены. То что обработчик клика меняет состояние не отражает сути функции. Функция handleSetPriority звучит так, что она должна обработать смену статуса. Типо статус поменялся, она подписана на событие изменения статуса. И по этому событию например должно показать пользователю сообщение, что статус успешно изменён
переименовал в handleDropdownItemButtonClick
Странно выглядит, что в модели у тебя есть dir "state", а внутри у тебя slice. Хочется либо в разные файлы, либо подругому назвать папку