-
Notifications
You must be signed in to change notification settings - Fork 1
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
#305-skeletons #329
#305-skeletons #329
Conversation
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.
Отличная работа, но есть парочка замечаний
'' | ||
)} | ||
</div> | ||
{isLoading ? ( |
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.
давай сделаем как с остальными скелетонами
PageDescription - не нужно знать о загрузке и тд, по сути мы его можем переиспользовать в других компонентах, где даже может не быть обращение к серверу.
Тоесть, здесь тоже делаем компонент PageDescriptionSkeleton и используем его в зависимости от флага загрузки
@@ -88,7 +92,16 @@ export const ProductsPage = () => { | |||
<CategoryList /> | |||
<div className={styles['content-main']}> | |||
<section className={styles['content-products']}> | |||
{categoriesProducts.results.length > 0 ? ( | |||
{isLoading ? ( |
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.
вот так же делаем с PageDescriptionSkeleton
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.
это храним не в shared, а в компоненте PafeControlsSkeleton, например в папке ui. Потому что skeleton относится именно к своему компоненту
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 | ||
? Array(15) | ||
.fill(0) | ||
.map(sk => <Skeleton className={styles['sk-category-list__item']} inline={true} key={sk} />) |
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.
так не делаем, здесь должен быть компонент CategoryItemSkeleton, его уже здесь рендерим
|
||
return ( | ||
<div className={styles['category-list']}> | ||
<Heading type={HeadingType.NORMAL} className={styles['category-list__title']}> | ||
Категории | ||
</Heading> | ||
<ul className={styles['category-list__items']}> | ||
{categoryBranches.branches?.length > 0 | ||
{isLoading | ||
? Array(15) |
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.
15 - в константы
Внесла отмеченные тобой корректировки:
|
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.
Все отлично, кроме того, что почему то используются компоненты из папки components - их необходимо перенести в соответствии с fsd
Приложу скрин, как выглядит прогрузка со скелетонами.