-
Notifications
You must be signed in to change notification settings - Fork 0
State Management and Context API #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: hooks-and-routing
Are you sure you want to change the base?
Conversation
maiano
commented
Aug 3, 2025
- Task: link
- Screenshot:

- Deploy: link
- Done 04.08.2025 / deadline 04.08.2025
- Score: 100 / 100
- State management is properly implemented with either Redux Toolkit or Zustand - 35
- Selected items are managed through the state store, selected items are persistent across pages - 25
- Flyout component is showed/hidden based on the presence of selected items, displays the number of selected items - 15
- "Unselect all" button and "Download" button work according to the requirements - 10
- User can switch the theme of the application using Context API - 15
AleksGoodness
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Видно использование deepSeek =) но в целом все отлично. Советую все же писать код самому что бы лучше в нем разбиратся. Все мы не без грешны.
Очень понравились твои тесты сам стиль их написания моки выглядят красиво.
комментарий пишу до того как смотрел весь код в vscode. С требованиями rs-school нужно быть более внимательным а то можно влететь на большой минус
| /* eslint-disable @typescript-eslint/no-unsafe-return */ | ||
| /* eslint-disable @typescript-eslint/no-unsafe-call */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
думаю да же в тестах так лучше не делать. Все же эти правила нужны что бы код писать однообразно
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, согласен, тем не менее иногда линтер слишком строг
| import { Button } from '@/components/Button'; | ||
| import { UI_STRINGS } from '@/shared/constants/ui-strings'; | ||
| import { downloadCharactersCSV } from '@/shared/utils/download-characters-csv'; | ||
| import { useSelectedCharactersStore } from '@/store/selectedCharactersStore'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не помешали бы разделители по строке было бы удобней читать
| import SunIcon from '@/assets//sun-com.svg?react'; | ||
| import MoonIcon from '@/assets/moon-com.svg?react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
я как понимаю использовал собственные иконки. хотелось бы знать почему было приянто такое решение.
и посмотри на пути что то не верно в этом пути //
| import { FallBack } from '@/components/FallBack/'; | ||
| import { ThemeProvider } from '@/context/ThemeProvider.tsx'; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ну ты же понимаешь что так нельзя за это -20 балов в проверке
Usage of ts-ignore (-20 points)
темболие решается это через обычный if
const root = document.getElementById('root');
if (root)
createRoot(root).render(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
я пока снимать не буду но все же исправь. И я в шоке что до меня тебе такое ни кто не писал
| import { CardList } from '@/components/CardList'; | ||
| import { CharacterDetails } from '@/components/CharacterDetails'; | ||
| import { Flyout } from '@/components/Flyout'; | ||
| import { LoadingOverlay } from '@/components/LoadingOverlay'; | ||
| import { Pagination } from '@/components/Pagination'; | ||
| import { SearchBar } from '@/components/SearchBar'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
у тебя настроены алиасы это хорошо еще бы я посоветовал бы создать в папке компонентов index.ts файл и в него импортировать все компоненты и просто сделать реэкспорт.
это позволит сократить количество импортов и сделать общий код читаемей
вот пример как у меня это выглядит:
import { Button } from './button/Button';
import { CountryItem } from './country-item/CountryItem';
import { CountryList } from './country-list/CountryList';
import { ErrorBoundary } from './error-boundary/ErrorBoundary';
import { Flyout } from './flyout/Flyout';
import { GeneralLayout } from './layout/GeneralLayout';
import { Pagination } from './pagination/Pagination';
import { SearchForm } from './search-form/search-form/SearchForm';
import { SearchInput } from './search-input/SearchInput';
import { SkeletonListItem } from './skeleton-list-item/SkeletonListItem';
import { ThemeChanger } from './theme-changer/ThemeChanger';
export {
Button,
CountryItem,
CountryList,
ErrorBoundary,
Flyout,
GeneralLayout,
Pagination,
SearchForm,
SearchInput,
SkeletonListItem,
ThemeChanger,
};
и вот результат в странице home
import clsx from 'clsx';
import { useState } from 'react';
import { Outlet, useLoaderData, useParams } from 'react-router';
import {
Button,
CountryList,
Flyout,
Pagination,
SearchForm,
} from '@/components';
import type { ICountry } from '@/interfaces';| characterId={characterId} | ||
| onClose={() => | ||
| navigate(`/character?${searchParams.toString()}`) | ||
| {isLoading ? null : isError ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тернарники трудно понмать советую упростить до строчки
по типу
{isLoading && <loader/>}
{isError && <Error/>}
{isLoading === false && isError === false && <Cards/>}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кстати, да накопилось нужен рефакторинг
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это прикольный подход =)
| const link = document.createElement('a'); | ||
| link.setAttribute('href', url); | ||
| link.setAttribute('download', `${characters.length}_items.csv`); | ||
| document.body.appendChild(link); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это опять -20 балов за использование прямых манипуляций с дом
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
использование set прям красавчик!