Feat/#59 토스트 컴포넌트 구현 #73
Conversation
간단한 reset css 추가
1. 토스트 context 생성 및 provider 구현 2. 토스트 context 커스텀 훅 구현
토스트 위치 조절을 위한 ToastList 컴포넌트 구현
There was a problem hiding this comment.
👍👍
토스트 이쁘게 구현 잘되었네요!!
window가 빠진 부분 외에 큰 문제는 없는 것 같아요~
두 가지 의견만 추가로 남길게요!
💬 useToastContext를 이용해서 사용하는 것이라면 네이밍을 useToast라고 해도 괜찮을 것 같아요.
// @/components/@common/Toast/index.ts
export { default as useToast } from './hooks/useToastContext';
// 사용처
import { useToast } from '@/components/@common/Toast';💬 showToast와 hideToast가 덜 분리 된 것 같아요.
toast를 띄울 때, toast가 사라질 때의 행동을 조금 더 구분해주면 좋을 것 같아요.
const showToast = useCallback((message: string) => {
if (toastTimer.current !== null) return;
setIsToastShow(true);
setMessage(message);
const timer = window.setTimeout(hideToast, 2000);
toastTimer.current = timer;
}, []);
const hideToast = useCallback(() => {
if (toastTimer.current === null) return;
setIsToastShow(false);
setMessage('');
window.clearTimeout(toastTimer.current);
toastTimer.current = null;
}, []);| import { styled } from 'styled-components'; | ||
| import type { PropsWithChildren } from 'react'; | ||
|
|
||
| const ToastList = ({ children }: PropsWithChildren) => { |
There was a problem hiding this comment.
💬 토스트 리스트의 용도가 궁금해요! 😯
There was a problem hiding this comment.
리스트 컴포넌트는 현재 토스트 컴포넌트의 위치를 조정하는 역할을 합니다!
(리스트 컴포넌트가 absolute 속성을 가지게 했어요!)
또 이후에 토스트가 메세지가 여러개 올라와야한다는 요구사항도 충분히 생길 수 있다고 생각해서
추후 확장성을 생각해 만들게 되었어요.
만약 확장한다면, toast 객체를 가지고 이를 순차적으로 랜더링 하는 역할로 사용될거에요
정리하면,
(여러개의)토스트가 랜더링 될 위치를 지정하고
토스트의 랜더링을 담당할 컴포넌트 입니다.
There was a problem hiding this comment.
도밥 너무 고생 많았어요. 오랜만에 context API를 통해 시도한 점 멋집니다!
궁금한 것들이 있는데요.
- ToastProvider는 전역에서 한 번만 사용할 수 있는 컴포넌트일까요?
- Storybook에는 Toast 컴포넌트로 테스트하셨는데, Toast 컴포넌트만 독립적으로 사용해도 되는 것일까요?
개인적으로 네이밍만 봐서는 어떤 역할을 하는 것인지 확인하기 힘들었어요.
- useToastContext는 호출하면 showToast를 반환하잖아요. 그러면 useShowToast나 useTriggerToast가 더 직관적이지 않을까요?
- ToastList는 커멘트로 남겼습니다.
- useToastContext의 반환 값이
ToastContextProps타입인데, 타입만 봐서는 어떤 값인지 예측하기가 힘들었어요.
| import { ToastContext } from '../ToastProvider'; | ||
|
|
||
| const useToastContext = () => { | ||
| const value = useContext(ToastContext); |
There was a problem hiding this comment.
value가 어떤 값인지 한눈에 확인하기 어려운 것 같아요. 해당 값이 무엇인지 알려면 ToastContext까지 들어가야 알 것 같습니다.
There was a problem hiding this comment.
코멘트를 보고 고민을 해봤는데요!
커스텀 훅인데 return type이 바로 보이지 않는게 다소 어색하긴 하네요
한편으로는 이 부분은 useContext를 사용한다면 언제든 있을 수 있는 불편함 같아요..!
interface가 무엇인지 눌러보는 방법외에 바로 타입을 확인할 수 있는 방법이 있을까요?
따로 대화해보고 싶네요!
|
|
||
| const useToastContext = () => { | ||
| const value = useContext(ToastContext); | ||
| if (!value) throw new Error('토스트 컨텍스트에 제공되는 값이 없습니다.'); |
There was a problem hiding this comment.
💬 '토스트 컨텍스트에 제공되는 값이 없습니다.' 라는 말이 context provider에 value를 넣지 않았을 때 인가요? -> 그러면 toastProvider을 사용자가 다른 곳에서도 사용할 수 있게끔 설계를 하신 걸까요?
There was a problem hiding this comment.
- context provider에 value를 넣지 않았을 때 인가요?
네 맞습니다! 아래 그림의 type 처럼 showToast를 제공해야하는데, 개발자가 실수했을 경우를 대비한 ERROR에요!
null 유니온 타입을 사용하지 않고, showToast Type을 만족하는 기본함수로 초기화 해도 되지만,
이 방식보다는 null을 할당하고 에러를 띄워서 개발자의 실수를 잡아주고 싶었어요.
실수로 provider를 씌워주지 않거나, provider 영역 밖에서 사용하는 경우 에러가 발생하겠죠!
- toastProvider을 사용자가 다른 곳에서도 사용할 수 있게끔 설계를 하신 걸까요?
네 맞습니다!
코드에서는 전역적으로 감싸두었지만, 이후 언제든지 지역적으로 사용할 수 있습니다.
그런 의미에서는 제가 아예 전역에 감싸두지 않고 설명을 남겨놓는 것이 좋았겠네요!
이 부분 반영하도록 할게요!
| @@ -0,0 +1,25 @@ | |||
| import { keyframes } from 'styled-components'; | |||
|
|
|||
| export const fadeInBottomToUp = keyframes` | |||
There was a problem hiding this comment.
💬 재사용을 위해서 파일로 분리하신 걸까요?
There was a problem hiding this comment.
키프레임 코드가 워낙 길다보니 컴포넌트 내에 두기엔 가독성이 좋지 않았습니다.
애니메이션의 동작을 변수로 설명하고 가독성도 챙기고 싶어서 파일로 분리했어요!
초기에는 키프레임 재사용이 가능할거라고 생각해서 src/styles 디렉터리에 분리했었는데,
토스트 컴포넌트 외에는 적용하기 힘들어 보여서 토스트 관련 디렉터리에 두었습니다!
| const ToastList = ({ children }: PropsWithChildren) => { | ||
| return <Position>{children}</Position>; | ||
| }; | ||
|
|
||
| export default ToastList; | ||
|
|
||
| const Position = styled.div` | ||
| position: fixed; | ||
| bottom: 10%; | ||
| left: 50%; | ||
| transform: translate(-50%, 0); | ||
| `; |
There was a problem hiding this comment.
| const ToastList = ({ children }: PropsWithChildren) => { | |
| return <Position>{children}</Position>; | |
| }; | |
| export default ToastList; | |
| const Position = styled.div` | |
| position: fixed; | |
| bottom: 10%; | |
| left: 50%; | |
| transform: translate(-50%, 0); | |
| `; | |
| const ToastList = styled.div` | |
| position: fixed; | |
| bottom: 10%; | |
| left: 50%; | |
| transform: translate(-50%, 0); | |
| `; | |
| export default ToastList |
- 💬 위와 아래에 차이가 있는 것일까요? 스타일만 있는 컴포넌트인 것 같은데 따로 react component로 선언한 이유가 궁금해요.
- ToastList라고 하면 여러 Toast가 띄워지는 ui로 생각이 드는데, 나중에 구현할 것을 생각하고 이름을 지으신 걸까요?
There was a problem hiding this comment.
-
개인적으로는 위쪽을 선호합니다~ 추후 컨벤션에 따라 styled.tsx 파일로 분리될 로직이라 따로 이야기 나눠보죠!
또ts vs tsx도 한번 이야기 나눠보고 싶네요! -
맞습니다~! 코난 리뷰에 이에 대한 자세한 답변을 해서 링크 남길게요!
Feat/#59 토스트 컴포넌트 구현 #73 (comment)
| import type { PropsWithChildren } from 'react'; | ||
|
|
||
| interface ToastContextProps { | ||
| showToast: (message: string) => void; | ||
| } | ||
|
|
||
| export const ToastContext = createContext<null | ToastContextProps>(null); | ||
|
|
||
| const ToastProvider = ({ children }: PropsWithChildren) => { | ||
| const [isToastShow, setIsToastShow] = useState(false); | ||
| const [message, setMessage] = useState(''); | ||
| const toastTimer = useRef<NodeJS.Timeout>(); |
There was a problem hiding this comment.
👍 와우 NodeJs.Timeout 타입인 것을 처음 알았네요. 그리고 timer을 useRef로 사용한 점 배워갑니다.
💬 createContext는 기본값으로 null을 주었는데 useRef는 인자를 비워둔 이유가 있을까요?
There was a problem hiding this comment.
어헛 다행입니다
이 부분 코난이 정정해줬는데, 우코에게 바로 메타인지 들어갈게요
우선 해당 Ref의 타입은 잘못되었어요.
제가 작성한 코드가 실행될 런타임은 node.js 가 아닌 브라우저입니다.
setTimeout 대신 window.setTimeout 으로 명시함으로서 브라우저에서 실핼될 코드임을 알려줄 수 있습니다.
TS가 이 부분을 혼동하지 않게하는 역할도 있구요
때문에 해당 Ref type은 현재 아래와 같이 변경되었습니다!
const toastTimer = useRef<number | null>(null);간단하고 명료하게 설명된 블로그 링크 첨부합니다.
1. toastTimer ref type 변경 2. timeout관련 메서드 window 객체에서 사용 3. Timer ref type 변경으로 인한 show/hide 메서드 내 로직 변경(null 처리 등) 4. setter 순서 의미에 맞게 변경
컴포넌트 제공자의 입장으로 전역에 씌워놓은 Provider태그는 혼동을 줄 수 있어 삭제함.
|
@cruelladevil @ukkodeveloper 🖐 우선 리뷰 감사합니다.ㅎㅎ 우코의 질문중 코멘트로 답변하지 못한 내용 여기에 남겨요!
이 부분은 의도하지 않았어요. 나머지 부분에 대한 씽크는 같이 맞춰보죠~ㅎㅎ 📜 리뷰 반영 요약표
이외 PR에 포함된 기능
|
📝작업 내용
💬리뷰 참고사항
Provider와 Context 구현으로 코드 전역에서 토스트를 사용할 수 있도록 했습니다.
showToast함수 하나만으로 선언적으로 사용할 수 있습니다.기본 글로벌 스타일을 정의하고 커밋했습니다.
svg와 같은 assets에 대한 설정이 없어서, 웹팩 설정이 필요해보입니다!
코드 전반적인 스타일이나, 의문을 모두 적어주시면 감사하겠습니다!
#️⃣연관된 이슈
close #59
close #82