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

refactor(Toast): transfer to functional component & refactor tests/stories #2740

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

JackUait
Copy link
Contributor

@JackUait JackUait commented Jan 24, 2022

Для этого компонента придётся оставить как старый (классовый) вариант, так и новый (функциональный) вариант с использованием хука так как одно из важных преимуществ, которое дают функциональные компоненты - больше средств для следования декларативному подходу, но если переписывать текущее API на функциональный лад, то действительно получится тот самый рефактор ради рефактора.

Как использовать новое API:

Cтарое API:
Toast.push('Successfully saved', {
    label: 'Cancel',
    handler: () => Toast.push('Canceled'),
  });

---------

Аналог с использованием нового API:
const { addToast } = useToast()

addToast('Successfully saved', {
	label: 'Cancel',
	handler: () => addToast('Canceled'),
})
  1. Удалил анимации exit и exitActive, так как с ними и без них анимация исчезновения была одинаковой.
  2. Вынужденно сократил API компонента, так как теперь он превратился в хук. Большинство из проблем сокращения API решаются добавлением пропа render, в который можно будет передать кастомный компонент. Все остальные проблемы решаются расширением API второго аргумента хука. Конкретно сейчас я вижу два "проблемных" пропа: onClose и onPush. Я ещё не реализовывал эту логику, так как прежде хотел обсудить плюсы и минусы такого решения.
    Пример того, о чём я говорю:
addToast('Successfully saved', {
	onClose: () => console.log('Toast was closed'),
	onPush: () => console.log('Toast was opened'),
	// Полностью заменяет собой внешний вид тоста, оставляя только поведение.
	// При создании тоста сверху просто вылезет надпись "Hello World" без стилей. 
	render: () => {
		<p>Hello World</p>
	}
})
  1. Убрал возможность иметь сразу несколько тостов на экране одновременно, так как текущее поведение при нескольких тостах на экране выглядит как баг. В процессе обсуждения этого вопроса с @dzekh.
Текущее поведение
2022-01-24.18.52.34.mov
Возможное решение
2022-01-24.20.32.37.mov
  1. Правильно понимаю, что для того, чтобы пользователь мог использовать что-то из библиотеки завязанное на контексте, ему нужно обернуть своё приложение в соответствующий провайдер? Есть ли какие-то другие механизмы реализации этого поведения?

@JackUait JackUait requested a review from zhzz January 24, 2022 20:25
@JackUait
Copy link
Contributor Author

Для того, чтобы протестировать хук, нужна библиотека для тестирования хуков из #2747.

@JackUait JackUait changed the title feat(Toast): transfer to functional component & refactor tests/stories refactor(Toast): transfer to functional component & refactor tests/stories Jan 31, 2022
@JackUait
Copy link
Contributor Author

JackUait commented Feb 1, 2022

Смёржил актуальный мастер.

@dzekh
Copy link

dzekh commented Feb 7, 2022

Убрал возможность иметь сразу несколько тостов на экране одновременно

Да, всё правильно, хорошо что убрал. В гайдах так и написано что должен старый скрываться: https://localhost:3000/components/toast/#08

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

Successfully merging this pull request may close these issues.

2 participants