-
Notifications
You must be signed in to change notification settings - Fork 2
[Refactor] 홈 화면 qa 반영 #477
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
Changes from all commits
a2dc6af
f0224e8
df92b1d
f62cf26
5a52a86
62b1ef5
2840ea3
086a37c
6a9d30c
61a8024
792d8b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 문제, 코멘트, 스터디 탭에서 지운 알림이 전체 탭에는 그대로 남아 있는 문제가 있으므로 이 부분을
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 삭제는
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
comment 관련된 건 FeedItem 컴포넌트에서 분리됐으면 좋겠어요!
comment 없는 경우 해당 컴포넌트를 아예 렌더링하지 않게끔!
지금은 comment 관련해서 optional 체이닝이 많아서 관심사 분리가 안 되고 컴포넌트 최적화가 안 돼보여요!
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.
어차피 지금 피드가 뜨는게 댓글이 달려있는경우에 뜨는거라 FeedItem에서 댓글이 없는 경우는 없어요!
그리고 FeedItem 전반에서 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.
아래 사항은 readAllNotifications가 mutate 함수라서 바로 넣으면 event 객체를 매개변수로 받지 않아서 에러가 떠요.
따라서 화살표 함수로 한번 감싸서 넣어줘야 타입에러가 안터지네요!