[Refactor] 홈 화면 qa 반영 #477
Conversation
| </div> | ||
| <button | ||
| className={readAllButtonStyle({ isAllRead: notiCounts === 0 })} | ||
| onClick={() => readAllNotifications()} |
There was a problem hiding this comment.
| onClick={() => readAllNotifications()} | |
| onClick={readAllNotifications} |
| } | ||
|
|
||
| const FeedItem = ({ solutionId, groupId }: FeedItemProps) => { | ||
| const commentCountRef = useRef(3); |
There was a problem hiding this comment.
useState로 visibleCount 를 관리하는 게 UI에 직접 영향을 줄 수 있을 것 같은데 ref로 상태 관리하는 이유가 궁금해요!
There was a problem hiding this comment.
컴포넌트 외부에 DEFAULT_VISIBLE = 3 상수화해서 매직넘버 없애면 좋을 것 같습니다!
There was a problem hiding this comment.
저도 궁금하네요. 굳이 리렌더링 일으킬 필요없는 값이라면 아마 let commentVisibleCount = 3으로 해도 똑같이 동작하지 않을까 싶어요.
There was a problem hiding this comment.
comments?.slice(0, commentCountRef.current)를 그냥 렌더에서 계산해버리고 이 친구의 length 기반으로 계산하면 JSX 구문의 계산식들은 다 빼도 될듯 ? 싶은데
There was a problem hiding this comment.
현재 최대 3개까지의 댓글만 피드에 보여지는게 정책이에요.
이때 피드에서는 직접 댓글을 달 수도 있는데 댓글을 달면 기본 3개 + 실시간으로 달은 댓글들 이 보이는게 자연스로운 UI라 생각해요.
만약 댓글을 달았는데 계속 3개의 댓글만 보여주는 정책이 유지되면, 맨위에 있던 댓글은 사라지고 새로단 댓글이 맨밑에 생길텐데 이러한 흐름은 UX에 안좋다 생각해요!
그래서 실시간으로 달은 댓글 수를 보관하기 위해 ref를 사용했어요.
그런데 제가 화면에 보여져야하는 댓글 수로 퉁쳐서 관리해서 이해하기 어려운 코드가 된것 같네요.
ref를 아예 실시간으로 달은 댓글 수만 보관하고 DEFAULT_VISIBLE + ref 값을 기반으로 계산을 하는게 좋겠네요!
state를 안쓰는 이유는 어차피 댓글 달때만 바뀌는 값이고, 댓글 달면 쿼리 inavalidate되어 리랜더링은 이미 이루어져서 굳이 state를 안써도 상관없다고 생각했답니다!
There was a problem hiding this comment.
문제, 코멘트, 스터디 탭에서 지운 알림이 전체 탭에는 그대로 남아 있는 문제가 있으므로 useReadNotiItemMutation의 onSettled에서 NotificationType.ALL을 처리하도록 추가해야 해요.
이 부분을 invalidate로 처리하기 보다는 setQueryData와 Array.filter 메서드를 사용해서 데이터를 직접 처리하는 게 좋아요. 단순히 전체 리스트에서 해당 알림 하나만 지우면 되는 부분이고, 페이지네이션을 쓰고 있거나 또 다른 쿼리를 사용하여 조합하는 복잡한 구성의 데이터가 아니므로 invalidate를 사용하지 않아도 되기 때문이에요.
There was a problem hiding this comment.
삭제는 useDeleteNotiMutation 담당이라 해당 훅에 반영했습니다!
There was a problem hiding this comment.
전체 탭의 알림, 즉 다른 탭의 알림을 삭제해주는 로직이어서 굳이 onSettled에서 처리하기 보다는 onSuccess에서 삭제 성공시 처리해주는게 효율적이라 생각해서 onSuccess에 로직 추가해주었어요.
| {comments?.length > displayedCommentCount && ( | ||
| <Link | ||
| href={`/group/${groupId}/solved-detail/${solutionId}`} | ||
| className={moreCommentContainer} | ||
| > | ||
| <div className={moreCommentWrapper}> | ||
| <span>{`댓글 +${comments?.length - displayedCommentCount}`}</span> | ||
| <span className={moreCommentButtonStyle}>더보기</span> | ||
| </div> | ||
| </Link> | ||
| )} |
There was a problem hiding this comment.
comment 관련된 건 FeedItem 컴포넌트에서 분리됐으면 좋겠어요!
comment 없는 경우 해당 컴포넌트를 아예 렌더링하지 않게끔!
지금은 comment 관련해서 optional 체이닝이 많아서 관심사 분리가 안 되고 컴포넌트 최적화가 안 돼보여요!
There was a problem hiding this comment.
어차피 지금 피드가 뜨는게 댓글이 달려있는경우에 뜨는거라 FeedItem에서 댓글이 없는 경우는 없어요!
그리고 FeedItem 전반에서 comment 데이터가 사용되어서 컴포넌트 분리를 해도 크게 관심사 분리가 되지않는다고 생각해서 같이 두었어요.
또한 옵셔널 체이닝도 더보기 버튼 하나 말고는 없어서 분리하는것보다 현상태를 유지하는게 좋다고 생각합니다!
There was a problem hiding this comment.
아래 사항은 readAllNotifications가 mutate 함수라서 바로 넣으면 event 객체를 매개변수로 받지 않아서 에러가 떠요.
따라서 화살표 함수로 한번 감싸서 넣어줘야 타입에러가 안터지네요!
| </div> | ||
| <button | ||
| className={readAllButtonStyle({ isAllRead: notiCounts === 0 })} | ||
| onClick={() => readAllNotifications()} |
rtttr1
force-pushed
the
refactor/#fe-130/qa
branch
from
9755900 to
792d8b1
Compare
🚀 CI Check Results
|
✅ Done Task
☀️ New-insight
💎 PR Point
📸 Screenshot
2025-12-11.10.29.29.mov