Skip to content
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

fix(MessagesList): do not replace container when switching conversations #14327

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Feb 10, 2025

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏡 After

2025-02-10_09h13_03.mp4

🏁 Checklist

  • 🌏 Tested with different browsers / clients:
    • Chromium (Chrome / Edge / Opera / Brave)
    • Firefox
    • Safari
    • Talk Desktop
    • Not risky to browser differences / client
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
…hats

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@ShGKme
Copy link
Contributor

ShGKme commented Feb 10, 2025

Could you explain the change?

@@ -7,7 +7,6 @@
<!-- size and remain refer to the amount and initial height of the items that
are outside of the viewport -->
<div ref="scroller"
:key="token"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was added during messages list soft update refactoring, but now it seems to be covered with messagesList watcher
With key in place, ResizeObserver loses reference to the scroller

@@ -387,6 +386,9 @@ export default {
this.$refs.scroller.scrollTo({
top: this.$refs.scroller.scrollHeight,
})
} else if (this.$refs.scroller.clientHeight === this.$refs.scroller.scrollHeight) {
// chat is not scrollable
this.setChatScrolledToBottom(true)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For non-scrollable chats, removes the scroll to bottom on resize

@@ -931,10 +933,10 @@ export default {
this.displayMessagesLoader = false
this.debounceUpdateReadMarkerPosition()
return
} else if (scrollHeight > clientHeight) {
this.setChatScrolledToBottom(false)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After swtiching to conversation, handleScroll is triggered. Regardless of container height, it sets value to false (whereas it shouldn't for non-scrollable chats)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants