-
Notifications
You must be signed in to change notification settings - Fork 5
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] api에러 등의 에러를 잡아 핸들링 #232
Changes from 10 commits
b346179
db2dffa
743547f
a5a826c
ec1a0e2
f0cd278
714fc3e
d4363ee
bee6989
94fb5a8
2ab518b
050172a
88e628a
4af8344
4608e68
4d4ea35
eaf7584
92b3665
f002c57
2652c2e
c3bb543
8431796
7e0478f
2b35ee0
fe2be35
252fa01
9593155
dadc181
20eface
c65f6e7
559ccc3
a45f845
d12979a
4130ecd
892dbb0
eded4da
322d21c
5beb45a
87aae32
99747d5
79cd93a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
import ERROR_MESSAGES from '@constants/errorMessage'; | ||
import React, {createContext, useState, useContext, ReactNode} from 'react'; | ||
|
||
// 에러 컨텍스트 생성 | ||
interface ErrorContextType { | ||
hasError: boolean; | ||
errorMessage: string; | ||
setError: (error: ServerError) => void; | ||
clearError: () => void; | ||
} | ||
|
||
const ErrorContext = createContext<ErrorContextType | undefined>(undefined); | ||
|
||
// 에러 컨텍스트를 제공하는 프로바이더 컴포넌트 | ||
interface ErrorProviderProps { | ||
children: ReactNode; | ||
} | ||
|
||
type ServerError = { | ||
code: string; | ||
message: string; | ||
}; | ||
|
||
export const ErrorProvider: React.FC<ErrorProviderProps> = ({children}) => { | ||
const [hasError, setHasError] = useState(false); | ||
const [errorMessage, setErrorMessage] = useState(''); | ||
|
||
const setError = (error: ServerError) => { | ||
setHasError(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요것은 에러를 초기화해주는 역할인가요? true에서 false로 바꿔주기 위해 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 이 코멘트를 읽다가 갑자기 생각난 의견입니다. 그렇다면 이에 대해서 에러 토스트가 페이지를 모두 덮을 정도로 무수히 많아진다면?이 떠오르는데요. 에러 토스트 스택에 높이 제한을 주는 것은 어떨지 생각이 드네용.. 근데 이건 정말 제안이라서.. 무시하셔도 됩니댱 |
||
setErrorMessage(''); | ||
|
||
setTimeout(() => { | ||
setHasError(true); | ||
setErrorMessage(ERROR_MESSAGES[error.code] ?? '지금은 에러 코드가 안바뀌어서 에러 메세지가 없어요.'); | ||
}, 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. setTimeout을 쓴 이유가 궁금해요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. setter겹치면 제일 마지막 세터만 실행되는 줄 알았는데 setTimeout이 없어도 둘 다 실행되네요. 새로이 알아갑니다. 감사해요 |
||
}; | ||
|
||
const clearError = () => { | ||
setHasError(false); | ||
setErrorMessage(''); | ||
}; | ||
|
||
return ( | ||
<ErrorContext.Provider value={{hasError, errorMessage, setError, clearError}}>{children}</ErrorContext.Provider> | ||
); | ||
}; | ||
|
||
// 에러 컨텍스트를 사용하는 커스텀 훅 | ||
export const useError = (): ErrorContextType => { | ||
const context = useContext(ErrorContext); | ||
if (!context) { | ||
throw new Error('useError must be used within an ErrorProvider'); | ||
} | ||
return context; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import {useError} from '../ErrorProvider'; | ||
import {useState} from 'react'; | ||
|
||
interface FetchOptions<T> { | ||
queryFn: () => Promise<T>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 리액트쿼리와 유사해서 이름을 그대로 가져갔었어요. 지금은 queryFunction으로 바꿔보았습니다!! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아하 리액트쿼리를 미션에서 딱 한번 써본적밖에 없어서 몰랐네요 ㅜ |
||
} | ||
|
||
export const useFetch = () => { | ||
const {setError, clearError} = useError(); | ||
const [loading, setLoading] = useState(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. boolean 변수다 보니 isLoading, setIsLoading이 더 어울리지 않을까 싶어요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오 맞네요. 바로 반영하였습니다! |
||
const [data, setData] = useState<any>(null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a...n...y.... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ㅋㅋㅋㅜㅜ 바로 지웠습니다. |
||
|
||
const request = async <T,>({queryFn}: FetchOptions<T>): Promise<T | null> => { | ||
setLoading(true); | ||
clearError(); | ||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://haengdong.pro/event/{eventId}/home 주소로 접속하면, eventName을 불러오는 요청과, eventLog를 불러오는 요청이 두개가 동시에 날라가게되는데요, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. �고려해보진 않은 상황인데요. 무엇이 맞는진 잘 모르겠지만 여러 개의 api가 모두 에러가 나고 있다면 결국 두 api모두 유사한 이유이지 않을까요.. ? 네트워크라던지 인증이라던지, eventId가 올바르지 않는다던지.. 어찌됐든 여러 개가 오류난다면 정상은 아니니 그냥 에러 바운더리로 보내버리는 것도 방법이겠습니다..... 정리하자면 제 생각안에서는 아래 방법들이 그나마 가능성 있어보이는데요.
토다리는 무엇이 더 적합한 방법이라고 생각하시나요? 이 방법 말고도 좋은게 있다면 지식을 나누어 주세요..! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 말한 상황은, 두가지 모두 오류가 나는 상황이 아니라 두가지 요청을 보내는 상황에 clearError()으로 인해서 이전의 에러가 지워지지 않는가 물어본거였습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 두 가지 요청을 보내는 상황에 이전의 에러가 지워진다면 이후 에러도 있다는 뜻이니 두 개가 에러가 난다는 뜻이 아닌가요?! |
||
const result = await queryFn(); | ||
setData(result); | ||
return result; | ||
} catch (error) { | ||
if (error instanceof Error) { | ||
const errorBody = await JSON.parse(error.message); | ||
setError(errorBody); | ||
console.log(error); | ||
} else { | ||
setError(new Error('An unknown error occurred')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 궁금해서 물어보는 건데, catch에서 잡는 error가 Error의 instance 가 아닌 경우도 있나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. promise 같은 객체를 throw 해버리는 case도 있는 것 같은데, 만약 promise를 throw했다면 이 부분은 unkown error가 되는 것인가 싶어서... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 없을리 없다고 생각합니다. 에러를 개발자가 잡아서 명확한 이름으로 던지지 않는다면 그게 언노운에러니까요..! 물론 저 분기문은 타입 에러때문에 쓴 것입니다. 언노운 에러에는 message가 없을 수 있습니다. 프로미스를 던지는 경우는 잘 모르겠는데 아니라면 혹시 예시가 있을까요? 그리고 프로미스를 던져야하는 이유가 있을까요?! 이런 쪽으로는 생각해보지 못해서요. 좋은 아이디어라면 공유해주세용 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 간단한 예시로, Suspense의 내부동작에서 promise를 던지고, 이 결과를 잡아서 처리하는 것으로 알고 있습니당 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 예를들면 {status: 400, errorMessage: ~~~} 와 같이 던지는 경우같은걸 말하는거죠? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아 서스펜스가 던지는 에러가 프로미스 리젝트 에러라는 말씀이실까요?!?! |
||
} | ||
return null; | ||
} finally { | ||
setLoading(false); | ||
} | ||
}; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 아마 미래에는 캐싱이 필요해질 수도 있을 것 같지만.. 지금은 너무 섣부를 것 같아 따라만 해봤습니다. |
||
return {data, loading, request}; | ||
}; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,67 @@ | ||||||||||||||||||||||||||||||||||
import {css, keyframes} from '@emotion/react'; | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. emotion에 이런 기능이 있었군요~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도몰랐습니다... 신피티... |
||||||||||||||||||||||||||||||||||
import {ToastPosition} from './Toast.type'; | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
type ToastMarginStyle = { | ||||||||||||||||||||||||||||||||||
position?: ToastPosition; | ||||||||||||||||||||||||||||||||||
bottom?: string; | ||||||||||||||||||||||||||||||||||
top?: string; | ||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||
Comment on lines
+5
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이런 부분들은 렌더링 상태와 엮어 useFixedLayout과 같은 hook으로 만들어 준다면, 다양한 곳에서 사용해 볼 수도 있겠네요! There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. css layout을 바꿔주는 친구들인데 |
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
// 애니메이션 키프레임 정의 | ||||||||||||||||||||||||||||||||||
const fadeIn = keyframes` | ||||||||||||||||||||||||||||||||||
from { | ||||||||||||||||||||||||||||||||||
opacity: 0; | ||||||||||||||||||||||||||||||||||
transform: translateY(20px); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
to { | ||||||||||||||||||||||||||||||||||
opacity: 1; | ||||||||||||||||||||||||||||||||||
transform: translateY(0); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
`; | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
const fadeOut = keyframes` | ||||||||||||||||||||||||||||||||||
from { | ||||||||||||||||||||||||||||||||||
opacity: 1; | ||||||||||||||||||||||||||||||||||
transform: translateY(0); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
to { | ||||||||||||||||||||||||||||||||||
opacity: 0; | ||||||||||||||||||||||||||||||||||
transform: translateY(20px); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
`; | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fadeIn / fadeOut 이름의 키프레임이, fade 작업만이 아니라, transform에 대한 변경사항도 포함하고 있으니, 키프레임 이름도 이를 포함하게 작성해 주는 것이 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 좋은 아이디어네용. 바로 반영했습니다~!!!! |
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
export const toastMarginStyle = ({position, bottom, top}: ToastMarginStyle) => | ||||||||||||||||||||||||||||||||||
css({ | ||||||||||||||||||||||||||||||||||
position: 'absolute', | ||||||||||||||||||||||||||||||||||
bottom: position === 'bottom' ? `${bottom}` : 'auto', | ||||||||||||||||||||||||||||||||||
top: position === 'top' ? `${top}` : 'auto', | ||||||||||||||||||||||||||||||||||
left: '50%', | ||||||||||||||||||||||||||||||||||
transform: 'translate(-50%)', | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
width: '100%', | ||||||||||||||||||||||||||||||||||
maxWidth: '48rem', | ||||||||||||||||||||||||||||||||||
paddingInline: '1rem', | ||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
export const toastStyle = (isVisible: boolean) => | ||||||||||||||||||||||||||||||||||
css({ | ||||||||||||||||||||||||||||||||||
width: '100%', | ||||||||||||||||||||||||||||||||||
padding: '0.625rem 1rem', | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
backgroundColor: 'gray', | ||||||||||||||||||||||||||||||||||
boxShadow: '0 8px 12px rgba(0, 0, 0, 0.16);', | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 전부 rem 단위를 사용하고 있어서 rem으로 통일해서 작성해주는게 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 엌.... 이거 그러면 Hdesign도 수정해야할 것 같습니다. 일단 클라이언트에있는건 수정했어요! |
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
borderRadius: '1.25rem', | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
// 애니메이션 추가 | ||||||||||||||||||||||||||||||||||
animation: `${isVisible ? fadeIn : fadeOut} 0.5s forwards`, | ||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
export const textStyle = () => | ||||||||||||||||||||||||||||||||||
css({ | ||||||||||||||||||||||||||||||||||
width: '100%', | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
color: 'white', | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
whiteSpace: 'pre-line', | ||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prop을 받지 않는 css는 함수로 만들어주지 않아도 간단할 것 같아요~ 이름 자체가 get~~와 같은 함수형 네이밍이 아니기도 하고... ㅎㅎ
Suggested change
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,79 @@ | ||||||||||||||||||||||||||||||||||||
/** @jsxImportSource @emotion/react */ | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. HDesign에서 그대로 복사해오느라 남아있었네요. 바로 지웠습니다! 꼼꼼한 확인 고마워요~~!!! |
||||||||||||||||||||||||||||||||||||
import {createPortal} from 'react-dom'; | ||||||||||||||||||||||||||||||||||||
import {useState, useEffect} from 'react'; | ||||||||||||||||||||||||||||||||||||
import {toastStyle, textStyle, toastMarginStyle} from './Toast.style'; | ||||||||||||||||||||||||||||||||||||
import {ToastProps, ToastType} from './Toast.type'; | ||||||||||||||||||||||||||||||||||||
import {Button, Flex, Icon, Text} from 'haengdong-design'; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const renderIcon = (type: ToastType) => { | ||||||||||||||||||||||||||||||||||||
switch (type) { | ||||||||||||||||||||||||||||||||||||
case 'error': | ||||||||||||||||||||||||||||||||||||
return <Icon iconType="error" />; | ||||||||||||||||||||||||||||||||||||
case 'confirm': | ||||||||||||||||||||||||||||||||||||
return <Icon iconType="confirm" />; | ||||||||||||||||||||||||||||||||||||
case 'none': | ||||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||
default: | ||||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Toast type은 error, confirm, none 세가지 밖에 없는데, 같은 null을 반환하는 none과 한번도 걸러지지 않을 default의 case를 분리한 이유가 있을까요?! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Toast를 수정해서 사용하려고 복붙해오게 되었는데요. Hdesign의 Toast는 지노킴씨가 작업하셨으니 쿠키씨에게 여쭤볼게요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지노킴이 대답할게요! 이거 개발환경 중에 그럴일은 없겠지만 error, confirm, none 이외의 다른 문자가 들어갔을 때 오류를 방지하기 위해 이렇게 했어요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 개발환경이면 애초에 vsc가 걸러줄 것 같긴 한데, 확인했습니당~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'none'과 default의 return이 null로 동일하기 때문에 case 'none'에 대한 부분은 없어도 괜찮지 않을까 싶어요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아.. Hdesign에서 복붙해온거라 아마 거기에도 쓰여있을 거에요. 일단 client에서는 지웠습니다! |
||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아래와 같이 작성할 수도 있겠네요~!
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 엇.. 토스트 피드백은 전부 클라이언트에만 반영하겠습니다. |
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const Toast = ({ | ||||||||||||||||||||||||||||||||||||
type = 'confirm', | ||||||||||||||||||||||||||||||||||||
top = '0px', | ||||||||||||||||||||||||||||||||||||
bottom = '0px', | ||||||||||||||||||||||||||||||||||||
isClickToClose = true, | ||||||||||||||||||||||||||||||||||||
position = 'bottom', | ||||||||||||||||||||||||||||||||||||
message, | ||||||||||||||||||||||||||||||||||||
onUndo, | ||||||||||||||||||||||||||||||||||||
onClose, | ||||||||||||||||||||||||||||||||||||
...htmlProps | ||||||||||||||||||||||||||||||||||||
}: ToastProps) => { | ||||||||||||||||||||||||||||||||||||
const [isVisible, setIsVisible] = useState(true); | ||||||||||||||||||||||||||||||||||||
const styleProps = {position, top, bottom}; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
useEffect(() => { | ||||||||||||||||||||||||||||||||||||
const timer = setTimeout(() => { | ||||||||||||||||||||||||||||||||||||
setIsVisible(false); | ||||||||||||||||||||||||||||||||||||
setTimeout(() => { | ||||||||||||||||||||||||||||||||||||
if (onClose) onClose(); | ||||||||||||||||||||||||||||||||||||
}, 500); // fadeOut 애니메이션 시간과 동일하게 설정 | ||||||||||||||||||||||||||||||||||||
}, 3000); // 토스트가 화면에 보이는 시간 설정 | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 이게 어떤 말씀인지 잘 모르겠습니다.. 혹시 더 구체적으로 설명해주실 수 있나요? 일단 지금은 아래와 같이 바꿔두었습니다. }, ANIMATION_TIME); // fadeOut 애니메이션 시간과 동일하게 설정
}, showingTime - ANIMATION_TIME); // 토스트가 내려가는 시간 확보 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 이런 시간 설정 매직넘버도 constants로 만들어도 좋을 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오 바로 반영했습니다. |
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
return () => { | ||||||||||||||||||||||||||||||||||||
clearTimeout(timer); | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
}, [onClose]); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const handleClickToClose = () => { | ||||||||||||||||||||||||||||||||||||
if (!isClickToClose || !onClose) return; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
setIsVisible(false); | ||||||||||||||||||||||||||||||||||||
setTimeout(() => { | ||||||||||||||||||||||||||||||||||||
onClose(); | ||||||||||||||||||||||||||||||||||||
}, 500); // fadeOut 애니메이션 시간과 동일하게 설정 | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
return createPortal( | ||||||||||||||||||||||||||||||||||||
<div css={toastMarginStyle({...styleProps})} {...htmlProps} onClick={handleClickToClose}> | ||||||||||||||||||||||||||||||||||||
<div css={toastStyle(isVisible)}> | ||||||||||||||||||||||||||||||||||||
<Flex justifyContent="spaceBetween" alignItems="center"> | ||||||||||||||||||||||||||||||||||||
<Flex alignItems="center" gap="0.5rem"> | ||||||||||||||||||||||||||||||||||||
{renderIcon(type)} | ||||||||||||||||||||||||||||||||||||
<Text size="smallBodyBold" css={textStyle()}> | ||||||||||||||||||||||||||||||||||||
{message} | ||||||||||||||||||||||||||||||||||||
</Text> | ||||||||||||||||||||||||||||||||||||
</Flex> | ||||||||||||||||||||||||||||||||||||
{onUndo && ( | ||||||||||||||||||||||||||||||||||||
<Button variants="tertiary" size="small" onClick={onUndo}> | ||||||||||||||||||||||||||||||||||||
되돌리기 | ||||||||||||||||||||||||||||||||||||
</Button> | ||||||||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||||||||
</Flex> | ||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||
</div>, | ||||||||||||||||||||||||||||||||||||
document.body, | ||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export default Toast; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
export type ToastPosition = 'bottom' | 'top'; | ||
export type ToastType = 'error' | 'confirm' | 'none'; | ||
|
||
export interface ToastStyleProps { | ||
bottom?: string; | ||
top?: string; | ||
} | ||
|
||
export interface ToastOptionProps { | ||
position?: ToastPosition; | ||
type?: ToastType; | ||
onUndo?: () => void; | ||
isClickToClose?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isClickToClose는 무엇을 하는 메소드 명인지 명확하지 않은 것 같아요.
1번이라면, canClickToClose There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. boolean을 나타낼 떄 is를 붙이라는게 모든 상황에서 is를 붙이라는건 아닌걸로 알고 있어요.
상태를 나타내는 형용사가 아닌 동사나 명사형을 쓰는경우라면, can, have 등이 가능할 것 같네요.
정 is를 쓰고 싶다면,
정도로 바꿔쓰면 어떨까 싶어요! There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 이것도 마찬가지로 라이브러리에서 긁어온것인데요. 아마 토스트사용하는 방식을 좀 바꿀 것 같습니다. 프로바이더를 사용하지 않는 방식으로요! 그래서 이슈를 만들어 두었는데 이 곳에 할 일로 적어뒀습니다. |
||
onClose?: () => void; | ||
} | ||
|
||
export interface ToastRequiredProps { | ||
message: string; | ||
} | ||
|
||
export type ToastProps = React.ComponentProps<'div'> & ToastStyleProps & ToastOptionProps & ToastRequiredProps; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
/** @jsxImportSource @emotion/react */ | ||
import {createContext, useCallback, useContext, useEffect, useState} from 'react'; | ||
import {ToastProps} from './Toast.type'; | ||
import Toast from './Toast'; | ||
import {useError} from '../../ErrorProvider'; | ||
|
||
export const ToastContext = createContext<ToastContextProps | null>(null); | ||
|
||
interface ToastContextProps { | ||
showToast: (args: ShowToast) => void; | ||
} | ||
|
||
type ShowToast = ToastProps & { | ||
showingTime?: number; | ||
isAlwaysOn?: boolean; | ||
}; | ||
|
||
const ToastProvider = ({children}: React.PropsWithChildren) => { | ||
const [currentToast, setCurrentToast] = useState<ShowToast | null>(null); | ||
const {hasError, errorMessage, clearError} = useError(); | ||
|
||
const showToast = useCallback(({showingTime = 3000, isAlwaysOn = false, ...toastProps}: ShowToast) => { | ||
setCurrentToast({showingTime, isAlwaysOn, ...toastProps}); | ||
}, []); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. toast에 영향을 미치는 prop들을 useCallback의 dependancy array에 추가해서 값이 변경되면 memo된 값을 변경시켜주지 않아도 되나요?! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. useCallback아마 지피티한테 밥줘서 생긴 것 같습니다. 사용할 의도가 전혀 없었어서 지금은 지웠습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 밥줘서 ㅋㅋㅋㅋㅋㅋㅋㅋㅋ |
||
|
||
const closeToast = () => { | ||
setCurrentToast(null); | ||
}; | ||
|
||
useEffect(() => { | ||
if (hasError) { | ||
showToast({ | ||
message: errorMessage || 'An error occurred', | ||
showingTime: 5000, // 필요에 따라 시간 설정 | ||
isAlwaysOn: false, | ||
position: 'bottom', | ||
bottom: '100px', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 마찬가지로 rem단위면 좋을 것 같습니다~! |
||
}); | ||
|
||
const timer = setTimeout(() => { | ||
clearError(); | ||
}, 5000); // 토스트가 표시되는 시간과 동일하게 설정 | ||
|
||
return () => clearTimeout(timer); | ||
} | ||
|
||
return; | ||
}, [hasError, errorMessage, showToast, clearError]); | ||
|
||
useEffect(() => { | ||
if (!currentToast) return; | ||
|
||
if (!currentToast.isAlwaysOn) { | ||
const timer = setTimeout(() => { | ||
setCurrentToast(null); | ||
}, currentToast.showingTime); | ||
|
||
return () => clearTimeout(timer); | ||
} | ||
|
||
return; | ||
}, [currentToast]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기 useEffect를 나누어서 두 번 적은 이유가 궁금합니다~ 그리고 나중 이야기지만 Provider에 isAlwaysOn, showingTime 설정할 수 있도록 열어줘도 좋지 않을까 살짝 의견!? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. showingTime이랑 isAlwaysOn이 지금은 5초랑 false로 고정이 되어있는데, 이를 변경하고 싶다고 할 때 외부에서 값을 넣어주면 조정하기 쉬울 것 같아서. 마치 아래 queryClient의 staleTime처럼 const queryClient = new QueryClient({ defaultOptions:
{ queries:
{ staleTime: Infinity,
},
},
}) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. provider의 value로 넣어서 children에서 useToast로 사용할 수 있게 하자는 얘기이지 않나 싶어요~ |
||
|
||
return ( | ||
<ToastContext.Provider value={{showToast}}> | ||
{currentToast && <Toast onClose={closeToast} {...currentToast} />} | ||
{children} | ||
</ToastContext.Provider> | ||
); | ||
}; | ||
|
||
const useToast = () => { | ||
const context = useContext(ToastContext); | ||
|
||
if (!context) { | ||
throw new Error('useToast는 ToastProvider 내에서 사용되어야 합니다.'); | ||
} | ||
|
||
return context; | ||
}; | ||
|
||
export {ToastProvider, useToast}; |
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.
혹시 react-simple-toasts는 어떤 것인가요??!!?
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.
아 .. 토스트가 행동 라이브러리에 묶여있어서 조작이 불가능해 임시로 쓰려고 가져왔던건데요. 그냥 행동 라이브러리 토스트 폴더 그대로 복붙해와서 사용하는걸로 바꿔서 지금은 사용 안합니다! 패키지에선 지웠는데 코드엔 남아있었네요. 꼼꼼한 확인 고마워영~~!!!