[FE] 알림 컴포넌트의 UI를 변경하고, 채팅을 입력한 날짜를 알림 컴포넌트를 활용하여 알리도록 개선 #938
Conversation
- ThreadList 자체에 양옆 여백이 있어 불필요하다고 판단
- 연/월/일 + 시간을 포맷팅해주는 유틸 함수 외에, 연/월/일 만을 포맷해주는 유틸 함수가 필요해지게 되었음 - 채팅방에서의 날짜 표시는 "YYYY년 MM월 DD일" 과 같이 표시되어야 함
- 단순히 쪼개는 split() 함수를 사용 가능하나, 이 경우 date의 타입이 string으로 추론되는 문제가 있음 - 이번 기능 구현에서는 date와 time 데이터가 빈번하게 따로 쓰이고, date의 타입 추론 또한 YYYYMMDD로 되어야 하므로 이를 위해 본 유틸 함수를 구현
- 스레드의 각 페이지의 최상단 스레드의 날짜/시간 데이터를 관리할 수 있는 커스텀 훅 - 이미 ThreadList 컴포넌트가 매우 복잡하므로 조금이라도 복잡성을 줄이기 위해 커스텀 훅을 구현 - 본 커스텀 훅이 반환하는 정보는 각 스레드가 당일의 첫 번째 스레드인지를 판별하는 데 사용 예정
- isFirstThreadOfDay: 해당 스레드가 당일의 첫 번째 스레드인지를 의미. true 값일 경우 날짜 알림을 추가로 렌더링해 주어야 함 - checkFirstThreadOfDay: 해당 스레드가 당일의 첫 번째 스레드인지를 판별하는 함수 - 새로운 페이지가 불러와질 때마다 useEffect가 실행되어 각 페이지의 첫 번째 스레드가 state로써 갱신됨
- 기존에는 날짜 + 시간 정보를 보여줬지만, 날짜가 지날 때마다 Notification 컴포넌트를 별도로 렌더링하므로, 이제 시간 정보만을 보여주어도 상관 없음, 이에 따라 가능을 축소
| } | ||
|
|
||
| return currentDate !== firstDate; | ||
| }; |
There was a problem hiding this comment.
아마 여기가 생소할 수도 있을 것 같다
가능한 한 알아보기 쉽게 짜고 싶었지만, 이미 컴포넌트의 덩치가 크고 변수들도 많다보니까 쉽지 않더라고
함수 이름은 checkFirstThreadOfDay야, 얘가 하는 기능은 관련된 스레드들의 날짜들을 토대로 해당 날짜(currentDate)를 지니는 스레드가 당일의 첫 번째 스레드인지 판별하는 거. 어떤 스레드든 간에, 그 날의 첫 번째 스레드가 있기 마련이잖아?
checkFirstThreadOfDay 가 true 를 반환하면, isFirstThreadOfDay 변수가 true 가 될 것이고, 그렇게 되면 해당 스레드의 바로 위에 XXXX년 XX월 XX일 알림 컴포넌트를 추가로 렌더링하도록 작동하게 돼.
본 함수는 return 문을 포함하여 총 3개의 조건을 거처.
-
첫 번째 조건: 만약 지금 검사하고자 하는 스레드가 첫 번째 스레드가 아니라면, 이전 스레드와 날짜를 단순히 비교하는 것으로 어렵지 않게 이 스레드가 당일의 첫 번째 스레드인지 확인할 수 있겠지?
-
두 번째 조건: 여기까지 왔다면 검사하고자 하는 스레드가 첫 번째 스레드여서, 다른 페이지의 스레드와의 비교가 필요한 상황일 거야(
previousDate = undefined). 여기서firstDate마저undefined라는 것은, 이미 더 이상 불러올 페이지가 없어서undefined가 할당된 것이기에, 검사하고자 하는 메시지는 팀플레이스의 역사적인 첫 스레드라는 뜻이 돼. 이 경우에는 항상 당일의 첫 스레드이므로true를 반환해야 해. -
세 번째 조건:
firstDate가undefined가 아니니까, 검사하고자 하는 메시지의 바로 윗 메시지가firstDate가 돼. 이 경우에는 두 메시지를 첫 번째 조건처럼 비교하면 되겠지.
There was a problem hiding this comment.
-
두 번째 조건이 적용된 예시
-
팀플레이스의 첫 스레드는 항상 당일의 첫 스레드이다.
| index - 1 >= 0 && authorId === threads[index - 1].authorId; | ||
| index - 1 >= 0 && | ||
| authorId === previousThread.authorId && | ||
| !isFirstThreadOfDay; |
There was a problem hiding this comment.
Q. 왜 isContinue 변수에 조건 추가함?
A. 한 사람이 혼자 채팅을 치고 있는 상황을 생각해 보자. 그런데, 이 사람이 이야기하던 중에 하루가 지났어. 이 경우 별도의 처리를 안 해주면, 날짜 컴포넌트가 전날의 채팅과 새로운 날의 채팅을 가릴 거야. 여기까지는 괜찮아. 그런데, isContinue 값을 그대로 true로 냅둔다면, 아래와 같이 어색한 UI가 생길 수 있을 거야.
대부분의 채팅 웹 서비스도 그렇고, 중간에 다른 알림이 섞인 식이라면, 같은 사람이 치던 채팅이더라도 중간에서 끊어주고 다시 프로필 사진과 이름을 띄워주는 것이 어울릴 거라 생각해.
그래서 강제로 isContinue 를 false 로 바꾸도록 구현해 준거야.
이렇게 하면, 아래와 같이 채팅창이 보이도록 만들 수 있어!
| onClickImage={onClickImage} | ||
| {...rest} | ||
| /> | ||
| </Fragment> |
There was a problem hiding this comment.
여기가 스레드일 경우 스레드를 렌더링하는 부분. <Thread> 위에 <Notification> 이 새로 생겼지? 이 <Notification> 이 이제 해당 스레드가 당일의 첫 번째 메시지라면 XXXX년 XX월 XX일과 같이 스레드 위에 자동으로 알림을 보여 줄 거야!
| time={currentDateTime.time} | ||
| /> | ||
| </Fragment> | ||
| ); |
There was a problem hiding this comment.
여기가 알림일 경우 알림을 렌더링하는 부분. 알림 자체는 렌더링할 때 시간만을 보여주기 때문에, 마찬가지로 알림이당일의 첫 번째 채팅이라면 똑같이 처리될 거야!
| firstTime: splittedFirstDateTime?.time, | ||
| setFirstDateTime, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
모든 스레드들을 하나의 배열을 활용하여 보여준다면 이전 스레드의 날짜를 쉽게 알 수 있으니 이런 커스텀 훅도 필요없을 거야.
하지만 불행히도 경우에 따라 스레드의 이전 스레드를 확인할 때 페이지 단위로 건너가야 할 수도 있어.
그렇기 때문에, 이전 페이지의 날짜를 여기에 저장해두고 쓰도록 구현한거야.
|
|
||
| if (!isYYYYMMDD(date)) { | ||
| throw Error('잘못된 dateTime 변수가 대입되었습니다.'); | ||
| } |
There was a problem hiding this comment.
좀 어색하기는 하지만, 리턴되는 date 데이터가 YYYYMMDD 타입으로 추론되기 위해서 넣었어.
만약 더 자연스러운 방법이 있다면 그것도 한 번 생각해 보면 좋을듯
| content: string; | ||
| icon?: ReactElement; |
There was a problem hiding this comment.
이미지 컴포넌트 처럼 type을 두어서 정해진 형태로 사용하도록 하면 될 것 같은데 icon을 받게 구현한 이유가 있으면 알려줘!
타입에 따라 사용할 수 있다면
- 알림 컴포넌트 코드만으로 구현방식을 추측하기 쉬움
- 사용처에서 원하는 모습을 만들기 위해 다른 액션을 취할 필요 없음(어떤 아이콘을 import 해야하는지 등)
- 알림 컴포넌트 내부에서 사용되는 svg 아이콘을 확인하고 관리하기 편함
- 일관적인 디자인으로 구현할 수 있음
라는 장점이 있을 것 같아.
물론 새로운 아이콘을 사용해야한다면 확장성에서 나빠질 것 같아
하지만 아직 feed 하위로 포함되어있는 컴포넌트인 만큼 feed에서 사용으로 사용처를 좁힐 수 있으니 아이콘을 확장할 일은 당장은 없지 않을꺄?
There was a problem hiding this comment.
호오, 아이콘을 외부에서 직접 주도록 하는 방법밖에 생각이 안 나서 이렇게 했었는데, type을 주고 어떤 아이콘을 보여줄지는 그 컴포넌트가 직접 결정하게 하는 게 더 괜찮아 보이네
비록 지금은 입장/퇴장 알림, 날짜 변경 알림 외에는 어떤 종류의 알림이 더 등장할 지 불분명하나, 알림의 종류가 몇십 개가 될 것 같지는 않으니 이후 알림의 유형이 추가되도 type 에 더 다양한 값을 허용해 주는 식으로 변경해도 될 것 같다
확장성 면에서는 루루가 말한대로 나빠질 수 있겠으나 개발자의 의도를 벗어난 확장은 필요도 없을 뿐더러, 일관성을 저해하는 요소가 될 수 있다고 나도 생각해. type 을 사용하면 모든 의도를 다 드러낼 수 있으니 그 외의 확장은 필요 없겠지.
고로, 이 코멘트는 반영해서...
type = 'normal' | 'join' | 'leave' | 'date'로 해 두고,type값에 따라 보여주게 될 아이콘을 변경
하도록 리팩터링 해볼게
| @@ -43,12 +66,19 @@ const ThreadList = (props: ThreadListProps) => { | |||
| useIntersectionObserver(observeRef, onIntersect, hasNextPage); | |||
|
|
|||
| useEffect(() => { | |||
There was a problem hiding this comment.
useEffect 내부의 로직?
우선 75번째 줄까지는 그냥 리팩터링만 한 거니 로직의 변화가 없어, 그러니 설명하지 않을게
75번째 줄까지의 로직을 제외한 나머지 로직은, 페이지 개수에 변화가 있는 경우(= 초기 렌더링 or 무한 스크롤으로 인한 페이지 불러오기 발동) 실행되는데, 그 때마다 가장 오래된 채팅의 날짜를 업데이트하는 거야 -- 이 정보를 알아야 새로운 페이지를 불러올 때, 이미 있었던 가장 오래된 채팅의 날짜와 비교해서 날짜를 구분하는 알림을 넣어줘야 할 지 말지를 판단할 수 있을 것 같아
threadPages.pages.at(-1)?.threads.at(-1)?.createdAt 에서 threadPages.pages.at(-1)?.threads.at(-1) 부분은, 그림과 같아, 주의할 점은 내가 처리하는 부분은 루루가 스레드를 반대로 뒤집기 전이라는 것!
There was a problem hiding this comment.
만약 하루에 대화를 엄청 많이해서 여러 페이지에 걸쳐서 대화가 되어있는거야. 근데 내가 페이지 첫 로드를 해서 이전 페이지의 정보(무한 페이지 조회 전)을 가지고 있지 않다면 어떻게 나오는 거야?
이 질문에 대한 대답을 한 문장으로 요약하면, 확실한 상황이 아닌 한, 알림 컴포넌트를 렌더링하지 않는다야.
이것만으로는 설명이 너무 빈약하니, 여러 사례들을 들어줄게.
어제 친 채팅을 시간 순으로
- 당연하지만
$B_2$ 는 해당 날의 첫 번째 채팅이 아니니까.
-
$B_1$ 는 분명 해당 날의 첫 번째 채팅이지만, 프론트엔드 입장에서 생각해 보자 -- 아직 무한 스크롤이 더 진행되지 않았기 때문에,$B_1$ 이 해당 날의 첫 번째 채팅인지가 불확실한 상황이야. 따라서 날짜 컴포넌트는 보이지 않아.
-
$A_3$ 도 무한스크롤을 통해 렌더링된 상황이고,$B_1$ 이 해당 날의 첫 번째 채팅임이 확실해진 상황이야. 따라서 날짜 컴포넌트를 보여 주게 돼.
-
$A_1$ 은 분명 해당 날의 첫 번째 채팅이지만,$B_1$ 의 사례와 마찬가지로 불확실한 상황이므로, 날짜 컴포넌트는 보이지 않아. - 단, 이 상태에서 한 번 더 무한스크롤을 해 렌더링을 시도할 경우, 마지막 페이지임을 알 수 있게 되고,
$A_1$ 이 그 날의 첫 번째 채팅임이 확실해져. 따라서, 이 때는 날짜 컴포넌트가 보이게 돼.
따라서, 이를 바탕으로 루루의 질문에 답하자면:
- 계속해서 무한스크롤을 통해 렌더링을 시도해도, 더 이상 페이지를 불러올 수 없는 경우거나, 다른 날짜의 채팅이 보이게 되지 않은 한, 날짜 컴포넌트는 렌더링 되지 않아.
이렇게 구현한 이유는, 해당 채팅이 첫 번째 채팅이 아님에도 날짜 알림으로 구분선을 그려준 경우, 사용자가 해당 채팅을 첫 번째 채팅으로 오인할 가능성이 있어 보이고, 사용자가 무한 스크롤을 할 때 새로운 채팅이 보이면 기존의 날짜 알림은 사라지게 되어 어색하다고 느낄 수 있다고 판단해서야.
프론트엔드는 서버가 아니다 보니까, 새로운 채팅을 더 불러오지 않는 한 알고 있는 정보는 한정적이고, 한계는 있다고 생각해. 그래서, 제한 조건 안에서 내가 생각하는 최선의 선택을 해 봤어.
어이쿠 얘기가 너무 길어졌네, 루루가 적어준 코멘트 이야기도 해야 하는데.
type에 대한 제안은 정말 좋은 것 같아서, 새롭게 추가해 봤는데 확인해 볼래?- 질문의 경우
useEffect내부의 코드를 질문한 것 같은데 설명글 적었어
리뷰 감사!
| content: string; | ||
| icon?: ReactElement; |
There was a problem hiding this comment.
호오, 아이콘을 외부에서 직접 주도록 하는 방법밖에 생각이 안 나서 이렇게 했었는데, type을 주고 어떤 아이콘을 보여줄지는 그 컴포넌트가 직접 결정하게 하는 게 더 괜찮아 보이네
비록 지금은 입장/퇴장 알림, 날짜 변경 알림 외에는 어떤 종류의 알림이 더 등장할 지 불분명하나, 알림의 종류가 몇십 개가 될 것 같지는 않으니 이후 알림의 유형이 추가되도 type 에 더 다양한 값을 허용해 주는 식으로 변경해도 될 것 같다
확장성 면에서는 루루가 말한대로 나빠질 수 있겠으나 개발자의 의도를 벗어난 확장은 필요도 없을 뿐더러, 일관성을 저해하는 요소가 될 수 있다고 나도 생각해. type 을 사용하면 모든 의도를 다 드러낼 수 있으니 그 외의 확장은 필요 없겠지.
고로, 이 코멘트는 반영해서...
type = 'normal' | 'join' | 'leave' | 'date'로 해 두고,type값에 따라 보여주게 될 아이콘을 변경
하도록 리팩터링 해볼게
| @@ -43,12 +66,19 @@ const ThreadList = (props: ThreadListProps) => { | |||
| useIntersectionObserver(observeRef, onIntersect, hasNextPage); | |||
|
|
|||
| useEffect(() => { | |||
There was a problem hiding this comment.
useEffect 내부의 로직?
우선 75번째 줄까지는 그냥 리팩터링만 한 거니 로직의 변화가 없어, 그러니 설명하지 않을게
75번째 줄까지의 로직을 제외한 나머지 로직은, 페이지 개수에 변화가 있는 경우(= 초기 렌더링 or 무한 스크롤으로 인한 페이지 불러오기 발동) 실행되는데, 그 때마다 가장 오래된 채팅의 날짜를 업데이트하는 거야 -- 이 정보를 알아야 새로운 페이지를 불러올 때, 이미 있었던 가장 오래된 채팅의 날짜와 비교해서 날짜를 구분하는 알림을 넣어줘야 할 지 말지를 판단할 수 있을 것 같아
threadPages.pages.at(-1)?.threads.at(-1)?.createdAt 에서 threadPages.pages.at(-1)?.threads.at(-1) 부분은, 그림과 같아, 주의할 점은 내가 처리하는 부분은 루루가 스레드를 반대로 뒤집기 전이라는 것!
| content: string; | ||
| icon?: ReactElement; |
[FE] 알림 컴포넌트의 UI를 변경하고, 채팅을 입력한 날짜를 알림 컴포넌트를 활용하여 알리도록 개선
이슈번호
PR 내용
본 PR에서는 크게 아래의 두 작업을 수행하였다.
기능 자체는 간단한 편이기에 참고 자료 의 스크린샷을 통해 각 기능을 설명하고자 한다.
참고자료
1. 알림 컴포넌트의 UI를 변경
2. 채팅을 입력한 날짜를 알림 컴포넌트를 이용해 표시하도록 변경
의논할 거리
1번 기능을 수행하는 건 어렵지 않았는데 2번 기능을 수행할 때 거대한 컴포넌트를 건드리다 보니 코드가 좀 중구난방인데 개선되면 좋을 부분 있어보이면 찍어주십시오...
쉬운 줄 알았는데 은근 예외처리할 거 많았던 듯?