-
-
Notifications
You must be signed in to change notification settings - Fork 198
Fix MessageList not updating/scrolling down on incoming message #1229
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
Conversation
@@ -41,8 +41,10 @@ export default function MessageList ({ chat, refComposer, locationStreamingEnabl | |||
|
|||
useEffect(() => { | |||
if (scrollToBottomIfClose === false) return | |||
if (messageListRef.current.scrollHeight - messageListRef.current.scrollTop > 400) { | |||
// we are close to bottom so scroll down to show the new message | |||
// For whatever reason the scrollTop is ~1000 off... |
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.
For the protocol: I do not like this "we don't know what its doing lets fix it anyway somehow" approach
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.
Me neither. Feel free to figure it out, didn't find the reason for this. Actually i threw out that code path as it's weird ux.
The scrolling is not working (I only tested it with the window in the background) but the update bug is fixed so I will approve |
@@ -66,8 +66,7 @@ chatStore.reducers.push(({ type, payload, id }, state) => { | |||
messages: { | |||
...state.messages, | |||
...payload.messagesIncoming | |||
}, | |||
scrollToBottomIfClose: true |
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.
this line is needed: in my tests it only scrolls down if this is true. Why did you remove it?
For whatever reason some commits aren't shown in this pr, this is probably why stuff is missing. Not sure why. |
Fixes #1222