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

Message panel scrolls to the bottom after downloading a submitted file, possibly moving the file entry out of view #144

Closed
zenmonkeykstop opened this issue Nov 10, 2018 · 8 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@zenmonkeykstop
Copy link
Contributor

If a message thread has a downloaded file at the beginning, and the user clicks Download, the message panel is redrawn once the file is downloaded and decrypted, scrolled all the way to the bottom.

If the thread is longer than can be displayed all at once, this means that the thread appears to jump down to the bottom messages, putting the file that was just downloaded out of view.

It would probably be better for the message thread to be redrawn at the same point as it was when the user clicked Download.

@redshiftzero redshiftzero added the help wanted Extra attention is needed label Nov 10, 2018
@redshiftzero redshiftzero added this to the 0.1.0beta milestone Nov 10, 2018
@redshiftzero redshiftzero added the bug Something isn't working label Nov 11, 2018
@sssoleileraaa
Copy link
Contributor

Bug confirmed, and yes it was surprising for it to no longer be in view once downloaded.

@deeplow
Copy link
Contributor

deeplow commented Feb 14, 2019

Found the bug.
The following lines in gui/widgets.py tell the conversation to go to the bottom whenever the length of the conversation changes.

        # Completely unintuitive way to ensure the view remains scrolled to the
        # bottom.
        sb = self.scroll.verticalScrollBar()
        sb.rangeChanged.connect(self.move_to_bottom)

It must have been assumed that the conversation would only grow if new content was added, but in fact the "Download" bubble of a file differs in size from "Open" by 15 (pixels I am assuming). Therefore, it also scrolls to the bottom once the file is downloaded

@deeplow
Copy link
Contributor

deeplow commented Feb 14, 2019

The issue also happens when there are various <Message not yet downloaded> and the user presses refresh. It scrolls to the bottom once again because the length of the widget with <Message not yet downloaded> is different (from a multi-line message)

@redshiftzero
Copy link
Contributor

redshiftzero commented Feb 14, 2019

We just chatted about this in Jitsi and decided on the following Slack-like behavior:

  1. If user is scrolled to the bottom of the conversation view, then new messages as they appear will also scroll to the bottom (see also: conversation should appear scrolled all the way to the bottom #61 where this behavior was first implemented).
  2. If user is scrolled up in the conversation view (i.e. not at the bottom of the panel), then the conversation will stay at the current position as new messages come in.

If people think this is confusing/incorrect behavior, please comment!

@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented Feb 14, 2019

This is the behavior that I would expect as a user.

Eventually there could be a way to show notifications in a popup with a go-to/ jump link that takes you to new activity if something happened outside the scope of the current view, or a recent activities view to see things like a file finishes downloading or if there are unseen messages, as discussed here: #187

@deeplow
Copy link
Contributor

deeplow commented Feb 15, 2019

I agree on that behavior. The simplest way to solve this issue would be to modify the self.move_to_bottom function to act more something like update_conversation_position (implying renaming) which would go to the bottom if the conversation was in the bottom too before the last update. The thing is that the self.move_to_bottom is called once the conversation length changes, once it is called the scrollbar is no longer at the bottom.
This means that simply adding a condition like this to self.move_to_bottom, would not work

current_val = self.scroll.verticalScrollBar().value()

if current_val == max_val:
    self.scroll.verticalScrollBar().setValue(max_val)

we would need to add some tolerance such as being within a viewport's height from the bottom:

current_val = self.scroll.verticalScrollBar().value()
viewport_height = self.scroll.viewport().height()

if current_val + viewport_height > max_val:
    self.scroll.verticalScrollBar().setValue(max_val)

This means that the conversation would scroll to the bottom. Either this or instead storing the max_val from the last update.

But this solution seem quite right and does not provide a solution to:

@deeplow
Copy link
Contributor

deeplow commented Feb 15, 2019

I am trying to make it in a way that can keep track of the of the last conversation item added and detect whether or not is if visible.

@sssoleileraaa
Copy link
Contributor

@deeplow - your tolerance solution makes sense. I like how simple it is and how it keeps the scope to fixing this bug. Since we don't have a new/seen/unseen notifier yet as discussed in #187, I think it's a good solution.

deeplow added a commit to deeplow/securedrop-client that referenced this issue Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants