[CLNP-7850]Add feature to scroll to top of new message instead of bottom #1378
Conversation
✅ Deploy Preview for sendbird-uikit-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
||
| rafId = requestAnimationFrame(() => { | ||
| forceScrollToMessage(messageScrollRef, message); | ||
| setNewMessageIds([]); |
There was a problem hiding this comment.
newMessageIds 가 여러개 일때 MessageView를 가지고있는 List에서 렌더링순서나 아니면 메세지 정렬방식, 또는 requestAnimationFrame 의 함수가 실제로 호출되는 순서에 따라서 race condition으로 버그 가능성이 있어 보입니다. scroll 해야할 메세지를 MessageView 에서 선택하는것보다는 상위 MessageList 같은곳에서 선택해서 처리하는게 좀더 안전한 방식일것 같은데 의견 부탁드립니다
There was a problem hiding this comment.
일단 newMessageIds를 onMessagesReceived가 호출이 되었을때만 set을 하고 있는데, 실제로는 1개만 들어 오고 있긴 합니다.
채널 진입시에는 newMessageIds = [] 또는 undefined 일거라..
Scroll의 조건이 Message Content의 height가 현재 message list의 height보다 넘어가는지를 체크를 해야 하는데, 이럴러면 Content가 다 그려졌는지에 대한 보장이 필요해서 messageView쪽에서 다 그려진 여부만 체크를 했고, 실제 움직이는건 MessageList에서 처리를 하긴 했습니다.
There was a problem hiding this comment.
onMessagesReceived 가 1개의 메세지를 보장하지않고 서버에서 여러개의 메세지를 보낼수 있는 상태라 아래와 같은 케이스를 고민해볼수 있을것 같습니다.
- 1,2 두개의 새로운 메세지가 왔을때
- useLayoutEffect 에서 호출되는 requestAnimationFrame 이 바로 호출되는게 아니라 큐잉후 특정 시점에 한번에 호출
- if (newMessageIds?.length > 0 && newMessageIds.includes(message.messageId)) 조건을 1,2 메세지 둘다 통과 하여 requestAnimationFrame 호출
- requestAnimationFrame 큐잉순서에따라서 1번 2번 메세지 모두 forceScrollToMessage(messageScrollRef, message);
setNewMessageIds([]); 호출 - 1번 메세지의 top 에 포커싱이 되어야하지만 2번메세지의 top 에 포커싱 되는현상 발생
위 케이스에 대해서 문제없다면 괜찮을것 같습니다
There was a problem hiding this comment.
흠 .. 이게 저희 onMessagesReceived는 message를 []로 받기는 하는데요..
이게 MESG가 내려와야 발생을 하는거라.. MESG에서는 Message Payload가 1개만 존재를 해서.... 2개 이상의 MEssage가 오지는 않을 거 같습니다.
// PR title (Required)
[feat]: Add feature to scroll to top of new message instead of bottom
// PR description (Optional)
Added
isFocusOnLastMessageas a prop toSendbirdProvider.If this prop is set to true, then when a new message arrives and its height exceeds the screen, the scroll position will not automatically jump to the bottom. Instead, it will move to the top position of that new message.
// Footer (Recommended)
feat CLNP-7850
Changelogs
isFocusOnLastMessageprop toSendbirdProviderWhen set to true, if a newly received message is taller than the viewport, the scroll position will not auto-jump to the bottom.
Instead, the view scrolls to the top of the new message, keeping the start of the message in focus.
Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If unsure, ask the members.This is a reminder of what we look for before merging your code.
External Contributions