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

Add scrollable alignment option #1912

Merged
merged 8 commits into from
Jul 12, 2023
Merged

Conversation

tarkah
Copy link
Member

@tarkah tarkah commented Jun 13, 2023

This PR adds a new Alignment option to both directions of the scrollable.

It's sometimes preferred to have the content aligned to the end of the scrollable by default, where the offsets are based on the end of the scrollable. New content getting added will cause the scrollable to expand up instead of down. Alignment::End enables this behavior.

@tarkah tarkah force-pushed the feat/scrollable-alignment branch 2 times, most recently from 3e0b189 to cdd2a39 Compare June 13, 2023 14:26
@tarkah tarkah force-pushed the feat/scrollable-alignment branch 3 times, most recently from e1445be to bd4cad9 Compare June 29, 2023 03:36
@tarkah tarkah force-pushed the feat/scrollable-alignment branch from bd4cad9 to d79cedd Compare July 4, 2023 17:43
@hecrj hecrj added this to the 0.10.0 milestone Jul 6, 2023
@hecrj hecrj added feature New feature or request widget layout labels Jul 6, 2023
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! 🎉 I imagine this will be very useful for chat clients; especially when loading message history.

I have made some changes here and there to make things a bit more type-safe and disambiguate offset from translation. I have also made the fields in Viewport private again, since I'm not sure which use cases warrant exposing those. Hopefully we can tackle them directly!

Let's merge this! 🥳

@hecrj hecrj enabled auto-merge July 12, 2023 08:23
@hecrj hecrj merged commit 21bd514 into iced-rs:master Jul 12, 2023
@tarkah
Copy link
Member Author

tarkah commented Jul 12, 2023

I have also made the fields in Viewport private again, since I'm not sure which use cases warrant exposing those. Hopefully we can tackle them directly!

@hecrj I was using the bounds & content_bounds fields for calculating the new absolute offset needed to keep the scrollable anchored in the correct location when toggling between start / end alignments.

This is needed in a chat application, consider the following. We use end alignment by default so new messages show up. When scrolling up from the bottom, we convert to start alignment so new messages don't change the position of the viewport. When swapping alignments, we need to issue scroll_to w/ an absolute offset to achieve this effect. Those bounds are necessary for this computation.

See https://github.com/squidowl/halloy/blob/caa024de2962d9646d62c34bcf565b1c7e9056b8/src/buffer/scroll_view.rs#L298-L305 for how we do this.

So I guess a direct use-case solution might be to expose a swap_<direction>_alignment type method on Viewport which will return a new AbsoluteOffset which can be returned in scroll_to to keep the viewport stationary when that new alignment is used on the scrollable.

@hecrj
Copy link
Member

hecrj commented Jul 12, 2023

@tarkah I think we can expose a translation method that returns a Vector in Viewport to satisfy that use case. Would that work?

@tarkah tarkah deleted the feat/scrollable-alignment branch July 12, 2023 23:16
@tarkah
Copy link
Member Author

tarkah commented Jul 12, 2023

@tarkah I think we can expose a translation method that returns a Vector in Viewport to satisfy that use case. Would that work?

I don't believe this works because translation is "normalized" but scroll_to / AbsoluteOffset is relative to the Alignment.

Since Offset doesn't encode which Alignment it's for, we would need to store Alignment in Viewport. Then maybe the correct API is to just have with_<direction>_alignment methods that can return a new Viewport transformed for that Alignment and then caller can get it's absolute_offset for use w/ scroll_to.

@hecrj
Copy link
Member

hecrj commented Jul 13, 2023

@tarkah In the snippet you shared, it seems the only thing we'd need to expose is (viewport.content_bounds.height - viewport.bounds.height).max(0.0). This is the vertical translation.

It seems you are computing the alignment from the Status itself, which makes sense since the alignment is driven by your application logic and not the scrollable itself. Adding alignments to Viewport seems a bit redundant and unnecessary.

Am I missing something?

@tarkah
Copy link
Member Author

tarkah commented Jul 13, 2023

it seems the only thing we'd need to expose is (viewport.content_bounds.height - viewport.bounds.height).max(0.0). This is the vertical translation.

Having that exposed is all I need. However, this isn't the scroll translation, it's the max scroll height.

When flipping between alignments, I take max scroll height minus current absolute offset to match the change in alignment.

@hecrj
Copy link
Member

hecrj commented Jul 13, 2023

This is the vertical translation.

It's not, my bad! I think a simple absolute_offset_reversed method would work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request layout widget
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants