-
Notifications
You must be signed in to change notification settings - Fork 8
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] useSelectedFeedbackData 훅 오류 해결(#783) #784
Conversation
@@ -8,7 +8,7 @@ export const useFetchAlarmCount = (enabled: boolean = false) => { | |||
queryFn: getAlarmCount, | |||
enabled, | |||
refetchInterval: 60 * 1000, | |||
refetchIntervalInBackground: false, | |||
// refetchIntervalInBackground: false, |
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.
질문하려고 주석 처리를 하였는데요..! 다른 탭에 포커싱해서 실험하는 게 아닌가요??? 왜 저는 계속 refetch를 할까요..?
KakaoTalk_20241126_221612326.mp4
+) 기본값이 false라 이번 pr에서 삭제해도 되는지..?
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.
제가 생각했던 시나리오는 다음과 같아요. 코레아 사이트를 브라우저에 띄웠지만 그 위에 다른 브라우저 탭을 올려서 실행시키는 상황에서는 refetch가 필요없을 것 같아서 refetchIntervalInbackground
옵션을 사용했어요. 그래서 텐텐이 의도했던 것처럼 동시에 두개의 브라우저는 분할화면으로 띄우는 상황에서는 이 옵션이 적용되지 않는 것 같아요.
제 생각에는 분할 화면으로 브라우저를 띄울 경우에는 어쨋거나 실시간으로 데이터가 반영되는 게 좋다고 생각해서 refetchOnWindowFocus
옵션을 추가하지 않았는데 이야기 나눠보면 좋을 것 같아요~! (잘못된 게 있으면 알려줘,,,)
+) refetchIntervalInbackground
의 기본값이 false라서 지워도 될 것 같네용 👍
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.
요약정리 refetch 옵션 참고자료: tanstackquery docs
-
refetchIntervalInBackground
- 브라우저 탭이 백그라운드일 때 주기적 refetch 실행 여부
- "true"면 탭이 백그라운드여도 계속 refetch
- "false"면 중단 <- 기본값
-
refetchOnWindowFocus
- 탭을 다시 클릭했을 때 refetch 실행 여부
- "true"면 탭 포커스시 stale 데이터만 refetch <- 기본값
- "false"면 탭 포커스시 refetch 안 함
- "always"면 탭 포커스시 무조건 refetch
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.
아하 탭 포커싱
이라는 말이 화면에 보이는 상태군요!!! 분할 화면은 화면에 보이는 상태라 탭이 포커싱 되어 있다고 판단하고 있는 것 같네요.! 이에 따라 3초마다 refetch를 하도록 변경한 후 두 가지 실험을 해보았습니다.
- 탭이 다른 곳에 있을 때
5초 뒤에 코레아 탭으로 다시 돌아왔을 때 이전 상태 그대로임 -> refetch 안 함
KakaoTalk_20241127_154414056.mp4
- 브라우저가 최소화 상태일 때
5초 뒤에 창을 다시 켰을 때 이전 상태 그대로임 -> refetch 안 함
KakaoTalk_20241127_152246218.mp4
화면에 보일 때는 계속 refetch를 하는게 맞다고 생각해서 refetchOnWindowFocus 속성도 기본값인 true가 맞는 것 같아요ㅎㅎㅎ 공식 문서까지 첨부해서 알려주셔서 감사합니다🤩 refetchIntervalInBackground 주석 부분은 삭제하겠습니다!
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.
텐텐 빠르게 작업해주셔서 감사해요~
이번 PR에는 상황마다 동영상 첨부해주셔서 문제 상황과 해결된 상황을 확인할 수 있어 좋았어요 짱짱 :>
코멘트로 남겨놓은 부분 확인해보시고 같이 얘기나눠봐요~~
@@ -1,5 +1,7 @@ | |||
import { NonEmptyArray } from "@/@types/NonEmptyArray"; | |||
|
|||
export type FeedbackType = "받은 피드백" | "쓴 피드백"; |
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.
이것도 타입정리를 한번 해야겠다 생각했었는데, 말 안해도 딱 잘 해주셔서 감사합니다~
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.
useSelectedFeedbackData에 있던 타입이라 바로 분리했습니다ㅎㅎㅎ 감사합니다😊
@@ -8,7 +8,7 @@ export const useFetchAlarmCount = (enabled: boolean = false) => { | |||
queryFn: getAlarmCount, | |||
enabled, | |||
refetchInterval: 60 * 1000, | |||
refetchIntervalInBackground: false, | |||
// refetchIntervalInBackground: false, |
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.
제가 생각했던 시나리오는 다음과 같아요. 코레아 사이트를 브라우저에 띄웠지만 그 위에 다른 브라우저 탭을 올려서 실행시키는 상황에서는 refetch가 필요없을 것 같아서 refetchIntervalInbackground
옵션을 사용했어요. 그래서 텐텐이 의도했던 것처럼 동시에 두개의 브라우저는 분할화면으로 띄우는 상황에서는 이 옵션이 적용되지 않는 것 같아요.
제 생각에는 분할 화면으로 브라우저를 띄울 경우에는 어쨋거나 실시간으로 데이터가 반영되는 게 좋다고 생각해서 refetchOnWindowFocus
옵션을 추가하지 않았는데 이야기 나눠보면 좋을 것 같아요~! (잘못된 게 있으면 알려줘,,,)
+) refetchIntervalInbackground
의 기본값이 false라서 지워도 될 것 같네용 👍
@@ -8,7 +8,7 @@ export const useFetchAlarmCount = (enabled: boolean = false) => { | |||
queryFn: getAlarmCount, | |||
enabled, | |||
refetchInterval: 60 * 1000, | |||
refetchIntervalInBackground: false, | |||
// refetchIntervalInBackground: false, |
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.
요약정리 refetch 옵션 참고자료: tanstackquery docs
-
refetchIntervalInBackground
- 브라우저 탭이 백그라운드일 때 주기적 refetch 실행 여부
- "true"면 탭이 백그라운드여도 계속 refetch
- "false"면 중단 <- 기본값
-
refetchOnWindowFocus
- 탭을 다시 클릭했을 때 refetch 실행 여부
- "true"면 탭 포커스시 stale 데이터만 refetch <- 기본값
- "false"면 탭 포커스시 refetch 안 함
- "always"면 탭 포커스시 무조건 refetch
const toggleFeedbackSelection = (roomId: number) => { | ||
roomId === selectedFeedback ? handleDeselectedFeedback() : handleSelectedFeedback(roomId); | ||
}; | ||
|
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.
이렇게 수정되니 좋네요!
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.
저런 문제가 발생하는지 모르고 있었네요.. 😅
고생하셨어요~~
@@ -14,14 +13,20 @@ interface FeedbackCardListProps { | |||
feedbackData: FeedbackCardDataList[]; | |||
selectedFeedback: number | undefined; | |||
handleSelectedFeedback: (roomId: number) => void; | |||
handleDeselectedFeedback: () => void; |
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.
뜬금없지만 DeSelected 가 처음에는 없는 단어인줄 알았는데 실제로 사용되는 단어였네요!
하나 배워갑니당
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.
저도 저 단어 처음 보는데 gpt가 선택 해제할 때 쓰는 단어라고 알려줘서 사용했습니다ㅎㅅㅎ
📓 스토리북 링크
바로가기
📌 관련 이슈
✨ PR 세부 내용
🔥 문제 상황
기존의 handleSelectedFeedback 함수는 토글 형태로 되어있었습니다. 열려있는 피드백을 다시 눌렀을 때는 선택을 해제하여 다시 닫히게 하는 것이 목적이었기 때문입니다.
알림 페이지에서 피드백이 작성되었다는 알림을 받고 처음 피드백 페이지로 이동했을 때는 해당 피드백이 잘 열렸습니다.
하지만 이미 그 피드백이 sessionStorage에 저장이 되어있는 상태에서 다시 알림 페이지에서 해당 피드백으로 이동하면 하면 닫혀있습니다. handleSelectedFeedback에서 이미 선택된 피드백에 다시 접근하려고 하는 것으로 동작하고 있기 때문입니다.
-CoReA.Code.Review.Area.-.Chrome.2024-11-25.17-27-41.mp4
🔥 해결 방법
선택과 미선택 함수를 둘 다 만들어서 해결했습니다!
useModal 훅의 방식을 떠올려 해결 방법을 찾았습니다. 모달도 열리거나 닫히거나 둘 중 하나지만 비동기적으로 업데이트되거나 다른 곳에서 상태를 변경했을 때 예측하지 못 한 오류가 생길 수 있어 Open과 Close를 따로 만들었습니다.
useModal
이 아이디어를 착안하여 선택과 미선택 함수를 둘 다 만들고 사용처에서 이를 선택해서 호출하는 방식으로 변경하였습니다.
useSelectedFeedbackData
사용하는 곳
선택만 있는 곳
-CoReA.Code.Review.Area.-.Chrome.2024-11-26.22-10-14.mp4
선택/미선택 있는 곳
-CoReA.Code.Review.Area.-.Chrome.2024-11-26.22-11-19.mp4