[ Fix ] self QA - 뒤로가기 버튼 수정, 약속신청 모달 위치 조정, 이미 있는 약속 신청시 alert #225
Conversation
ijieun
requested review from
lydiacho and
se0jinYoon
and removed request for
lydiacho
There was a problem hiding this comment.
현재 지난 PR에 해당하는 commit들이 다 따라온 것으로 보여요! 그래서 간단한 작업들을 한 PR임에도 불구하고 files changed가 27개 😭😭😭
이런 사소한 부분들이 코드리뷰의 비효율성을 크게 높이기 때문에 PR별 commit은 반드시 챙겨주면 좋겠어요!
현재 커밋이 같이 딸려온 이유가, 지난번 PR과 연관된 작업들을 진행한 상태에서 fix/#222/patchSelfQA 라는 브랜치를 새로 만들어서 거기서 이어서 작업한 것으로 보이는데,
새로운 이슈에 대한 작업을 하고자 한다면 작업하던 브랜치가 아닌 develop으로 checkout한 후, 거기서 새로이 분기해서 작업하면 지난번 PR에 대한 작업물은 반영이 안되기 때문에 딸려오지 않겠죠?
하지만 지금 같은 경우 아마 지은이가 이전에 한 작업물을 반영한 채로 이어서 작업하고 싶어서 이렇게 한 것 같은데,
이럴 경우엔 PR을 올릴 때 base를 develop이 아닌 지난 작업 브랜치로 지정해서 PR을 올린 이후에 작업한 내용물이 diff에 잡히지 않도록 할 수 있어요.
일단 현상황에서는 base branch를 바꿔도 되고, 혹은 지난번 PR close해도 될 것 같으면 merge 시켜서 develop 브랜치에 지난 PR commit들을 반영해주는 방법도 있어요!
두 방법 중 하나 선택해서 PR과 무관한 commit 뺀 이후에 다시 코멘트로 멘션해주세요 !!
|
@lydiacho 느아아ㅏ 사소한거부터 마니 배웁니다!!!! base branch 를 바꾸는 방법으로 수정했습니다...🥔🥔🧡 |
| onError: (error) => { | ||
| if (axios.isAxiosError(error) && error.response?.status === 400) { | ||
| alert('이미 약속을 신청한 선배입니다.'); |
There was a problem hiding this comment.
p3) 해당 로직 에러처리 섬세하게 해준 거 너무 굿입니다! 한가지 제안하고 싶은 건 해당 에러 처리를 컴포넌트 내에서 하는게 아닌, 쿼리함수 내에서 하는 것은 어떨까요? 에러 발생시 state와 관련된 로직이 동작하는게 아닌 간단한 alert 처리만 해주면 되기 때문에 UI가 그려지는 컴포넌트 로직과 에러 처리 로직을 분리하면 더 좋을 것 같슴니다! 어떻게 생각하시나요!
There was a problem hiding this comment.
너무너무 좋은거 같습니다!! 확실히 에러 핸들링을 쿼리함수에 분리하니 가독성이 올라서 굿이네요..🧡👍🏻
| timeList: [ | ||
| { | ||
| date: selectedTime[0].clickedDay, | ||
| startTime: selectedTime[0].selectedTime.split('-')[0], | ||
| endTime: selectedTime[0].selectedTime.split('-')[1], | ||
| }, | ||
| { | ||
| date: selectedTime[1].clickedDay, | ||
| startTime: selectedTime[1].selectedTime.split('-')[0], | ||
| endTime: selectedTime[1].selectedTime.split('-')[1], | ||
| }, | ||
| { | ||
| date: selectedTime[2].clickedDay, | ||
| startTime: selectedTime[2].selectedTime.split('-')[0], | ||
| endTime: selectedTime[2].selectedTime.split('-')[1], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
| timeList: [ | |
| { | |
| date: selectedTime[0].clickedDay, | |
| startTime: selectedTime[0].selectedTime.split('-')[0], | |
| endTime: selectedTime[0].selectedTime.split('-')[1], | |
| }, | |
| { | |
| date: selectedTime[1].clickedDay, | |
| startTime: selectedTime[1].selectedTime.split('-')[0], | |
| endTime: selectedTime[1].selectedTime.split('-')[1], | |
| }, | |
| { | |
| date: selectedTime[2].clickedDay, | |
| startTime: selectedTime[2].selectedTime.split('-')[0], | |
| endTime: selectedTime[2].selectedTime.split('-')[1], | |
| }, | |
| ], | |
| timeList: selectedTime.map(({ clickedDay, selectedTime }) => ({ | |
| date: clickedDay, | |
| startTime: selectedTime.split('-')[0], | |
| endTime: selectedTime.split('-')[1], | |
| })), |
p3) 이렇게 간결하게 중복코드 줄일 수 있을것같아용
There was a problem hiding this comment.
헙 너무 좋습니다!!! 이전 PR에 포함했습니다!!
#️⃣ Related Issue
Closes #222
✅ Done Task
☀️ New-insight
✨ mutate 함수에 두 번째 인자로 onError를 전달하여 오류 처리
mutate 함수의 두번째 인자에 onError 를 전달하여 서버 요청이 실패한 경우의 로직을 정의할 수 있다
💎 PR Point
🥕 뒤로가기 버튼 수정
navigate(-1) 을 통해 header 의 < 버튼을 선택하면 뒤로가도록 수정했습니다
🥕 약속신청 모달 위치 조정
공통 컴포넌트의 모달 위치가 잘 조정되어 있지 않은 이슈가 있었습니다. 이는 모달 컴포넌트를 감싸고 있던 부모 컴포넌트에 height가 적용되어 있던 탓에 비뚤어진 것이었습니다! 모달을 조건부 렌더링으로 불러오고, 부모 컴포넌트의 영향을 받지 않기 때문에 감싸는 부모 컴포넌트 바깥에 위치하도록 수정했습니다.
🥕 postAppointment 함수의 두번째 인자로 onError 핸들러 전달해서 error 핸들링
'이미 약속을 신청한 선배에게 다시 약속 신청을 하는 경우' console 에러만 기록되고 화면에는 아무 액션도 취해지지 않았었습니다!
이를 해결하기 위해 onError를 사용해서 상태코드가 400인 경우 alert 를 호출하도록 수정했습니다.
📸 Screenshot
2024-08-20.12.27.00.mov
2024-08-20.12.27.16.mov