iced-rs / iced

A cross-platform GUI library for Rust, inspired by Elm
https://iced.rs
MIT License
23.06k stars 1.06k forks source link

Scrolling programmatically #307

Closed krooq closed 2 years ago

krooq commented 4 years ago

The scroll_to method on the State in Scrollable is useful for scrolling programmatically but it requires knowledge of the layout bounds which isn't public.

Either this information should be exposed, probably for all widgets, or the State should contain the bounds of the Scrollable so that scroll_to can be used as public api without needing to create a custom widget.

What do you think?

EDIT: zulip links https://iced.zulipchat.com/#narrow/stream/213316-general/topic/infinite.20list https://iced.zulipchat.com/#narrow/stream/213445-help/topic/Scrollable.3A.3Apush_back https://iced.zulipchat.com/#narrow/stream/213445-help/topic/how.20to.20scroll.20programmatically.3F https://iced.zulipchat.com/#narrow/stream/213445-help/topic/scroll_to

hecrj commented 4 years ago

We have not added support for scrolling programmatically in update logic yet. In other words, we have still not considered what a good design for that use case would be. Suggestions welcome!

These methods are exposed so they can be used from custom widgets after layouting. We should probably clarify that in the docs.

krooq commented 4 years ago

These methods are exposed so they can be used from custom widgets after layouting. We should probably clarify that in the docs.

Still a Rust noob so this wasn't clear to me only because of that :P

I think the canonical use cases for programmatic scrolling are

  1. bring some content into view
  2. scroll by some amount
  3. scroll to some point I'd define "into view" as scrolling in some direction until some reference point is within the view-able bounds of the Scrollable. The reference point may depend on the scroll direction, e.g. maybe its the far side of a widget.

I'll continue thinking about this and how it could possibly fit in. :)

krooq commented 4 years ago

I think for this, perhaps we can just replace offset with the percentage factor and leave the bounds calculation to the draw function. It doesn't make much sense to have offset in the state which is derived from the bounds and then call pub fn offset(&self, bounds: Rectangle, content_bounds: Rectangle) -> u32 to recompute the offset in the draw function anyway. We can also do the calculation for pub fn scroll(&mut self,delta_y: f32, bounds: Rectangle, content_bounds: Rectangle) in the on_event method.

hecrj commented 4 years ago

I think for this, perhaps we can just replace offset with the percentage factor and leave the bounds calculation to the draw function

As I said in #323, this will cause the scrolling offset to change when the layout of the contents change.

It doesn't make much sense to have offset in the state which is derived from the bounds and then call pub fn offset(&self, bounds: Rectangle, content_bounds: Rectangle) -> u32 to recompute the offset in the draw function anyway.

This is necessary because the offset can be completely out of bounds if the scrollable contents change. The API is meant to be "fault-tolerant".

krooq commented 4 years ago

I'll continue the conversation from #323 here so its better tracked :)

I think for this, perhaps we can just replace offset with the percentage factor and leave the bounds calculation to the draw function

(#323) Using a relative value to store scroll position won't work well with dynamic content or layout. The scrolling offset will jump around instead of staying at a fixed position from the top.

I think using the offset will still have this issue if the user adds content above the current offset.

Yes, but I believe this is the behavior users will expect by default.

Keeping a bounds independent variable in the state is still desirable to allow the developer to make manual decisions. I can't think of many distinct use cases of dynamic content in a scrollable that would trigger this condition. Some typical use cases I can think of are:

In all of these cases (except text input) you wouldn't want the viewable area to move at all as this would be jarring to the user. For text input you would want to scroll to bottom. I cant think of a case where one would want the default functionality you describe or what I proposed.

I don't think we can consistently handle all the cases at once. We need the user to tell us what kind of behavior they want.

I agree. I think we should leave this decision up to the user to decide what to do in the update logic. Each use case will be different.

If we must pick a default I propose adding to_bottom and to_top as options for a Scrollable::scroll_on_change builder option.

I would rather do less and enable the developer to make their own decisions. Business logic shouldn't be in a framework and I think were getting very close to business logic.

krooq commented 4 years ago

In #323, you mentioned the concept of an Anchor. I think this could be the ideal solution. So the state would then have an offset: u32 and anchor: Anchor. Anchor could be either Start or End and it specifies the end of the Scrollable that is fixed so any content changes would result in the Scrollable growing in the opposite direction. This is the same as it currently is, it just allows the content to expand in the opposite direction if desired.

Should we also add a field to the state to remember if manual scrolling has occurred so that developers can decide how if they want to apply their manual scrolling logic or not. There already is a scroller_grabbed_at field but it is transient, we really want something the developer has to manually reset.

In terms of actual "Programmatic Scrolling" I'm still not sure what we can do. I think the to_bottom and to_top functions I mention above would still be really useful and its a hint as to the right direction. For anything else we really need to know the bounds to do some computation in update and I think ultimately that the big issue here. I don't think it's right to put the bounds in the state of the scrollable.
Arguably we could put only the height in there. To do any really custom stuff the dev either needs to make a custom scrollable widget OR we need to give the dev access to the bounds in update and let them do it there. The other thing is that there is "no" "callback" in Widget to process changes from update so the custom widget route is still problematic.

hecrj commented 4 years ago

I think we can start by removing the boundary checks in State::scroll. These aren't really necessary, as the only way to read the offset is through State::offset which should already clamp the value correctly.

If we then add the concept of an anchor, we should cover a bunch of use cases already. We need to decide how will the user define/change this anchor and the scrolling behavior.

krooq commented 4 years ago

Summarizing ideas thus far

Yes lets do these in the solution:

  1. Remove boundary checks in State::scroll
  2. Add concept of Anchor to Scrollable

More discussion:

hecrj commented 4 years ago

I think we can go ahead and implement (1).

For (2), we still need to answer some questions:

A good alternative is making offset an enum. We encode the different use cases there, and defer actual computation to Scrollable::offset.

Expose bounds to update function Expose bounds in some kind of after_update function in the widget

Both of these are deeply involved design changes that break current API guarantees, add complexity, and remove boundaries.

krooq commented 4 years ago

Anchor gives us bidirectionality in the scrollable and that's about it. I don't think the concept is actually coupled to the offset, merely a choice about which direction the hidden content grows in. It makes sense to keep the default at Start and let the dev choose when constructing the view maybe just anchor(Anchor::End) builder option. An offset enum could be good but I'm not sure that we want to cover so many cases in the default widget, I feel it should be more light-weight and provide typical functionality out of the box, nothing complicated. This covers things like text editors that expand downwards and chatboxes that expand upwards. I don't think it even makes sense to expand a scrollable from the middle or any other direction... unless we get into left/right support but I think that's a different ticket and in which case we would just add another Anchor for that direction, same pattern as height/width.

  • Expose bounds to update function
  • Expose bounds in some kind of after_update function in the widget

    Both of these are deeply involved design changes that break current API guarantees, add complexity, and remove boundaries.

Yup I agree, its a weighty topic. I think after_update is not what I wanted, more after_layout. But even so, I'm not sure that this would be the right way to do it. Also, even if you have the bounds exposed in update there is more to the story. When you recreate the view the bounds may change, so any decisions you make in update may not be valid when the layoutis complete. I have this exact issue in my proto (basically a terminal/chatbox). I am scrolling to the bottom when the user submits a new message, but because I'm working on the view of n-1 messages the bounds are off when layout is created and I cant see the new message. Really in this case I want to push a new event into the loop to do the scroll_to_bottom after layout has been able to process my updated content. I am quite afraid of this pattern. In JavaFX there is a function called Platform.runLater(Runnable) that essentially does this and it is a source of many many bugs in my daily work. The solution we use is to attach a callback to the needsLayout property that runs whenever it changes and then request a layout pass, but this can be computationally prohibitive for things like tables since it invalidates the current layout and it'd probably be a nightmare in Rust. The problem with Platform.runLater is that it pushes tasks onto the end of the queue. If we could jump the queue with the next event (scroll_to_bottom event for instance) then this could be a nice solution. Because the layout is hashed we would only be reevaluating the parts of the tree that need re-layouting (I hope this is how it works) then we just call update in a regular fashion with updated layout. Of course we still need some way to provide the bounds to update. We could use the Scrollable State and update it in draw as it has mutable access but that seems like a total hack. This could also get tricky since now we have lifecycle between events in update. On a larger project this would easily become a nightmare.

How does the elm world do scrollable widgets?

krooq commented 4 years ago

I'm actually sorta leaning more towards after_layout or I guess on_layout to fit the other callback functions. It can just be a pub fn that takes a closure taking bounds and returning a Message like the on_change passed to TextInput::new for example. We can restrict it to only Scrollable and other widgets that may need it so it's not a breaking change if it's not desired. We can handle the logic in update like anything else with the knowledge that the layout has been performed for the requested widget.

krooq commented 4 years ago

331

krooq commented 4 years ago

I've found the following user story helpful.

Normally it works like this

  1. I receive an event
  2. Logic in update (or view) mutates the state
  3. The view is rebuilt using the new state
  4. If the view changed the widget performs a layout computation
  5. The new view is drawn according to the layout

But we hit a subtle problem when the user wants to perform some logic that depends on the layout like programmatic scrolling i.e.

Problem If the state contains information about the current layout then that information is invalid as soon as the state is mutated.

So as a user, how can I correctly implement logic that depends on the layout?

I have thought of a few high level ways this might be achieved:

  1. Create a command/message at or downstream of update e.g. in the view to trigger another event cycle. This does mean we will draw one "frame" with a view that we don't really want and has other challenges.
  2. Add an after_layout function to the Application that takes in the newly rebuilt layout allowing last minute state changes before draw is called. Caveat of this is that changes made here should not be allowed to alter the layout but can through altering the state.
  3. Do [2.] but only inside the Widget so that the state mutations can be controlled. This also impacts the API of the widgets as users would be calling functions in update/view to tell the widget to do something in after_layout, it might be icky.
  4. Give update access to the layout function so it can recalculate when/if required. This is how some other UI frameworks do things, not sure if it's even possible.

Other general questions on the principles of the framework that I'm unsure on:

  1. Should update know about the current layout?
  2. Should view know about the current layout?
  3. Should state be mutable after view but before draw?
hecrj commented 4 years ago

I think we are over-abstracting things here. Let's keep this focused to the programmatic scrolling use cases.

I am not convinced we need to expose layout to the user (and break the Elm Architecture) or introduce functions with temporal constraints (any kind of after_* function is normally a code smell) to satisfy most of the programmatic scrolling use cases.

As I said in my previous comment, if we encode offset as some kind of enum with an Anchor, we could leverage interior mutability to defer the absolute offset computation until the next time layout bounds are provided (i.e. next time State::offset is called). Is there any reason why we discarded this approach?

krooq commented 4 years ago

As I said in my previous comment, if we encode offset as some kind of enum with an Anchor, we could leverage interior mutability to defer the absolute offset computation until the next time layout bounds are provided (i.e. next time State::offset is called). Is there any reason why we discarded this approach?

Nothing has been discarded, I am taking the enum approach with the Anchor. I'm not a Rust master by any means and interior mutability is something new to me. Maybe this was mentioned in Zulip and I have forgotten or it went over my head 😨

For my understanding, is it correct that in this approach with the interior mutability we mutate the offset when State::offset is called?

If so, that is a neater solution although maybe less pure. I agree this is a good way to go if we are only mutating "internal" widget state. Could you provide a small code example of the Anchor enum showing what would be exposed to the user?

Do you think the offset should be inside the Anchor enum or separate? At the moment I have them separate, offset is always stored relative to the top but State::scroll and State::offset work according to the current Anchor variant. I'm just not sure what the best approach is for handling change of Anchor variant. I do think having them together would probably be best, this will make the horizontal case easier to implement in the future.

hecrj commented 4 years ago

@krooq Sorry! I did not mention interior mutability because I'd prefer to focus on the actual design before focusing on the implementation details (the internals).

For the Anchor path, we still need to state what use cases are we exactly tackling with it and how will the public API change to accomodate this. Once we settle on this, then we can start discussing the implementation details (maybe interior mutability isn't necessary after all!).

More or less in order, I believe we need to answer these questions:

  1. Which use cases do we want to satisfy? Which ones are we intentionally leaving out?
  2. How will the public API change to satisfy them?
  3. Will there be any specific challenges in the implementation?
  4. Are there any good design alternatives?

For (1), we mentioned already:

  • chatbox/terminal
  • sorted list
  • leaderboard
  • text input

Can we elaborate on the needs of these? For instance, a chatbox needs the concept of "snapping":

Let's try to design the most ideal public API first, without compromises.

hecrj commented 4 years ago

Let's say we add the following public methods to scrollable::State:

Which use cases would be satisfied by this design? Which ones would not?

krooq commented 4 years ago

Let's try to design the most ideal public API first, without compromises.

Ok but I think you may not see my point of view. These are my use cases, I think it covers most users.

  1. As a user, I want to alter the viewport of a Scrollable when an event occurs.
  2. As a user, I want to alter the viewport of a Scrollable when the content changes.

I'm not really thinking in terms of API. I just want to be able to do these things and I don't care how. I can iterate on these ideas myself, until I get something decent to present.

What I cant do is make decisions about fundamental aspects of the framework. This is where I need your guidance as I don't have your overarching vision, I just have a user perspective.

Let's say we add the following public methods to scrollable::State:

These cover most of the cases, whats missing:

krooq commented 4 years ago

Although we still have the issue I describe above in all of these. It's not dependent on the API choice. When the content changes we need some way to delay computation of the scrolling until after the layout has completed.

This issue doesn't just apply to scrolling, anything that changes content in update and needs to perform some layout adjustment on the new layout has this same issue. What should the API for that look like?

hecrj commented 4 years ago

These are my use cases, I think it covers most users.

  1. As a user, I want to alter the viewport of a Scrollable when an event occurs.
  2. As a user, I want to alter the viewport of a Scrollable when the content changes.

I understand, but I believe these are not really use cases but a way to satisfy many of them (XY problem). I think we should be focusing on actual problems, for instance:

I don't believe these use cases are inherently tied with a viewport, layout, and content changes. These seem like implementation details to me. Does the user really need to care about them?

These cover most of the cases, whats missing:

What are these cases? We need examples! How and when will these methods be used? What are we solving? I am asking you, the user! If we don't have clear examples for a particular one, then let's simply not implement it.

When the content changes we need some way to delay computation of the scrolling until after the layout has completed.

Yes, I believe we agreed this is possible without any further abstractions.

This issue doesn't just apply to scrolling, anything that changes content in update and needs to perform some layout adjustment on the new layout has this same issue. What should the API for that look like?

For now, there will be no generic API for that. We will specifically build APIs for each of the particular set of problems we identify, unifying as necessary.

krooq commented 4 years ago

Ok, lets not plan for a future that may never exist. For starters we should do just what I require, if someone else has a use case they can chime in and we will add it to the list. Going off your previous post.

  1. Which use cases do we want to satisfy?

My application use case is a chatbox/terminal (simple chatbox).

  1. .... Which ones are we intentionally leaving out?
  1. How will the public API change to satisfy them?

I don't really know whats best here and I don't really mind.

I really dunno with these until I try them and see what feels good.

  1. Will there be any specific challenges in the implementation?
  1. Are there any good design alternatives?

FYI, I was able to get this working with the interior mutability and delayed computation in layout without much ugliness at all, so that could definitely be a good route to take when the API is decided on. I tried it in draw and noticed a lag between the content changing (expanding) then the scroll to bottom, do you know why that would be?

krooq commented 4 years ago

Been trying out the API..

Another item in the view for enable_snap: bool could work well for optionally enabling/disabling the snapping behaviour?

Not sure about this one, its heavily dependent on the user application. A terminal would want it, a chatbox might want it, an async loaded list would not want it. I'm not sure if its actually much differnt to the Anchor, without this the Anchor really only makes sense when the content is smaller than the bounds of the Scrollable.

Another item in the view snap_on_change: Fn(content) -> bool to filter content changes and do the snapping if there is some change?

I'm not really sold on the closure. Might be better to just call a once off on the Scrollable in update something like scroll_to_(bottom|top|content|anchor) as previously mentioned.

hecrj commented 4 years ago

I believe most of your use cases can be satisfied with a simple snap_to_bottom method in scrollable::State that can be called from Application::update:

When the content of the chatbox changes and snap-to-bottom is enabled, I want to scroll to the bottom

The content can only change through an Application::update (i.e. a new message was received). You'd need to simply call snap_to_bottom accordingly.

I want to be able to disable snap-to-bottom when I manually scroll

This should be default behavior, after calling snap_to_bottom the widget should unsnap automatically on a "scroll up" event.

I want to be able to enable snap-to-bottom when I scroll to the bottom

For this, we can consider adding an on_scroll method to Scrollable. This method would be called with a message constructor and used to handle scroll events in Application::update.

I want to be able to enable snap-to-bottom when I submit text in the chatbox text input.

Call snap_to_bottom in Application::update.

I want to be able to toggle the behaviour of snap-to-bottom when I toggle a checkbox

Call snap_to_bottom in Application::update.

When the content of the chatbox changes and snap-to-bottom is disabled, I do not want the viewable portion of the chatbox to change.

This should work by default. If snap_to_bottom is not enabled, then the distance from top will be used as offset. Any new messages pushed at the end will not change the scrolling position.

I want to be able to reverse the direction of the chatbox so that all the above behaviours work in reverse i.e. snap-to-top instead of snap-to-bottom

This is not really a use case, but the default behavior is probably what you want here.

I want the above behaviours to be supported for any kind of content/widget

The above behaviors only make sense for a Scrollable.

krooq commented 4 years ago

Yep the snap_to_bottom and on_scroll solution sounds like it'll solve all of what I need.

I want to be able to disable snap-to-bottom when I manually scroll

This should be default behavior, after calling snap_to_bottom the widget should unsnap automatically on a "scroll up" event.

Yep ok, if I want to jump to new content when it appears even after a "scroll up" event I can just use the on_scroll function to handle that. Even though it may be an odd case I think that the user may want to disable snapping without scrolling at all, we should allow this.

I want to be able to reverse the direction of the chatbox so that all the above behaviours work in reverse i.e. snap-to-top instead of snap-to-bottom

This is not really a use case, but the default behavior is probably what you want here.

Yeah last 2 are more like requirements than use cases. Here I just mean that I may want the the chatbox to work in the opposite direction, some consoles do this where the input is at the top and text flows downwards. Simply meaning we should also have a sort of scroll_to_top method as well and some way to disables any snapping the same as a "scroll up" event would to bring us back to the default.

I want the above behaviours to be supported for any kind of content/widget

The above behaviors only make sense for a Scrollable

This just meant that I want to be able to use any widget as the content inside a Scrollable and still have the snapping functionality. This will be possible with the on_scroll method you mention since the user is in control of when to snap or not.

The State for the Scrollable currently has scroll_to and scroll which take some bounds. Do you want to keep these ideas and add access on the widget? or create new methods? Something like this maybe?

EDIT3 (clarity) scrollable::State

Scrollable

Keeping in mind we should also consider what the horizontal case would look like. It might be relevant for left-to-right vs right-to-left text.

EDIT1: Do you think we should also supply the previous scroll value or delta to the on_scroll message constructor? EDIT2: Should the on_scroll method only be triggered when the Scrollable has actually scrolled or on any scroll event? i.e. does a downwards scroll at the bottom of a Scrollable trigger the on_scroll?

yusdacra commented 3 years ago

I faced this issue while I was trying to implement some features for my Matrix chat client. I created a branch in my fork of iced, which solves the issues I had.

This doesn't really implement things said in this thread (and the code looks ugly) but it works for my case. Just wanted to share it if someone wants to take a look.