Skip to content

Conversation

@PVince81
Copy link
Member

@PVince81 PVince81 commented Nov 4, 2020

When embedded into the right sidebar, the scrolling is achieved by a
parent element instead of the MessagesList itself.

To be able to listen to and control the scrolling, now a reference to
the scroll container is passed in.

Fixes #4527

This issue was due to the recent changes in scrollbar containment in the sidebar.

Manual tests

Scrolling

  • scroll up in main chat view: chevron-down button appears
  • scroll up in call sidebar chat view: chevron-down button appears
  • scroll up in files sidebar chat view: chevron-down button appears
  • auto scroll to bottom works in main chat view (sticky)
  • auto scroll to bottom works in call sidebar chat view (sticky)
  • auto scroll to bottom works in files sidebar chat view (sticky)

Layout

  • call sidebar has proper height when few messages
  • files sidebar has proper height when few messages

When embedded into the right sidebar, the scrolling is achieved by a
parent element instead of the MessagesList itself.

To be able to listen to and control the scrolling, now a reference to
the scroll container is passed in.

Signed-off-by: Vincent Petry <[email protected]>
@PVince81 PVince81 added bug regression feature: chat 💬 Chat and system messages labels Nov 4, 2020
@PVince81 PVince81 added this to the 💚 Next Major (21) milestone Nov 4, 2020
@PVince81 PVince81 self-assigned this Nov 4, 2020
@PVince81
Copy link
Member Author

PVince81 commented Nov 4, 2020

this took me a while as I first tried to use computed properties but bumped against this issue: https://stackoverflow.com/questions/43531755/using-refs-in-a-computed-property

One should not use "$refs" in computed properties as it might not be defined initially, and also $refs is not reactive so even if it becomes defined the computed property won't re-evaluate. Hence I've used watchers instead.

@PVince81
Copy link
Member Author

PVince81 commented Nov 4, 2020

back to draft as I'm going to investigate the size issue #4527 (comment) and there's a slight risk that I might need to reshuffle the containers to make it work 🙈

@PVince81 PVince81 marked this pull request as draft November 4, 2020 12:50
@PVince81
Copy link
Member Author

PVince81 commented Nov 4, 2020

I knew it... to fix the container I have to set height: 1px on it as per https://stackoverflow.com/a/21836870

zero-height-magic mkv

but doing so breaks the scrolling events and measurements on that element.

needs further research

@PVince81
Copy link
Member Author

PVince81 commented Nov 4, 2020

I found a simpler fix that fixes both... #4530

@PVince81 PVince81 closed this Nov 4, 2020
@nickvergessen nickvergessen deleted the bugfix/4527/fix-chat-sidebar-scroll-container branch November 12, 2020 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scroll to bottom button is missing

2 participants