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

Update "jump to bottom" button visibility upon a change in timeline items #131

Closed
kevinaboos opened this issue Aug 29, 2024 · 4 comments · Fixed by #247 · May be fixed by #206
Closed

Update "jump to bottom" button visibility upon a change in timeline items #131

kevinaboos opened this issue Aug 29, 2024 · 4 comments · Fixed by #247 · May be fixed by #206
Assignees
Labels
good first issue Good for newcomers help wanted Looking for help from anyone!

Comments

@kevinaboos
Copy link
Member

Currently the visibility of the "jump to bottom" button is based solely on the scrolling actions emitted by the PortalList.

This works, but misses one key scenario in which the actual underlying contents of the timeline (its items Vector) is changed by an update received in the background task.
For example, when a new message is received (and pushed onto the end of the timeline), but the user is not currently scrolled all the way down, the PortalList will not auto-tail (automatically track and scroll down to the last/latest/newest item). Thus, it isn't immediately obvious to the user that a new message has actually arrived.

To remedy this, we should (at the very least) update the visibility of the jump_to_bottom_button upon receiving a TimelineUpdate::NewItems() update. That way, the button would be re-shown again any time new messages exist beyond the current viewport.


Stretch goal: it'd be even nicer to display a UI element showing that unread messages are below the current viewport.

Element does this for both the upwards direction and the downwards direction. Here's an example of Element showing the count of unread messages beneath the current view:
image

And here's an example of Element's blue-green dot that is rendered as an ornament atop the "jump upwards to unread messages" button:
image

@smarizvi110
Copy link
Contributor

smarizvi110 commented Sep 2, 2024

Hello, just got a chance to look at this.
Yes, this is one of the issues the logic did not account for in the implementation of the button. I can try and look more into TimelineUpdate::NewItems().
I do have question on this, though. Would it be helpful in adding another attribute to the messages in PortalList, such as something like read? It could be false by default, and made true based on certain criteria to determine if the message has actually been read or not. Additionally, if a given message for some reason has this attribute set to false, such as when loading up the chat by opening it, but a more recent message has the attribute turn true, then the messages above would also have that attribute set to true.
However, this update logic does seem performance intensive, and I'm not exactly sure how it would account for showing actually unread messages that the client received but hasn't opened yet for the sake of showing how many unread messages there are as an icon badge, notification, or in the stretch goal you mentioned...
I think if we can figure this out, we could even come up with a more robust system for determining the visibility for the button that hopefully won't be problematic on Windows as it currently seems to be, unfortunately.

Also, is there a slack for this project? I'd love to join if there is one!

@kevinaboos
Copy link
Member Author

kevinaboos commented Sep 4, 2024

Hello, just got a chance to look at this. Yes, this is one of the issues the logic did not account for in the implementation of the button. I can try and look more into TimelineUpdate::NewItems().

Great, thanks for looking into this! It should be pretty quick/straightforward to add this logic into the handler for the TimelineUpdate::NewItems variant.

I do have question on this, though. Would it be helpful in adding another attribute to the messages in PortalList, such as something like read? It could be false by default, and made true based on certain criteria to determine if the message has actually been read or not. Additionally, if a given message for some reason has this attribute set to false, such as when loading up the chat by opening it, but a more recent message has the attribute turn true, then the messages above would also have that attribute set to true. However, this update logic does seem performance intensive, and I'm not exactly sure how it would account for showing actually unread messages that the client received but hasn't opened yet for the sake of showing how many unread messages there are as an icon badge, notification, or in the stretch goal you mentioned... I think if we can figure this out, we could even come up with a more robust system for determining the visibility for the button that hopefully won't be problematic on Windows as it currently seems to be, unfortunately.

The "read" or "unread" state of a message is actually totally separate from this issue, and it's decently supported by the Matrix SDK itself. We already support retrieving and displaying the logged-in user's ReadMarker, and another contributor is currently working on sending read receipts based on the current viewport (See #124, which addresses #86)

Also, is there a slack for this project? I'd love to join if there is one!

Nope, Slack is expensive! 😅 Also, we can't be using Slack for a matrix project, right? haha

We do have a matrix room where all public Robrix conversations can take place. It's linked at the top of the README: https://matrix.to/#/#robius-robrix:matrix.org

@kevinaboos kevinaboos added good first issue Good for newcomers help wanted Looking for help from anyone! labels Sep 6, 2024
@smarizvi110
Copy link
Contributor

smarizvi110 commented Sep 8, 2024

Great, thanks for looking into this! It should be pretty quick/straightforward to add this logic into the handler for the TimelineUpdate::NewItems variant.

Yep I think I have a few ideas to mess around with on that front

The "read" or "unread" state of a message is actually totally separate from this issue, and it's decently supported by the Matrix SDK itself. We already support retrieving and displaying the logged-in user's ReadMarker, and another contributor is currently working on sending read receipts based on the current viewport (See #124, which addresses #86)

That makes perfect sense!

Nope, Slack is expensive! 😅 Also, we can't be using Slack for a matrix project, right? haha
We do have a matrix room where all public Robrix conversations can take place. It's linked at the top of the README: https://matrix.to/#/#robius-robrix:matrix.org

Haha yes that makes perfect sense. I'll join that then!

@alanpoon
Copy link
Contributor

Combine PR #206 tested together with sending read receipt by scroll

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