Add "jump to bottom" button to the room timeline screen

[smarizvi110]

Fix #85

This PR for now only has a basic button added in the top right corner of the message Timeline.

The issue has a few comments by me that ask for some clarification to polish this button further.

Additionally, parameters for PortalList::smooth_scroll_to_end() are not final. Perhaps they should be tweaked such that the speed of the scroll depends on how far up you've scrolled and how long jumping to the bottom should take as an interval that we can define? And max_delta could be some similar number we define, perhaps a certain percentage of speed?

[kevinaboos]

@smarizvi110 I pushed a small commit that wraps the jump_to_bottom_button in a View. This allows us to set that as an flow: Overlay with a bottom right alignment (align: { x: 1.0, y: 1.0 }), which gives you the positioning that you were looking for.

I also simplified the Rust code a bit, as you can obtain a ref to the PortalList element with a single DSL query, via id!(timeline.list).

I left a few things as to-do items that you can tackle (if you want):

• Setting visibility of the jump_to_bottom_view to false by default, and then setting it to true only when the timeline is not scrolled to the bottom (more accurately, when the last few messages are not visible -- you don't want the button to show up too soon)
• The params passed into smooth_scroll_to_end(). I'll let you fine-tune that, but IMHO we want to shoot for a total time of around 1.5 - 2 seconds for the entire scrolling animation.
  • I've never used this scrolling function before, so I'm not sure of the best way to do it, but I like your ideas about using relative/percentage values for max_delta and speed, that sounds good to me.

[smarizvi110]

@smarizvi110 I pushed a small commit that wraps the jump_to_bottom_button in a View. This allows us to set that as an flow: Overlay with a bottom right alignment (align: { x: 1.0, y: 1.0 }), which gives you the positioning that you were looking for.

I also simplified the Rust code a bit, as you can obtain a ref to the PortalList element with a single DSL query, via id!(timeline.list).

I left a few things as to-do items that you can tackle (if you want):

• Setting visibility of the jump_to_bottom_view to false by default, and then setting it to true only when the timeline is not scrolled to the bottom (more accurately, when the last few messages are not visible -- you don't want the button to show up too soon)

• The params passed into smooth_scroll_to_end(). I'll let you fine-tune that, but IMHO we want to shoot for a total time of around 1.5 - 2 seconds for the entire scrolling animation.

  • I've never used this scrolling function before, so I'm not sure of the best way to do it, but I like your ideas about using relative/percentage values for max_delta and speed, that sounds good to me.

Awesome, I'll take a look! Wasn't sure about the syntax for getting the ref but I'll keep this in mind! Thanks for the help with the UI too!

And yes I'll be trying my hand at the visibility and scroll params!

[smarizvi110]

@kevinaboos I added a function that can help with deciding when to make the button visible, and it attempted to leverage PortalListRef::scroll_position(). However this function doesn't seem to return what I thought it would. For instance, upon simply scrolling up with a fixed window size, here's what the log showed on Linux using X11:

src/home/room_screen.rs:1926:5 - Scroll position: 5.109999999999943, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -7.890000000000057, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: 4.109999999999943, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -16.49000000000005, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -4.490000000000052, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: 7.509999999999948, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -95.09000000000007, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -83.09000000000007, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -71.09000000000007, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -59.090000000000074, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -47.090000000000074, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -35.090000000000074, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -23.090000000000074, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -11.090000000000074, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: 0.9099999999999255, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -12.090000000000074, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -0.09000000000007446, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: 11.909999999999926, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -90.6900000000001, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -78.6900000000001, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -66.6900000000001, viewport height: 720
src/home/room_screen.rs:1926:5 - Scroll position: -54.6900000000001, viewport height: 720

and on Windows:

src\home\room_screen.rs:1926:5 - Scroll position: 100.54000000000002, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 62.22, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 7.939999999999969, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 3.19999999999996, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 8.599999999999937, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 70.88999999999993, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 7.459999999999894, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 12.859999999999872, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: -17.97000000000014, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 102.02999999999986, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 196.14999999999984, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 18.179999999999808, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 12.45999999999978, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 107.45999999999978, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 30.039999999999765, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 220.03999999999976, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 325.1899999999997, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 87.84999999999968, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 93.24999999999966, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 113.24999999999966, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 162.49999999999963, viewport height: 693
src\home\room_screen.rs:1926:5 - Scroll position: 117.89999999999961, viewport height: 693

I'm not quite sure how to get information regarding how far up the user has scrolled in pixels from the very bottom. Thank you!

[smarizvi110]

After further analysis, it seems the Windows log indicates the scroll amount per scroll action. The values aren't very consistent, but I tested the cumulative values by scrolling so that the initial top of the screen content was now at the bottom. The total wasn't far off from the viewport height, but not exactly the same, which makes sense since the viewport covers the entire Roomscreen, not just the Timeline. Therefore, by tracking cumulative scrolls and comparing them to the viewport height, we can decide when to show the button. To prevent negative cumulative values when the user has scrolled to the bottom, we can clamp the cumulative scroll to a minimum of zero. If the viewport size changes, the visibility check function must be called again, which needs to be implemented in some way.

On Linux, the scroll values are inconsistent, often showing several negative values despite only scrolling up. Windows showed just one negative value. This might be related to how messages are fetched and added to the timeline while scrolling, affecting the scroll distance. If this is the case, we might need to reconsider our approach.

[kevinaboos]

yeah, i am not sure how scroll_position works in Makepad, perhaps that is not the best way to determine this. I'm not much of a UI expert so it's hard to say.

I think the best option is to check out how Moxin does this (Moxin is another one of our Robius + Makepad projects) here: https://github.com/moxin-org/moxin/blob/97b08b0e69262e02b4f9eba70035d213d964d29e/src/chat/chat_panel.rs#L158

Search that file for jump and you'll see all the places where the jump_to_bottom button is used. Also, here is how they invoke smooth_scroll_to_end(): https://github.com/moxin-org/moxin/blob/97b08b0e69262e02b4f9eba70035d213d964d29e/src/chat/chat_panel.rs#L713

You can also take a look at portal_list_end_reached, that has some lightweight state tracking where the tail is and if the view is stuck to the bottom.

[kevinaboos]

Ok, here are some more updates after chatting with the Makepad & Moxin dev teams:

1. scroll_position is intended to return the delta (the positional difference) from the top of the current viewport to the top of the first item being displayed. So it doesn't do what we're hoping here, mostly because PortalList cannot possibly function like that because it cannot know the height of all of its items until it actually draws them. So offering a true "scroll position" function where it returns something like "the percentage of the entire view that we're currently scrolled to" is impossible.
   • Also, the auto_tail feature in the PortalList is currently broken.
2. Moxin's model list uses a reversed PortalList where the latest (most recent) message at the bottom is actually the item with index 0, not index N-1. So, in their other file model_list.rs, you can see how they roughly approximate whether the viewport is scrolled to the very top (which is the very bottom for them) here: https://github.com/moxin-org/moxin/blob/97b08b0e69262e02b4f9eba70035d213d964d29e/src/landing/model_list.rs#L140

[smarizvi110]

Awesome, thank you! I'll take a look!

[smarizvi110]

@kevinaboos The button with visibility functionality was implemented. I decided to use the values they used over at Moxin for the params passed to smooth_scroll_to_bottom(), but as for determining whether or not to show the button I chose not to reverse the list or anything of the sort. Just some conditional boolean logic was sufficient. The comments in the code should elaborate on that! This isn't as dynamic of an approach as I initially hoped for unfortunately but I'm sure with some more work on Makepad this could be revisited at some point!

[kevinaboos]

Awesome, thanks so much for the contribution! I'll test it out soon and let you know how well it works on my machine.

[kevinaboos]

@kevinaboos The button with visibility functionality was implemented. I decided to use the values they used over at Moxin for the params passed to smooth_scroll_to_bottom(), but as for determining whether or not to show the button I chose not to reverse the list or anything of the sort. Just some conditional boolean logic was sufficient. The comments in the code should elaborate on that! This isn't as dynamic of an approach as I initially hoped for unfortunately but I'm sure with some more work on Makepad this could be revisited at some point!

Thanks, it's pretty close to being perfect, so nice work! I pushed a commit with a tiny bit of cleanup / efficiency improvements.

I did some testing on my mac, and the heuristic for determining whether the view is scrolled to the very bottom still needs a bit of fine tuning.

I captured a screen recording that demonstrates the issue here -- you can see flickering even when the view is scrolled all the way down. Towards the end of the video, the rapid flickering is caused when i attempt to scroll down further, even though the view is already scrolled to the very bottom.

https://github.com/user-attachments/assets/86b29f6f-edfd-4e79-b4d6-fc5e13865959

If you're able to figure out another technique for determining if the view is actually all the way at the bottom, then that's great! If not, I will ask the Moxin team if they have the same issue for their jump-to-bottom button; we can probably devise a more accurate way to do this.

[kevinaboos]

Here's the response from the Moxin team regarding the jump to bottom button:

... we faced the same problem. Honestly, we don't have a proper solution yet. The function I made on PortalList, further_items_bellow_exist, is where the issue originates and I was not able to fix it... maybe we should just remove this function until we have a more reliable implementation.

In Moxin, we came up with this creative workaround to solve the problem: https://github.com/moxin-org/moxin/commit/2f9db67403195b6d7e9a0dbc9fa78d51bb465923

Note it relies on adding an extra item to the PortalList, that is not visible, but it forces the portal list logic to invoke your rust code with item_id == total_number_of_items... and you are now "detecting" the list is rendering the bottom part. See https://github.com/moxin-org/moxin/commit/2f9db67403195b6d7e9a0dbc9fa78d51bb465923#diff-5ce5826d63b3c10a35884ba3e84b524227755729ec530a702227b74699eee8ecR834-R838

perhaps checking out their other implementation techniques could give you some ideas?

[smarizvi110]

Hmm Moxin's approach seems pretty interesting! I'll definitely take a look soon and see what can be done! And thanks for making the code more Rust like, really helps me learn a lot!

[smarizvi110]

I was taking a look at Makepad's further_items_bellow_exist() function and trying to see what exactly goes wrong there. If we can find a way to fix that, not only would it help Robrix but really anything that intends to use Makepad. I'm a bit confused though as to how to exactly go about making changes to a local copy of the branch that Robrix uses of Makepad. If I could get help on this, maybe I can take a look at the function and see what's causing it to be so inconsistent?

[kevinaboos]

I'm a bit confused though as to how to exactly go about making changes to a local copy of the branch that Robrix uses of Makepad.

Certainly, I can help with that. It's standard procedure for using git; in fact, the current version of Robrix is actually using my own customized fork of the makepad project, as you can see here in the Cargo.toml: https://github.com/project-robius/robrix/blob/8a60937cc0804b62a1df54b14f1114632a87ddb4/Cargo.toml#L20C1-L20C104

You can do the same thing by having your own local copy of makepad, which you can then modify. All you need to do is fork the Makepad project on github, then git clone it to your local machine, and then change that above line in the Cargo.toml file to point to your local version of makepad. Then, when you modify that version of makepad, it will automatically rebuild it when you build Robrix.

- makepad-widgets = { git = "https://github.com/kevinaboos/makepad", branch = "rik_with_old_font_stack" 
+ makepad-widgets = { path = "../makepad/widgets" }

(assuming your makepad directory is in the same directory as robrix)

Also, when you create a new branch, make sure you are branching off of my custom branch of makepad, which is on my remote (kevinaboos) with the branch name rik_with_old_font_stack: https://github.com/kevinaboos/makepad/tree/rik_with_old_font_stack

[smarizvi110]

I'm a bit confused though as to how to exactly go about making changes to a local copy of the branch that Robrix uses of Makepad.

Certainly, I can help with that. It's standard procedure for using git; in fact, the current version of Robrix is actually using my own customized fork of the makepad project, as you can see here in the Cargo.toml: https://github.com/project-robius/robrix/blob/8a60937cc0804b62a1df54b14f1114632a87ddb4/Cargo.toml#L20C1-L20C104

You can do the same thing by having your own local copy of makepad, which you can then modify. All you need to do is fork the Makepad project on github, then git clone it to your local machine, and then change that above line in the Cargo.toml file to point to your local version of makepad. Then, when you modify that version of makepad, it will automatically rebuild it when you build Robrix.

- makepad-widgets = { git = "https://github.com/kevinaboos/makepad", branch = "rik_with_old_font_stack" 
+ makepad-widgets = { path = "../makepad/widgets" }

(assuming your makepad directory is in the same directory as robrix)

Also, when you create a new branch, make sure you are branching off of my custom branch of makepad, which is on my remote (kevinaboos) with the branch name rik_with_old_font_stack: https://github.com/kevinaboos/makepad/tree/rik_with_old_font_stack

Thanks a lot! I'll follow this step by step. I tried basically doing this with I believe the main branch for Makepad and ran into issues with some things in the code not being defined after I changed the .toml file. I'll try this now though!

[kevinaboos]

Thanks a lot! I'll follow this step by step. I tried basically doing this with I believe the main branch for Makepad and ran into issues with some things in the code not being defined after I changed the .toml file. I'll try this now though!

Yeah, sorry about that. We're currently in the middle of a lot of uncoordinated changes to the Makepad project, but within the next month everything should be merged back together into the main Makepad repo, at which point it'll be a lot easier to track and modify Makepad components themselves.

[smarizvi110]

@kevinaboos I've tried two approaches with Makepad:

1. I attempted to consistently check for further items below but couldn't figure out an alternative logic that didn't rely on reaching the end or visibility in the viewport. I also couldn’t use the existing metrics in a consistent way.

2. I tried to figure out implementing the original behavior I proposed that accounts for all PortalList viewport display sizes (not the RoomScreen viewport size in the context of Robrix which contains the PortalList viewport), including when the window is resized or new messages come in. This approach involves checking if a certain portion of the list, in terms of pixels or a similar metric, is still below, accounting for long messages by considering the actual space they occupy on the screen, which aligns better with the desired UX.

I believe the second approach is more desirable and now feasible, since we’ve decided to work directly on Makepad to benefit both Robrix and other projects. I'd like to take this on as a project under your guidance. I’m learning a lot and see this as a valuable opportunity to contribute to some exciting projects.

[kevinaboos]

Thanks @smarizvi110. Maybe @jmbejar or @joulei can chime in here, as they've both worked on Makepad in various ways.

[kevinaboos]

@smarizvi110 So, it turns out the PortalList already supports this in Makepad, it just wasn't exposed. 😅 I added a new method to PortalList called is_at_end(), which returns true if the PortalList is scrolled all the way down.

If you merge in my latest changes from the main branch, which include using that newer version of my fork of Makepad, then you'll be able to use the is_at_end() function to determine whether the PortalList is at the bottom. Hopefully that'll work to quickly & easily resolve this.

[smarizvi110]

@smarizvi110 So, it turns out the PortalList already supports this in Makepad, it just wasn't exposed. 😅 I added a new method to PortalList called is_at_end(), which returns true if the PortalList is scrolled all the way down.

If you merge in my latest changes from the main branch, which include using that newer version of my fork of Makepad, then you'll be able to use the is_at_end() function to determine whether the PortalList is at the bottom. Hopefully that'll work to quickly & easily resolve this.

@kevinaboos Unfortunately it seems like that did not change much; the method essentially takes one of the booleans further_items_below() uses in its return and returns that. Logging showed that despite scrolling to the bottom, the return would not correctly tell me if I was at the end of the list even though I was. This certainly has to do with how at_end is updated upon scrolling...

[kevinaboos]

Huh, really? I tested it by logging the value of is_at_end() in the draw code (in Timeline::draw_walk()) and it seemed to behave exactly how we want.

What do you observe when using is_at_end()? Could you describe it in more detail and/or share a short video of what's happening?

[smarizvi110]

https://github.com/user-attachments/assets/62e4584c-bc25-469e-b6a0-78a9df108275

Here you are @kevinaboos! As you can see, even though I am indeed at the bottom of the list of messages, the function does not return that I am. Only when I scroll down a little more does the function correctly return that I am at the bottom. Here's an idea, could this be that because at_end is updated with regards to the viewport height in the scrolling logic, this is just due to some floating point errors?

[kevinaboos]

Thanks for the video! It's a bit hard to tell when at_end is not behaving as expected -- it seems that it's always doing what we want, even in your video. At what point (timestamp) in the video is it not behaving as expected?

Here's an idea, could this be that because at_end is updated with regards to the viewport height in the scrolling logic, this is just due to some floating point errors?

yep that could certainly be relevant here, but I'm not sure.

[smarizvi110]

So at 0:19, as you can see, I have reached the bottom of the list. But, the button is still visible and at_end returns false when it should be the opposite. But then, when I scroll down again as shown by the mouse input capture, I don't actually move the list at all (since I'm already at the bottom) but this time the return value is true and the button does disappear. What should have happened is that it should have should have returned true when I first scrolled to the bottom only at 0:19.

Note that this is on Windows.

[kevinaboos]

hmm ok, i think I see it, though it's still a bit hard to tell. Seems like a weird fluctuation perhaps related to how scrolling is implemented in the PortalList. It could also be a scroll-handling bug in Windows, or perhaps something to do with using the mouse wheel instead of a trackpad scroll or a finger drag action. I'll discuss it with the Makepad team in our meeting this week.

That being said, i think it's probably okay to merge in the current implementation and move on, leaving future improvements to be made in Makepad itself. There doesn't seem to be much we can do within Robrix if the PortalList is telling us that it's scrolled to the end when it actually isn't.

So, if you're okay with it, please go ahead and finish & clean up this PR with the visibility of the button being based on is_at_end(). It's still a major improvement even if it isn't perfect!

[smarizvi110]

hmm ok, i think I see it, though it's still a bit hard to tell. Seems like a weird fluctuation perhaps related to how scrolling is implemented in the PortalList. It could also be a scroll-handling bug in Windows, or perhaps something to do with using the mouse wheel instead of a trackpad scroll or a finger drag action. I'll discuss it with the Makepad team in our meeting this week.

That being said, i think it's probably okay to merge in the current implementation and move on, leaving future improvements to be made in Makepad itself. There doesn't seem to be much we can do within Robrix if the PortalList is telling us that it's scrolled to the end when it actually isn't.

So, if you're okay with it, please go ahead and finish & clean up this PR with the visibility of the button being based on is_at_end(). It's still a major improvement even if it isn't perfect!

Alright @kevinaboos! It's done for now! I think everything is cleaned up; obviously left the comments regarding the boolean logic. I'd love to hear about what the team says on this so solutions can form for Makepad itself and thus Robrix in the future! Thank you!

[kevinaboos]

alright, I'm happy to report that I've made progress debugging this issue after some additional investigation, w00t!

PortalList::is_at_end() is almost always correct, except for in one circumstance. When the user attempts to scroll past the bottom of the PortalList, it will perform a sort of "overshoot" calculation, which allows it to (optionally) perform an animation sequence where the list "bounces" up and then back down to indicate that the user has reached the end.

During this overshoot period, is_at_end() will return false, as the PortalList is technically "past" the end of its possible scroll bounds. Once the bounce/overshoot period has ended (which is very quick), is_at_end() will continue to return true as expected.

This is what's causing our "jump to bottom" button to flicker when the user scrolls down past the end of the list.