Skip to content
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

plasma-new-hope: Notification redesign #927

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

kayman233
Copy link
Contributor

@kayman233 kayman233 commented Dec 14, 2023

Notification

  • добавлен новый дизайн компонента
  • добавили новый api

Было:
Снимок экрана 2023-12-14 в 17 16 05

Стало:
Снимок экрана 2023-12-14 в 17 16 25

What/why changed

Добавлены иконки, кнопки для взаимодействия, а также 2 вида layout - горизонтальный и вертикальный

📦 Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
# or 
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-927/

@Salute-Eva
Copy link
Contributor

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-927/

@Salute-Eva
Copy link
Contributor

Copy link
Contributor

⚡ Component performance testing

Result: 🟢 OK


export const NotificationsProvider: React.FC<{ children: ReactNode; frame?: string }> = ({
children,
frame = 'theme-root',
frame = 'document',
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.

это я забыл убрать, пока тестил. Спасибо, что заметил

*/
actions?: ReactNode;
/**
* Тип направления расположения всех частей Notification.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Комментарий и название prop как мне кажется немного про разное.

Может - "схема расположение блоков Notification" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Да, так получше звучит, как ты предложил

&.${classes.horizontal} {
display: flex;
align-items: center;
flex: 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Уточнение]

Это сокращение - и тут значение 1 для: flex-shrink, flex-base, flex-grow.

Это так и задумывалось?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

вообще flex-grow

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-927/

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-927/

@Salute-Eva
Copy link
Contributor

Copy link
Contributor

⚡ Component performance testing

Result: 🟢 OK

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-927/

@Salute-Eva
Copy link
Contributor

@kayman233 kayman233 added this pull request to the merge queue Dec 19, 2023
Copy link
Contributor

⚡ Component performance testing

Result: 🟢 OK

Merged via the queue into dev with commit 6ddc660 Dec 19, 2023
25 checks passed
@kayman233 kayman233 deleted the kayman233/notification-redesign.PLASMA-2235 branch December 19, 2023 09:31
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

1 similar comment
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

This was referenced Dec 22, 2023
@Yakutoc Yakutoc mentioned this pull request Dec 22, 2023
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

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

Successfully merging this pull request may close these issues.

7 participants