-
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
Development 369 my account page #400
Conversation
…льзователя в шапке.
…нта. Поправил аккардеон
…тирование новых страниц
… удаления невалидного токена из заголовков по-умолчанию.
…rontend into development_369_my-accoun_page
…rontend into development_369_my-accoun_page
…rontend into development_369_my-accoun_page
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.
Отличная работа!
Парочка комментариев и вопросов
@@ -1,15 +1,16 @@ | |||
import { useEffect, useState } from 'react' | |||
|
|||
import { IProduct } from '@/shared/model/types/ProductModel' |
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.
Такие импорты можно тоже через type импортировать
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.
)
@@ -82,6 +89,32 @@ const HeaderAccount: FC<HeaderAccountProps> = ({ | |||
} | |||
}, [isModalOpen, isAuth]) | |||
|
|||
const logoutBtnHandle = () => { |
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 handleKeyUp = (e: KeyboardEvent<HTMLDivElement>) => { | ||
if (e.code === 'Enter' || e.code === 'Space') { | ||
e.preventDefault() | ||
dispatch(logout()) |
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 { data } = await extra.api.get(`api/${ApiRoutes.USER}/`) | ||
return data | ||
} catch (error) { | ||
if (axios.isAxiosError(error) && error.response && error.response.status === 401) { |
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.
вот здесь не очень понял логику, если у нас 401-я ошибка - мы выходим из приложения?
У нас рефреш токена нет?
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.
Да, другой ручки нет
navigate(Routes.HOME) | ||
} | ||
|
||
const sideBar = useMemo(() => { |
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.
Это только тут используется. Но условную отрисовку компонентов внутри вынес в HOC
@@ -5,7 +5,7 @@ export interface IProduct { | |||
category: string | |||
brand: string | |||
images: IImage[] | |||
price: string | |||
price: string | number |
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.
ты вроде в другом компоненте использовал Nullable, давай будем последовательными и здесь тоже его заиспользуем
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.
так тут вроде не с null
* @param {number} number количество товаров | ||
* @returns {string} окончание для слова товар, в зависимости от их количества | ||
*/ | ||
export const getEndingByNumber = (number: number): string => { |
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.
Обрати внимание на вот эту апишку:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/PluralRules
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.
О, здорово! применил
src/widgets/LastOrder/LastOrder.tsx
Outdated
<div className={styles.lastOrder__order}> | ||
<Paragraph className={styles.lastOrder__text}>Вы еще не совершали покупок!</Paragraph> | ||
</div> | ||
<div className={styles.lastOrder__footer}></div> |
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 className={styles.lastOrder__footer}></div> | |
<div className={styles.lastOrder__footer} /> |
@@ -34,15 +36,21 @@ const SideBarMenu: FC<ISideBarMenu> = ({ user, handleLogOut }) => { | |||
const handleKeyUp = (e: KeyboardEvent<HTMLLIElement>) => { | |||
if (e.code === 'Enter' || e.code === 'Space') { | |||
e.preventDefault() | |||
handleLogOut() | |||
dispatch(logout()) |
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.
Ага, переиспользовал хук
label_hit={item.label_hit} | ||
label_popular={item.label_popular} | ||
quantity={item.quantity} | ||
label_hit={item.label_hit as boolean} |
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.
а почему здесь типизируется через as, не подтягиваются типы?
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.
Отличная работа!
No description provided.