FIX: more consistent scroll to bottom (#20634)

This fix uses direct `scrollTop` manipulation instead of `scrollIntoView` when we are certain we actually want the bottom of the screen. This avoids a range of issues especially in safari but also chrome where the scroll position was not correct at the end of `scrollIntoView`, especially due to images.
This commit is contained in:
Joffrey JAFFEUX 2023-03-10 16:25:21 +01:00 committed by GitHub
parent 7df40fc905
commit 60ce3d24fa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 21 deletions

View File

@ -83,7 +83,7 @@
</div> </div>
<ChatScrollToBottomArrow <ChatScrollToBottomArrow
@scrollToBottom={{this.scrollToBottom}} @scrollToBottom={{this.scrollToLatestMessage}}
@hasNewMessages={{this.hasNewMessages}} @hasNewMessages={{this.hasNewMessages}}
@show={{or this.needsArrow @channel.canLoadMoreFuture}} @show={{or this.needsArrow @channel.canLoadMoreFuture}}
@channel={{@channel}} @channel={{@channel}}

View File

@ -187,11 +187,19 @@ export default class ChatLivePane extends Component {
this.scrollToMessage(findArgs["targetMessageId"], { this.scrollToMessage(findArgs["targetMessageId"], {
highlight: true, highlight: true,
}); });
} else if (fetchingFromLastRead) { return;
this.scrollToMessage(findArgs["targetMessageId"]);
} else if (messages.length) {
this.scrollToMessage(messages[messages.length - 1].id);
} }
if (
fetchingFromLastRead &&
messages.length &&
findArgs["targetMessageId"] !== messages[messages.length - 1].id
) {
this.scrollToMessage(findArgs["targetMessageId"]);
return;
}
this.scrollToBottom();
}) })
.catch(this._handleErrors) .catch(this._handleErrors)
.finally(() => { .finally(() => {
@ -268,15 +276,9 @@ export default class ChatLivePane extends Component {
// Edge case for IOS to avoid blank screens // Edge case for IOS to avoid blank screens
// and/or scrolling to bottom losing track of scroll position // and/or scrolling to bottom losing track of scroll position
schedule("afterRender", () => { if (!loadingPast && (this.capabilities.isIOS || !this.isScrolling)) {
if (
!this._selfDeleted &&
!loadingPast &&
(this.capabilities.isIOS || !this.isScrolling)
) {
this.scrollToMessage(messages[0].id, { position: "end" }); this.scrollToMessage(messages[0].id, { position: "end" });
} }
});
}) })
.catch(() => { .catch(() => {
this._handleErrors(); this._handleErrors();
@ -449,7 +451,7 @@ export default class ChatLivePane extends Component {
lastUnreadVisibleMessage = lastUnreadVisibleMessage.previousMessage; lastUnreadVisibleMessage = lastUnreadVisibleMessage.previousMessage;
if ( if (
!lastUnreadVisibleMessage && !lastUnreadVisibleMessage ||
lastReadId > lastUnreadVisibleMessage.id lastReadId > lastUnreadVisibleMessage.id
) { ) {
return; return;
@ -463,6 +465,27 @@ export default class ChatLivePane extends Component {
@action @action
scrollToBottom() { scrollToBottom() {
schedule("afterRender", () => { schedule("afterRender", () => {
if (this._selfDeleted) {
return;
}
// A more consistent way to scroll to the bottom when we are sure this is our goal
// it will also limit issues with any element changing the height while we are scrolling
// to the bottom
this._scrollerEl.scrollTop = -1;
this.forceRendering(() => {
this._scrollerEl.scrollTop = 0;
});
});
}
@action
scrollToLatestMessage() {
schedule("afterRender", () => {
if (this._selfDeleted) {
return;
}
if (this.args.channel.canLoadMoreFuture) { if (this.args.channel.canLoadMoreFuture) {
this._fetchAndScrollToLatest(); this._fetchAndScrollToLatest();
} else if (this.args.channel.messages?.length > 0) { } else if (this.args.channel.messages?.length > 0) {
@ -589,7 +612,7 @@ export default class ChatLivePane extends Component {
// If we are at the bottom, we append the message and scroll to it // If we are at the bottom, we append the message and scroll to it
const message = ChatMessage.create(this.args.channel, data.chat_message); const message = ChatMessage.create(this.args.channel, data.chat_message);
this.args.channel.addMessages([message]); this.args.channel.addMessages([message]);
this.scrollToBottom(); this.scrollToLatestMessage();
} else { } else {
// If we are almost at the bottom, we append the message and notice the user // If we are almost at the bottom, we append the message and notice the user
const message = ChatMessage.create(this.args.channel, data.chat_message); const message = ChatMessage.create(this.args.channel, data.chat_message);
@ -602,7 +625,7 @@ export default class ChatLivePane extends Component {
const message = this.args.channel.findMessage(data.chat_message.id); const message = this.args.channel.findMessage(data.chat_message.id);
if (message) { if (message) {
message.cooked = data.chat_message.cooked; message.cooked = data.chat_message.cooked;
this.scrollToBottom(); this.scrollToLatestMessage();
} }
} }
@ -727,7 +750,7 @@ export default class ChatLivePane extends Component {
this.loading = false; this.loading = false;
this.sendingLoading = false; this.sendingLoading = false;
this._resetAfterSend(); this._resetAfterSend();
this.scrollToBottom(); this.scrollToLatestMessage();
}); });
} }
@ -744,7 +767,7 @@ export default class ChatLivePane extends Component {
this.args.channel.addMessages([stagedMessage]); this.args.channel.addMessages([stagedMessage]);
if (!this.args.channel.canLoadMoreFuture) { if (!this.args.channel.canLoadMoreFuture) {
this.scrollToBottom(); this.scrollToLatestMessage();
} }
return this.chatApi return this.chatApi
@ -755,7 +778,7 @@ export default class ChatLivePane extends Component {
upload_ids: stagedMessage.uploads.map((upload) => upload.id), upload_ids: stagedMessage.uploads.map((upload) => upload.id),
}) })
.then(() => { .then(() => {
this.scrollToBottom(); this.scrollToLatestMessage();
}) })
.catch((error) => { .catch((error) => {
this._onSendError(stagedMessage.id, error); this._onSendError(stagedMessage.id, error);
@ -918,7 +941,7 @@ export default class ChatLivePane extends Component {
editButtonClicked(messageId) { editButtonClicked(messageId) {
const message = this.args.channel.findMessage(messageId); const message = this.args.channel.findMessage(messageId);
this.editingMessage = message; this.editingMessage = message;
this.scrollToBottom(); this.scrollToLatestMessage();
this._focusComposer(); this._focusComposer();
} }