-
Notifications
You must be signed in to change notification settings - Fork 2
feat : 편지작성, 랜덤편지 구현 90% 완료, 편지상세는 이후에 테스트 #67
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
feat : 편지작성, 랜덤편지 구현 90% 완료, 편지상세는 이후에 테스트 #67
Conversation
…연결 + 편지 유형 분기처리 작업중(어렵다)
…서 편지 이동시 location state 추가 + 랜덤 편지 조회 데이터 사용
…github.com/prgrms-web-devcourse-final-project/WEB2_3_9crops_FE into 49-feat-write/randomLetter/letterDetail-nd
…al-project/WEB2_3_9crops_FE into 49-feat-write/randomLetter/letterDetail-nd
…ter state 만들어 selectedLetter랑 따로 관리(Matched 페이지 전용)
…al-project/WEB2_3_9crops_FE into 49-feat-write/randomLetter/letterDetail-nd
tifsy
left a comment
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.
전투를 치르셨네요.. 수고 많으셨습니다! 🥹
아래는 공통적으로 리뷰 드리고 싶었던 점입니다! 💪🏻
try-catch로 api 호출을 감싸 예상치 못한 에러를 체크!res대신res.status로 어떤 에러인지 파악할 수 있도록catch (error)후에throw error;처리
| export { getRandomLetters }; | ||
| const postRandomLettersApprove = async (approveRequest: ApproveRequest, callBack?: () => void) => { | ||
| try { | ||
| console.log('엔드포인트 : /api/random-letters/approve'); |
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.
테스트용 console.log는 나중에 한 번에 삭제하거나 해야겠네용
| console.log('엔드포인트 : /api/random-letters/approve'); | ||
| console.log('request', approveRequest); | ||
| const res = await client.post('/api/random-letters/approve', approveRequest); | ||
| if (!res) throw new Error('랜덤편지 매칭수락 도중 에러가 발생했습니다.'); |
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.
!res 로는 어떤 에러인지 알기가 어려우니 res.status 로 확인하는 건 어떨까요?
if (res.status !== 200) { throw new Error('랜덤편지 매칭수락 도중 에러가 발생했습니다.'); }
이런 식으로요!
| if (callBack) callBack(); | ||
| return res; | ||
| } catch (error) { | ||
| console.error(error); |
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.
throw error; 까지 처리해주시면 좋을 것 같습니다!
| <ArrowLeftIcon className="h-6 w-6 text-white" /> | ||
| <button onClick={() => navigate(-1)}> | ||
| <ArrowLeftIcon className="h-6 w-6 text-white" /> | ||
| </button> |
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.
저도 늘 까먹지만 aria-label 붙여주시면 좋을 것 같습니다!
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.
확실히 코드가 짧아졌네요! 아주 좋습니다 💃🏻🕺🏻
| // createdAt: new Date(), | ||
| // }, | ||
| // ]; | ||
|
|
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.
더미 데이터 삭제 대신 주석 처리로 남겨두신 이유가 궁금해요!
| setMatchedLetter(res.data.data); | ||
| setOpenSelectedDetailModal(true); | ||
| } | ||
| }; |
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.
try-catch로 api 호출을 감싸는 게 좋아보입니다!
| setMatchedLetter(data); | ||
| } | ||
| } | ||
| }; |
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.
data가 null 이거나 undefined일 경우에 오류를 방지하는 로직이 있으면 어떨까요?
| if (res?.status === 200) { | ||
| setPrevLetter(res.data.data); | ||
| } else { | ||
| alert('이전 편지 데이터를 받아오는 도중 오류가 발생했습니다(잘못된 편지 접근입니다.)'); |
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.
console.log 가 아니라 alert 로 하신 이유가 있을까요? 사용자에게 보여줘야 할 부분은 아닌 것 같아서 여쭤봅니다!
src/pages/Write/index.tsx
Outdated
| getPrevLetter(letterId, setPrevLetter); | ||
| // MEMO : letterId는 쿼리파라미터를 통해 얻을수 있음 => 최초답장, 답장만 prevLetter을 받는 로직을 실행함 | ||
| if (!letterId) return; | ||
| if (location.state && location.state?.randomMatched) { |
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.
location.state?.randomMatched 만 사용해도 될 것 같습니당
✅ 요약
🪄 변경사항
🖼️ 결과 화면 (선택)
지금 랜덤편지 하나를 답장 완료한 상태라 쿨타임 화면밖에 안보입니당😅 추후에 다른 화면도 올리겠습니다!
💬 리뷰어에게 전할 말 (선택)