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

[FE] fix : 리뷰 제출 실패 시, ErrorPage가 아닌 모달을 띄우기 #612

Merged
merged 9 commits into from
Sep 18, 2024

Conversation

BadaHertz52
Copy link
Contributor

@BadaHertz52 BadaHertz52 commented Sep 14, 2024


🚀 어떤 기능을 구현했나요 ?

현재 리뷰 제출 실패 오류 대응 문제점

현재 리뷰 제출 실패 시(=api response.ok가 false일 경우), CardForm을 감싸고 있는 QueryErrorResetBoundary이 실행되어서 ErrorPage가 뜹니다. ErrorPage 컴포넌트가 렌더링되면서 현재 CardForm 컴포넌트가 언마운트되어 카드 작성 recoil 상태가 모두 초기화돼요. 이로 인해 리뷰 제출 시 ErrorPage가 뜨는 것은 사용자가 작성한 리뷰들이 모두 날아가서 사용자들이 불편함을 겪을 수 있다고 생각했어요.

SubmitErrorModal 생성

QueryErrorResetBoundary fallback으로 SubmitErrorModal을 지정해, 리뷰 제출 실패 시 SubmitErrorModal이 뜨도록 했어요.

🔥 어떻게 해결했나요 ?

구현 모습

-.Clipchamp.mp4

📝 어떤 부분에 집중해서 리뷰해야 할까요?

📚 참고 자료, 할 말

  • ... 버그가 없을 순 없겠지
  • 즐추!!

return (
<AlertModal closeButton={closeButton} isClosableOnBackground={true} handleClose={handleClose}>
<S.Contents>
<S.AlertTriangle src={AlertTrianglePrimaryIcon} alt="경고 마크" />
<p>{errorText}</p>
{children}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ErrorAlertModal에서 모달 내 글들을 다양한 방식(여러 줄, 스타일 적용)으로 사용할 수 있도록, children을 받는 것으로 수정했어요.

const { resetFormRecoil } = useResetFormRecoil();

useEffect(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

오류

BreadCrumb에서 reviewRequestCodeAtom을 사용하고 있고 이전에는 리뷰 연결 페이지에서만 reviewRequestCodeAtom의 상태를 업데이트하고 있었어요. 만약 사용자가 리뷰 작성 페이지를 북마크나 클립보드로 복사했다가 들어오는 경우 BreadCrumb를 통해 연결 페이지로 이동 시 reviewRequestCodeAtom 값이 빈값이라 router 오류가 나요.

해결

리뷰 연결 페이지를 거치지 않고 리뷰 작성 페이지로 바로 들어오는 경우 생기는 router오류를 막기 위해서, 리뷰 작성 페이지에서도 reviewRequestCodeAtom 값을 업데이트 하도록 했어요.

Copy link
Contributor

Choose a reason for hiding this comment

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

리뷰 작성은 별도의 확인 절차를 필요로 하지 않으니 이렇게 라우터 에러를 방지하는 게 맞는 것 같아요. js 파일 분리할 때 작성 페이지로 바로 들어가볼 생각은 못 했는데 나중에 다시 체크해봐야겠네요! 😯

Copy link
Contributor

Choose a reason for hiding this comment

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

리뷰 작성 페이지에서 BreadCrumb으로 연결 페이지 이동할 생각을 못했었네요.. 수정 전 코드로 확인해보니 라우터 에러가 뜨네요ㅠㅠ 꼼꼼하십니다!!!👍

Copy link
Contributor

@chysis chysis Sep 18, 2024

Choose a reason for hiding this comment

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

이 PR을 보기 전에 해당 문제를 발견했는데 이미 해결했군요👍

handleCloseModal={() => closeModal(CARD_FORM_MODAL_KEY.submitConfirm)}
/>
)}
<QueryErrorResetBoundary>
Copy link
Contributor Author

@BadaHertz52 BadaHertz52 Sep 14, 2024

Choose a reason for hiding this comment

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

리뷰 제출 실패 시, 모달( SubmitErrorModal)을 띄우기 위해 QueryErrorResetBoundary, ErrorBoundary를 사용한 이유

리뷰 제츨 api 핸들러에서 throw Error 시, CardForm을 감싸는 QueryErrorResetBoundary안의 ErrorBoundary의 fallback이 실행돼요.
useMutate의 onError에서 SubmitErrorModal을 띄우려고 해도, 상위 부모 컴포넌트에서 이미 QueryErrorResetBoundary를 사용하고 있어서 QueryErrorResetBoundary의 ErrorBoundary가 실행돼요.

그래서 onError만으로는 SubmitErrorModal을 띄울 수 없어서, 제출 api 핸들러를 실행하는 SubmitCheckModal을 QueryErrorResetBoundary로 감싸서 제출 실패 시 throw하는 것을 QueryErrorResetBoundary에서 받을 수 있도록 했어요.

Copy link
Contributor

Choose a reason for hiding this comment

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

일단 바다가 적용한 방법이 가장 간편하고 직관적인 에러 처리를 해 줬다고 생각하지만, 읽다 보니 또다른 에러 처리 방식이 불현듯 생각나서 코멘트 남겨봅니다!

에러 바운더리의 선언적인 장점을 극대화하기 위해 아예 ErrorFallback을 케이스별로 만든 뒤 에러를 던질 때 커스텀 에러 문구를 추가하는 방식 등으로 에러 바운더리가 케이스별로 Fallback을 다르게 보여줄 수 있도록 만드는 방법은 어떨까요?

Copy link
Contributor Author

@BadaHertz52 BadaHertz52 Sep 15, 2024

Choose a reason for hiding this comment

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

fallback을 띄우주는 부분이 사용자가 작성하는 내용이 없거나, 해당 컴포넌트가 언마운트 되어도 내용이 살아있다면 올리의 제안처럼 해도 좋아요!

다만, 걱정이 되는 부분은 올리의 제안처럼 가려면 전제되어야하는 부분들이 있어요.

1.에러별 대처해야하는 상황에 대한 유연성 문제

1-1 에러별 대처해야하는 UI가 다르다.

에러 문구나 버튼의 글씨나 클릭 이벤트는 어느 정도 괜찮겠지만, 리뷰 제출 페이지에서는 리뷰 질문지 데이터를 받는 것에서 오류가 나면 ErrorSection을 , 리뷰 제출 시 오류가 나면 사용자가 작성한 리뷰 내용을 유지하기 위해서 ErrorModal을 fallback으로 지정해줬어요. 이처럼 에러별 대체해야하는 방식이 달라질 수 있어요.

Errorboundary로 감싸는 컴포넌트와 그 하위 컴포넌트에서 일어나는 오류에 대한 처리가 공통적인 UI라면 오류 메세지를 바꾸는 것만으로도 괜찮지만, 리뷰 작성 페이지처럼 아예 다른 UI이고, 사용자가 작성한 내용을 살려야하는 경우라면 처리가 복잡해질 것 같아요.

1-2 오류는 예측하기 힘들고, 이를 하나의 UI로 대응하는 것은 한계가 있다.

또한 리뷰 제출 실패 시 ErrorModal이 추후에 생긴 것 처럼, 개발자가 예측하지 못한 오류 처리를 해야해서, fallback 에서 오류 메세지를 바꾸는 것만으로는 오류 대응에 한계가 있을 것 같아요. 에러에 따라서 사용자에게 대응할 수 있는 방식을 다양하게 주고 있어서 이 부분도 문제가 될 것 같아요 (router 오류 시에는 홈으로 이동, 페이지 렌더링 시 api 요청하는 경우는 새로 고침을 제공하고 있고 리뷰 제출 실패 ErrorModal은 다시 제출해도 모달 상 제출이 제대로 된것인지 확인하기 어려워서 ErrorModal은 닫고 다시 제출을 하도록 했지만, ErrorModal에서 제출 버튼을 만들 수도 있을 거에요.)

2. 여러 에러는 하나의 fallback으로 대응하는 것은 fallback의 역할이 커진다.

fallback에서 처리하는 에러가 여러개가 되면, 모든 react query상 에러가 fallback으로 가서, fallback쪽의 코드만으로 이게 어느 에러를 처리하는 것이 선언적으로 보기 어려우며 하나의 fallback이 처리하는 에러가 많아진다고 생각해요.(=역할이 커진다)

3. 선언적인 에러 대응

QueryErrorResetBoundary, ErrorBoundary로 에러를 대응한 컴포넌트를 감싸는 것만으로도, DOM상에서 A라는 컴포넌트에 대한 react query 에러가 나면 fallback으로 지정한 B가 나오는 구나라는 것을 한 눈에 알 수 있어요.

SubmitConfrimModal을 ErrorBoundary로 감싸는 것만으로도 SubmitErrorModal는 SubmitConfrimModal의 에러 대응 방법이라는 것을 알 수 있는 거죠.

Copy link
Contributor

Choose a reason for hiding this comment

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

구체적으로 설명 적어주셔서 정말 감사합니다......😭

+fallback을 케이스별로 분리할 경우, 바다가 말했던 것 처럼 UI가 이번 경우는 아예 달라서 다소 복잡해질 것 같아요. fallback의 역할이 너무 커지기도 하고요.

Copy link
Contributor

@ImxYJL ImxYJL Sep 15, 2024

Choose a reason for hiding this comment

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

코멘트 달았던 이유가 리뷰미 서비스에 이 방법을 도입하자!보다는 이런 방법론 자체에 대한 이야기를 해 보고 싶어서였는데 전달이 미흡했네요 😂

바다 말대로 리뷰미는 내용이 날아가면 안 되는 서비스이고, 여기서는 특별히 에러 페이지 대신 에러 모달을 띄워주고 있듯 다양한 에러 처리 UI를 사용하고 있는 만큼 모든 에러 처리에 도입하기에는 문제가 있다고 생각합니다.

1번은 어찌보면 당연한 전제라고 생각해서 별도로 언급하지 않았어요ㅎㅎ

2번 설명은 잘 이해가 가지 않는데, 에러 바운더리를 사용한 이상 하위 컴포넌트에서 처리되지 않은 에러는 모두 fallback이 받는 구조인데 이걸 명시적으로 케이스를 나눠 처리한다고 특별히 더 역할이 커진다고 볼 수 있는 건지 궁금해요.
단순 fallback 하나를 띄우는 것과 여러 fallback을 가질 수 있는 것의 차이일까요?

만약 그렇다면 역할이 커진다는 말은 이해할 수 있는데, 선언적이지 않다는 게 여전히 의문이 남습니다. 저는 선언적이라는 걸 어떤 경우에 어떻게 처리해라! 라고 코드 상에서 명시적으로 알려주는 거라고 알고 있어서 케이스 처리를 해 주는 게 선언적이라고 생각했습니다. 오개념이라면 알려주세용ㅎㅎ

마지막 3번은 결합도 관련 답변인 것 같은데, 에러 바운더리는 하위 컴포넌트를 무조건 감싸야 하는 게 단점이지만 그로 인해 그들의 에러가 어디서 처리되는지를 명시적으로 드러내는 효과도 있다는 걸로 이해했습니다.

Copy link
Contributor Author

@BadaHertz52 BadaHertz52 Sep 18, 2024

Choose a reason for hiding this comment

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

올리가 제안한 fallback 분기 처리를 저는 '하나의 Errorboundary에서 error를 props로 받아서 fallback으로 지정한 컴포넌트에서, props로 받은 error를 판단해 필요한 에러 대응 ui를 보여준다'는 방식으로 이해했어요.
제가 이해한 기준으로 설명하면 다음과 같아요.

에러는 모두 fallback이 받는 구조인데 이걸 명시적으로 케이스를 나눠 처리한다고 특별히 더 역할이 커진다고 볼 수 있는 건지 궁금해요.

저는 Errorboundary 로 감싼 컴포넌트들에서 일어나는 에러가 하나의 Errorboundary 에서 받는 것 자체가 하나의 Errorboundary의 역할이 커진다고 생각했어요.

올리가 생각하는 하나의 ErrorBoundary에서 fallback으로 에러를 분기처리하는 방식의 코드가 궁금하네요
코드 없이 설명을 하니, 서로 다른 코드를 보고 다른 관점을 이야기할 수도 있다 생각이 들어요.

올리 혹시 올리의 생각을 구현한 Errorboundary 코드가 있을까요?

선언적이지 않다는 게 여전히 의문이 남습니다

fallback으로 에러를 분기처리해주는 곳에서 어떤 에러에 대한 대응인지 Errorbounday의 fallback 으로 지정한 컴포넌트 코드만으로 어려울 수 있다고 생각했어요. 아마 에러를 분기처리한다면, error 을 받아서 에러 별 다른 ui을 띄워줄텐데, 해당 에러가 어디에서 나는 지를 error만으로도 알기는 힘들거라고 생각했어요. 에러 내용을 구체화하면 되지만 어떤 컴포넌트에서 에러가 났는 지를 네이밍하는 것이 쉽지는 않을 것 같아요. 이 작업이 여러 명이 동시에 따로 작업해서 붙인 경우라면 더 헷갈리겠네요

handleCloseModal,
}: SubmitCheckModalProps) => {
const SubmitCheckModal = ({ handleCancelButtonClick, handleCloseModal }: SubmitCheckModalProps) => {
const { submitAnswers } = useSubmitAnswers({ closeSubmitConfirmModal: handleCloseModal });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

useSubmitAnswers를 SubmitCheckModal에서 부르는 이유

이전에는 CardForm에서 기능에 필요한 메서드등을 불러서 props로 내리고 하위 컴포넌트는 웬만하면 UI만 담당하도록 했었어요.
그러나, 제출 api 요청 시 오류를 QueryErrorResetBoundary가 감지하기 위해서는 제출 api 핸들러를 사용하는 useSubmitAnswers가 QueryErrorResetBoundary가 감싸는 컴포넌트에서 사용되어야하더라구요.

Copy link
Contributor

Choose a reason for hiding this comment

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

ErrorBoundary나 Context API처럼 하위 컴포넌트를 무조건 감싸줘야 하는 방식의 한계(높은 결합도)가 이런 곳에서 드러나네요... 😶
그래서 QueryErrorResetBoundary를 사용하는 대신 useSubmitAnswers 안에서 try-catch문을 이용해 에러를 처리하는 방법도 잠깐 생각해봤는데, 비동기 처리 관련 로직과 UI를 분리할 수 있겠지만 대신 코드가 복잡해지고 리액트 쿼리 맞춤형 에러 처리는 어렵겠다는 생각이 들었습니다.
바다도 QueryErrorResetBoundary를 사용할 때의 이득이 사용하지 않았을 때의 장점보다 더 크다고 생각해서 도입한 건지 궁금해요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

에러 처리의 어려움

올리의 말처럼, 이번에 리뷰 제출 실패 시 에러 대응을 할 때 제가 원하는 QueryErrorResetBoundary가 아닌 CardForm을 감싼 QueryErrorResetBoundary가 에러를 잡아서 이를 해결하는 것에 시간이 걸렸어요. 제가 의도한 fallback을 실행하려면 에러를 발생시키는 react query문이 QueryErrorResetBoundary로 감싼 컴포넌트에서 실행되어야해요. 이를 위해서 모달 컴포넌트는 되도록 UI 컴포넌트였으면 하는 저의 구현 방향을 살짝 바꾸어야했구요

그럼에도 사용한 이유 : 장점이 더 크다

선언적인 에러 핸들링

위의 코멘트에서 말씀드렸듯이 QueryErrorResetBoundary를 사용한다는 것만으로도 어떤 컴포넌트에서 나는 react query를 어떤 에러 ui로 보여준다는 것을 선언적으로 알 수 있어요.

props drilling

또한 에러 처리를 위한 props drilling에서도 자유로울 수 있어요. 리뷰 제출 실패시 ErrorModal을 띄우는 것을 useSumitAnswer에서 했다면, 모달을 관리하는 CardModalContainer의 내부에 ErrorModal을 여는 상태를 추가하고 useSumitAnswer는 이 상태를 변경하는 메서드를 props로 받아서 실행할거에요. 지금은 리뷰 제출 실패 하나이지만 제출 실패 처럼 다루어야하는 오류가 늘어가면 props가 복잡해지겠죠? QueryErrorResetBoundary를 사용하면 QueryErrorResetBoundary를 감싸고 있는 컴포넌트나 사용하는 훅 어디에서나 throw를 해줘서 props drilling에서도 자유롭고 이로 인해 useSumitAnswer는 원래 의도한 대로 답변 제출 기능만 책임지면 돼요.

api 요청 실패 오류가 난 컴포넌트 언마운트와 reset 제공

QueryErrorResetBoundary는 api 요청 실패와 그로인한 fallback 실행을 reset할 수 있는 기능을 제공해요.
만약 useSubmitAnswer에서 오류가 난 컴포넌트를 언마운트 시키고 ErrorModal을 열고, 다시 리뷰 제출을 시도하려면 ErrorModal을 닫는 기능을 추가로 진행해야하죠. 하지만 QueryErrorResetBoundary의 경우 오류가 나면 알아서 오류가 난 컴포넌트가 언마운트되고 reset을 fallback에서 받아서 닫기 이벤트에서 실행하면 fallback을 닫을 수 있어요.
매우 간단하죠 😆


const SubmitErrorModal = ({ resetErrorBoundary, closeSubmitCheckModal }: SubmitErrorModalProps) => {
const handleClose = () => {
closeSubmitCheckModal();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🧐개발자 도구 elements에서는 SubmitErrorModal만 있는데 왜 SubmitCheckModal이 남아있는 거지?

SubmitErrorModal이 열리면, 개발자 도구 elements에서는 SubmitErrorModal에 대한 element만 있고 SubmitCheckModal는 없어요. SubmitErrorModal가 닫히면, SubmitCheckModal가 생기더라구요.

찾아보니. ErrorBoundary가 오류를 감지해 fallback이 실행되면, ErrorBoundary가 감싸는 컴포넌트는 언마운트되고 대신, fallback으로 지정한 컴포넌트가 대신 마운트되요. 그리고 ErrroBoundary의 reset으로 fallback 컴포넌트를 언마운트 시키면 ErrorBoundary가 감싸는 컴포넌트가 마운트된다고 하네요.

SubmitErrorModal을 닫을 때, SubmitCheckModal도 닫는 이유?

리뷰 제출 실패 시, useMutate의 onError에서 SubmitCheckModal을 닫으면 QueryErrorResetBoundary으로 감싸는 대상이 DOM에서 사라져, fallback이 실행되지 않았어요. 그래서 onError시, SubmitCheckModal을 닫지는 않아요.

대신, SubmitErrorModal 창이 닫히면 SubmitCheckModal이 남아 있어서 사용자에게 혼란
(모달 창을 닫았는데 또 다른 모달창이 남아 있다??? )을 줄 수 있겠더라구요. 그래서 SubmitErrorModal을 닫을 때, SubmitCheckModal도 닫히도록 했어요.

Copy link
Contributor

Choose a reason for hiding this comment

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

좀비 모달이 아니라 에러 바운더리의 동작에 따른 자연스러운 동작이었군요!
다양한 fallback은 앞서 코멘트 단 대로 보여줄 수 있을 것 같은데 이렇게 fallback을 보여줄 때 마운트/언마운트되는 문제는 확실히 에러 바운더리의 구조적 한계라는 생각이 들었습니다.
로직이 아직 크게 복잡하진 않지만 편한 만큼 나름의 단점도 있는 것 같아 언제 한 번 에러 처리에 대해 프론트에서 얘기 나눠봤으면 좋겠어요!

Copy link
Contributor

@ImxYJL ImxYJL left a comment

Choose a reason for hiding this comment

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

이제 휴일 첫 날인데 PR이 2개나 올라와서 깜짝 놀랐어요ㅋㅋㅋㅋ
심지어 이렇게 양질의 작업 + 버그 개선이라니...역시 바다!
그리고 코드에 직접 코멘트를 달아 설명해줘서 이해하기 훨씬 편했어요!

현재 코드를 바로 approve해도 좋다고 생각하지만, 개인적으로 에러 처리에 대해 궁금한 점도 있고 더 깊게 이야기 나눠보고 싶어서 코멘트 몇 개 달았습니다. 추상적인 내용이고 저도 당장 답을 알고 있는 게 아니라 당장 반영하기는 힘들 것 같지만 리뷰하는 김에 달아봤습니다...ㅎㅎㅎ approve 필요하면 바로 하러 올게요!

const { resetFormRecoil } = useResetFormRecoil();

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

리뷰 작성은 별도의 확인 절차를 필요로 하지 않으니 이렇게 라우터 에러를 방지하는 게 맞는 것 같아요. js 파일 분리할 때 작성 페이지로 바로 들어가볼 생각은 못 했는데 나중에 다시 체크해봐야겠네요! 😯

const { resetFormRecoil } = useResetFormRecoil();

useEffect(() => {
if (reviewRequestCode) {
setReviewRequestCode(reviewRequestCode);
Copy link
Contributor

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.

✅ 반영했습니다!

handleCloseModal,
}: SubmitCheckModalProps) => {
const SubmitCheckModal = ({ handleCancelButtonClick, handleCloseModal }: SubmitCheckModalProps) => {
const { submitAnswers } = useSubmitAnswers({ closeSubmitConfirmModal: handleCloseModal });
Copy link
Contributor

Choose a reason for hiding this comment

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

ErrorBoundary나 Context API처럼 하위 컴포넌트를 무조건 감싸줘야 하는 방식의 한계(높은 결합도)가 이런 곳에서 드러나네요... 😶
그래서 QueryErrorResetBoundary를 사용하는 대신 useSubmitAnswers 안에서 try-catch문을 이용해 에러를 처리하는 방법도 잠깐 생각해봤는데, 비동기 처리 관련 로직과 UI를 분리할 수 있겠지만 대신 코드가 복잡해지고 리액트 쿼리 맞춤형 에러 처리는 어렵겠다는 생각이 들었습니다.
바다도 QueryErrorResetBoundary를 사용할 때의 이득이 사용하지 않았을 때의 장점보다 더 크다고 생각해서 도입한 건지 궁금해요!

handleCloseModal={() => closeModal(CARD_FORM_MODAL_KEY.submitConfirm)}
/>
)}
<QueryErrorResetBoundary>
Copy link
Contributor

Choose a reason for hiding this comment

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

일단 바다가 적용한 방법이 가장 간편하고 직관적인 에러 처리를 해 줬다고 생각하지만, 읽다 보니 또다른 에러 처리 방식이 불현듯 생각나서 코멘트 남겨봅니다!

에러 바운더리의 선언적인 장점을 극대화하기 위해 아예 ErrorFallback을 케이스별로 만든 뒤 에러를 던질 때 커스텀 에러 문구를 추가하는 방식 등으로 에러 바운더리가 케이스별로 Fallback을 다르게 보여줄 수 있도록 만드는 방법은 어떨까요?

display: flex;
flex-direction: column;
gap: 0.8rem;
align-items: start;
Copy link
Contributor

Choose a reason for hiding this comment

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

개인적으로 메세지 가운데 정렬이 더 예쁜 것 같아요ㅋㅋㅋ

Copy link
Contributor

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.

메세지가 여러 줄 일때, 줄의 길이가 달라도 가운데 정렬을 더 선호하시는 편인가요?


const SubmitErrorModal = ({ resetErrorBoundary, closeSubmitCheckModal }: SubmitErrorModalProps) => {
const handleClose = () => {
closeSubmitCheckModal();
Copy link
Contributor

Choose a reason for hiding this comment

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

좀비 모달이 아니라 에러 바운더리의 동작에 따른 자연스러운 동작이었군요!
다양한 fallback은 앞서 코멘트 단 대로 보여줄 수 있을 것 같은데 이렇게 fallback을 보여줄 때 마운트/언마운트되는 문제는 확실히 에러 바운더리의 구조적 한계라는 생각이 들었습니다.
로직이 아직 크게 복잡하진 않지만 편한 만큼 나름의 단점도 있는 것 같아 언제 한 번 에러 처리에 대해 프론트에서 얘기 나눠봤으면 좋겠어요!

Copy link
Contributor

@soosoo22 soosoo22 left a comment

Choose a reason for hiding this comment

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

구체적인 설명과 함께 동영상까지 올려주셔서 너무 감사합니다! 바다!
BreadCrumb까지 꼼꼼하게 확인해주셨네요!

const { resetFormRecoil } = useResetFormRecoil();

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

리뷰 작성 페이지에서 BreadCrumb으로 연결 페이지 이동할 생각을 못했었네요.. 수정 전 코드로 확인해보니 라우터 에러가 뜨네요ㅠㅠ 꼼꼼하십니다!!!👍

display: flex;
flex-direction: column;
gap: 0.8rem;
align-items: start;
Copy link
Contributor

Choose a reason for hiding this comment

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

저도 그렇습니다...

handleCloseModal={() => closeModal(CARD_FORM_MODAL_KEY.submitConfirm)}
/>
)}
<QueryErrorResetBoundary>
Copy link
Contributor

Choose a reason for hiding this comment

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

구체적으로 설명 적어주셔서 정말 감사합니다......😭

+fallback을 케이스별로 분리할 경우, 바다가 말했던 것 처럼 UI가 이번 경우는 아예 달라서 다소 복잡해질 것 같아요. fallback의 역할이 너무 커지기도 하고요.

Copy link
Contributor

@ImxYJL ImxYJL left a comment

Choose a reason for hiding this comment

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

코멘트 남겨주신 거 확인했습니다! 자세한 설명 덕에 이해가 잘 됐어요 (●'◡'●)
다만 아직 궁금한 점이 하나 있어서 그에 대한 코멘트 하나 더 남겼습니다.
이 리팩토링 코드 자체에 대한 의문은 아니니 지금 approve할게요!

Copy link
Contributor

@chysis chysis left a comment

Choose a reason for hiding this comment

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

코멘트로 상황과 해결 방법을 구체적으로 작성해주어서 변경 사항을 파악하기 편했어요👍
개인적으로 에러 처리가 상황마다 어떻게 되고 있는지 헷갈리는 시점이 온 것 같아서 한 번 날 잡고 정리를 해야겠습니다...
고생했어요!

@BadaHertz52 BadaHertz52 merged commit 7ea1c6e into develop Sep 18, 2024
9 checks passed
@BadaHertz52 BadaHertz52 deleted the fe/fix/611_review_post_error_modal branch September 24, 2024 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FE] 리뷰 제출 실패 시, ErrorPage가 아닌 모달을 띄운다.
4 participants