Feature/yh 1915#1249
Conversation
artem-Tolmachev
added
review
| @@ -1,5 +1,5 @@ | |||
| export type SortOrder = 'ASC' | 'DESC'; | |||
|
|
|||
| export type SortBy = 'name' | 'difficulty' | 'createdAt' | 'updatedAt'; | |||
There was a problem hiding this comment.
[ISSUE] - Неправильно выносить типы относящиеся строго к задачам в shared. Перенеси туда же откуда ты вынес
Если ты делал по примеру SortOrder, то этот тип универсальный, так как других значений не будет. А название полей отличается у каждой сущности
| value?: string; | ||
| } | ||
|
|
||
| export const SortFilterSection = <T extends string>({value, onChange, data, title }: SortFilterSectionProps<T>) => { |
There was a problem hiding this comment.
[ISSUE] - Не вижу смысла создавать этот компонент фактически ради 1 функции обработчика. Так что предлагаю удалить его
| onChangeSortField, | ||
| onChangeSortOrder, | ||
| } = useTasksFilters({ page: 1 }); | ||
|
|
There was a problem hiding this comment.
[ISSUE] - Не добавлены фильтры новые в запрос
| onChangeLangIds={onChangeLangIds} | ||
| onChangeCategory={onChangeCategory} | ||
| onChangeCompanyId={onChangeCompanyId} | ||
| onChangeSortField={onChangeSortField} |
There was a problem hiding this comment.
[ISSUE] - Не вижу чтобы в задаче говорилось добавлять фильтры на эту страницу
| <TaskLanguagesFilter selectedLangIds={langIds} onChangeLangIds={onChangeLangIds} /> | ||
| <TaskCategoryFilterList onChooseCategory={onChangeCategory} selectedCategory={category} /> | ||
| <PublicCompanySelect value={companyId} onChange={onChangeCompanyId} /> | ||
| <SortTasksByField value={filters.sortBy} onChangeSortField={onChangeSortField} /> |
There was a problem hiding this comment.
[ISSUE] - Эти фильтры нужны только в админке
| active: boolean; | ||
| }; | ||
|
|
||
| export default function prepareSortFilterData<T extends string>({data, value, t}: FilterDataProps<T>): PreparedSortFilterItem<T>[] { |
There was a problem hiding this comment.
[ISSUE] - Дефольтный экспорт мы не используем. Исключение - lazy компонент так как по другому там никак
Плюс не вижу смысл выносить эту функцию отдельно. Слишком много всего тут ненужного включая типы
Предлагаю удалить этот файл
| @@ -0,0 +1,12 @@ | |||
| import { Tasks } from '@/shared/config'; | |||
|
|
|||
| export const SORT_ORDER_DATA = [ | |||
There was a problem hiding this comment.
[ISSUE] - Вынос этих данных тоже считаю не очень удачным решением, тем более в таком виду
Ты создаешь массив с полями, которые потом меняешь. Слишком много ненужных действий
К тому же тут нет нормальной типизации. as const здесь некорректен
Предлагаю не выносить эти данные тут
There was a problem hiding this comment.
Вернул обратно в компонент
|
|
||
| export const SortTasksByField = ({ onChangeSortField, value }: SortTasksFieldProps) => { | ||
| const { t } = useTranslation(i18Namespace.task); | ||
| const preparedData = prepareSortFilterData({data: SORT_FIELDS_DATA, value, t}) |
There was a problem hiding this comment.
[ISSUE] - Учитывая комментарии о prepareSortFilterData и SortFilterSection, предлагаю сделать по аналогии так, как реализовано тут QuestionSortByFieldFilter. Учитывая что я указал это в задаче
Не зачем городить что-то новое, если уже есть готовый пример
There was a problem hiding this comment.
Удалил prepareSortFilterData и SortFilterSection
|
|
||
| export const SortTasksByOrder = ({ onChangeSortOrder, value }: SortTasksByOrderProps) => { | ||
| const { t } = useTranslation(i18Namespace.task); | ||
| const preparedData = prepareSortFilterData({data: SORT_ORDER_DATA, value, t}) |
There was a problem hiding this comment.
[ISSUE] - Тут таке как и в соседнем компоненте
PerelomaDenis
added
comments
2 participants
Что сделано
SortTasksByFieldдля выбора поля сортировкиSortTasksByOrderдля выбора направления (ASC/DESC)sortByиsortOrderДополнительные исправления
QuestionsFilters.tsx(ошибка pre-commit hook):import { TopicFilterField } from '@/entities/topic/ui/TopicFilterField/TopicFilterField'import { TopicFilterField } from '@/entities/topic'