-
Notifications
You must be signed in to change notification settings - Fork 8
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
[FE] 토스트 UI 구현 #391
[FE] 토스트 UI 구현 #391
Conversation
Test Results9 tests 9 ✅ 11s ⏱️ Results for commit 87c6600. ♻️ This comment has been updated with latest results. |
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.
특별히 리뷰드릴 부분은 없는 것 같아 Approve 드립니다! 👏👏👏 멋진 Toast UI가 만들어졌네요!
하지만, 리렌더링 문제를 직접 화면상에 보기 전까지는 아직 잘 와닿지가 않네요..
그래서 그런데 버튼을 눌렀을 때 Toast UI가 보이는 Storybook도 하나 만들어주면 좋겠어용 :)
고생했어요 해리!
<div | ||
css={css` | ||
width: 43rem; | ||
`} | ||
> | ||
<Toast {...args} /> | ||
</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.
[P3]
중복되는 것은 decorator로 빼도 좋겠어요 해리포터군~
const animationTimer = setTimeout(() => { | ||
setIsOpen(false); | ||
}, duration - TOAST_ANIMATION_DURATION_TIME); | ||
|
||
return () => { | ||
clearTimeout(animationTimer); | ||
}; |
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.
해리🍀 이번 PR은 바로 ARROVE 갑니다~~~~ 깔끔하게 잘 구현해 주셨군요!
한 가지 가벼운 제안 코멘트 남겨두었습니다! merge 전에 가볍게 쓱 읽어봐 주세욜~
|
||
const TOAST_ANIMATION_DURATION_TIME = 500; | ||
|
||
export default function ToastContainer({ duration, type, message }: ToastContainerProps) { |
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.
[P3]
duration
기본값을 추가해주면 어떨까용?
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.
좋네요!
- 토스트의 타입에 따라서 아이콘, 아이콘의 배경색을 동적으로 결정할 수 있도록 구현
- 토스트 컴포넌트 UI 테스트를 쉽게할 수 있도록, 토스트가 사라질 때 애니메이션을 적용하기 위한 상태를 관리하는 Wrapper 컴포넌트 구현
- 사용자의 디바이스 너비에 따라서 동적으로 토스트 UI의 너비를 조절하기 위해 padding, width 속성 사용
- 동일한 피드백을 전달하는 토스트가 이미 렌더링중이라면 추가적으로 렌더링되지 않도록 처리
ae6f6f6
to
87c6600
Compare
* chore: 체크 svg를 재사용할 수 있도록 current 속성으로 변경 * chore: 경고 토스트를 보여줄 때, 사용할 svg 추가 * chore: 필요없는 공백 제거 * feat: 토스트 컴포넌트 구현 - 토스트의 타입에 따라서 아이콘, 아이콘의 배경색을 동적으로 결정할 수 있도록 구현 * design: 토스트 컴포넌트를 스타일링하기 위한 css 추가 * test: 토스트 컴포넌트 UI 테스트 * feat: 토스트를 감싸는 컴포넌트 구현 - 토스트 컴포넌트 UI 테스트를 쉽게할 수 있도록, 토스트가 사라질 때 애니메이션을 적용하기 위한 상태를 관리하는 Wrapper 컴포넌트 구현 * feat: 여러개의 토스트를 보여주기 위한 컴포넌트 구현 * design: 토스트 목록 컴포넌트 스타일링을 위한 css 추가 - 사용자의 디바이스 너비에 따라서 동적으로 토스트 UI의 너비를 조절하기 위해 padding, width 속성 사용 * feat: 전역으로 토스트의 렌더링 상태를 관리하기 위한 컨텍스트, 프로바이더 컴포넌트 구현 - 동일한 피드백을 전달하는 토스트가 이미 렌더링중이라면 추가적으로 렌더링되지 않도록 처리 * test: 토스트 컴포넌트 폴더 구조 이동, 공통 스타일 추출, 버튼을 클릭했을 때 토스트 UI가 렌더링되는 스토리 추가 * chore: 토스트 UI 기본 시간 추가
관련 이슈
resolves: [FE] 사용자의 행동에 피드백을 주기 위한 토스트 UI를 구현해요 #387
사용자에게 피드백을 전달해줄 수 있는 토스트 UI를 구현했습니다.
작업 내용
디자인
모모 서비스에서 토스트 UI를 사용할 때, 위 이미지처럼 3가지 상황에서 사용할 수 있습니다. (성공, 경고, 기본)
위 토스트 UI 디자인을 끝낸 후, 아래와 같은 고민들을 하며 토스트 UI를 구현했습니다.
Toast.tsx
컴포넌트는 최대한 UI와 관련된 책임을 가지도록토스트 컴포넌트는 최대한 UI만 그리는 책임을 가지도록 구현했습니다. 그 이유는 아래와 같습니다.
스토리북으로 테스트하기 용이
하나의 책임만 가지니, 가독성과 유지보수 측면에서도 좋음
리액트 컴포넌트는 언마운트되었을 때, 바로 DOM 트리에서 사라지므로 사라지는 애니메이션을 적용하기 힘들다, 어떻게 해결할 것인가?
토스트 컴포넌트가 최대한 UI와 관련된 책임만 가지도록 구현한 후, 사라지는 애니메이션에 대한 책임을 어떤 레이어에서 가질 것인가? 에 대한 고민을 추가로 하게 되었습니다.
ToastProvider
,ToastList
컴포넌트에서도 충분히 책임을 가질 수 있지만 결론부터 말씀드리자면ToastContainer
컴포넌트를 추가로 구현하게 되었습니다. 그 이유는 아래와 같습니다.위 이유들에서도 확인할 수 있듯, 두 컴포넌트는 이미 책임을 충분히 가지고 있으므로 추가로 부여하기 보다는 토스트 컴포넌트를 한 번 더 감싸는
ToastContainer
컴포넌트를 구현하는 것이 좋을 것 같다고 판단했어요. 이 부분에 대한 논의는 언제든지 환영입니다 :)여러개의 토스트 메시지를 보여주는 것으로 결정, 단 메시지가 같은 경우에는 해당 토스트 UI 렌더링을 무시할 수 있도록 한다.
여러개의 토스트 메시지를 보여줄 것인지에 대해서 논의를 했었는데요, 여러개의 토스트 메시지를 보여줄 수 있도록 구현하되 동일한 피드백을 전달해 주고 있는 토스트가 렌더링 중이라면, 추가로 렌더링되지 않는 방향으로 구현했습니다.
모모 서비스에서 한 번에 하나의 토스트만 보여준다면 여러 피드백을 동시에 전달해야 하는 경우에는 대처할 수 없다고 생각한게 가장 큰 이유입니다. 동일한 메시지를 전달하는 토스트 컴포넌트를 계속해서 렌더링한다면 토스트 컴포넌트가 계속 쌓여서 UX 측면에서 좋지않다고 판단한 것도 의사 결정 요소입니다 :)
특이 사항
결국, 해결하지 못한 렌더링 효율 문제
토스트 렌더링을 담당하는 컨텍스트(ToastStateContext), 토스트 렌더링을 트리거하는 컨텍스트(ToastDispatchContext)를 분리해서 렌더링 효율 문제를 해결하려고 했으나 해결하지 못한 채 PR을 보냅니다. 해결하지 못한 이유는 렌더링을 트리거하는 addToast 함수에서 현재 렌더링된 토스트의 상태(ToastStateContext)를 참조하기 때문입니다.
위 구현을 살펴보면 addToast 함수에서 이미 렌더링된 토스트인지 확인 절차를 거칩니다. 이 때 현재 렌더링된 토스트들의 상태를 참조하고 있기 때문에 컨텍스트를 분리할 수 없어 렌더링 효율 문제를 해결할 수 없었습니다...:( 슬프네요
모달, 토스트가 사라질 때 애니메이션을 적용하는 상태관리 커스텀 훅?
토스트 컴포넌트를 구현하기 전만해도 useAnimation이라는 커스텀 훅을 구현하고 모달, 토스트가 사라질 때 애니메이션을 적용할 수 있도록 하는 책임을 구분하려고 했는데요...결론적으로 구현하지는 못했습니다.
모달은 사용자가 특정 행동을 할 때, 사라지고 토스트는 특정 시간이 지나면 자동으로 사라지는데 이 두 상황을 모두 커버할 수 있는 커스텀 훅을 구현하기 위한 아이디어가 떠오르지 않아 너무 많은 시간이 지체될까바 일단 PR을 보냅니다.
리뷰 요구사항 (선택)