Skip to content
This repository has been archived by the owner on Sep 26, 2024. It is now read-only.

feat(UIKIT-873,ui,NotificationList): Добавлен компонент NotificationList #739

Merged
merged 10 commits into from
Dec 5, 2023

Conversation

max-cepelev
Copy link
Contributor

@max-cepelev max-cepelev commented Nov 14, 2023

Чеклист

  • Написать тесты для реализованного функционала
  • Описать jsdoc для экспортируемых функций, компонентов
  • Описать jsdoc для props экспортируемых компонентов
  • Реализовать или обновить storybook для компонента
  • Проверить код на соответствие style guide
  • Запросить ревью у дизайнера, если был изменен ui или создан новый компонент

@max-cepelev max-cepelev requested review from a team and DesignASoft and removed request for a team November 14, 2023 09:17
@DesignASoft
Copy link

уведомления считаются прочитанными, если пользователь открыл NotificationList и закрыл его. При этом Прочитанными будут только те уведомления, которые попали в поле зрения на странице. Те уведомления, которые были под скроллом и пользователь их не видел, должны остаться не прочитанными
Также уведомление становится прочитанным, если пользователь перешел по кнопке действия в нем

Кнопка Отметить все как прочитанные находится в правом нижнем углу модалки и имеет фиксированное положение.

В Storybook для вариантов With button этой кнопки нет, в варианте with icon есть кнопка отметить как прочитанное есть в каждом непрочитанном уведомлении. Со стороны дизайна такие варианты не рассматривались

@DesignASoft
Copy link

DesignASoft commented Nov 17, 2023

Для варианта with links в notification list нет кнопки "Отметить все как прочитанные"
Для without tabs - при закрытии окна и при нажатии "Отметить все как прочитанные" счетчик не меняется

@max-cepelev
Copy link
Contributor Author

Для варианта with links в notification list нет кнопки "Отметить все как прочитанные" Для without tabs - при закрытии окна и при нажатии "Отметить все как прочитанные" счетчик не меняется

Кнопка "Отметить все как прочитанные" сделана опциональной, т.к. в реализации ЭДО ее, скорее всего не будет, поэтому ее там специально нет. В without tabs пример сделан только для демонстрации, что есть такой вариант. Но логику добавлю. Вообще, логика обработки прочитанных уведомлений вынесена из компонента, т.к. предполагается, что она на каждом проекте может быть разной. У нас в ЭДО она будет реализована при переходе в определенный раздел.

packages/components/src/Dialog/Dialog.stories.tsx Outdated Show resolved Hide resolved
@@ -0,0 +1,3 @@
export { DialogHeader as SideDialogHeader } from '../DialogHeader';

export type { DialogHeaderProps as SideDialogHeaderProps } from '../DialogHeader';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для каждого компонента должен быть storybook. Смотри в contributing

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил пример с использованием SideDialogHeader в storybook SideDialog, нужно его перенести в SideDialogHeader?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, можно просто продублировать кейс

packages/components/src/NotificationList/styles.ts Outdated Show resolved Hide resolved
@AndTem
Copy link
Collaborator

AndTem commented Nov 27, 2023

Тригерни design review

packages/components/src/DialogHeader/styles.ts Outdated Show resolved Hide resolved
packages/components/src/DialogHeader/DialogHeader.tsx Outdated Show resolved Hide resolved
@@ -0,0 +1,3 @@
export { DialogHeader as SideDialogHeader } from '../DialogHeader';

export type { DialogHeaderProps as SideDialogHeaderProps } from '../DialogHeader';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

up

*/
title?: string;
/**
* @description Кнопка для закрытия модального окна
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это ж не кнопка, а обработчик. Здесь вообще комент не нужен и так все понятно из названия. В title также

</Typography>
{actions && <ListItemActions>{actions}</ListItemActions>}
</div>
{isDeleteButtonVisible ? (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isDeleteButtonVisible && <Listiem

Так проще и лучше читается

imgAlt: '',
errorList: [errorMessage || ''],
onRetry: handleRetry,
imgSrc: imagesMap.defaultErrorImgSrc,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Должно браться либо из пропсов, либо из контекста

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imagesMap берется из ConfigContext, а из imagesMap берем источники для изображений состояния

* @description флаг управляет, какие уведомления выводить при открытии списка, все/непрочитанные
* @default undefined
* */
isInitialUnreadOnly?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А если добавиться еще один таб, что делать?

@max-cepelev max-cepelev merged commit f39065c into main Dec 5, 2023
9 of 11 checks passed
@max-cepelev max-cepelev deleted the feat/UIKIT-873 branch December 5, 2023 11:25
Copy link
Contributor

github-actions bot commented Dec 5, 2023

🎉 This PR is included in version 3.55.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants