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

Expose methods to change viewport alignment #1953

Merged
merged 3 commits into from
Jul 13, 2023

Conversation

tarkah
Copy link
Member

@tarkah tarkah commented Jul 13, 2023

As discussed in #1912, consumers need to be able to calculate new viewport offsets when swapping between alignments.

This exposes two new with_<direction>_alignment methods. If user wants to keep the scrollable's viewport anchored in the same position while swapping alignments, they can call viewport.with_alignment(new_alignment) and return the new viewport's relative_offset to scroll_to.

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.

I have simply introduced an absolute_offset_reversed method instead. Will that work?

@hecrj hecrj force-pushed the feat/viewport-translation branch from d29de5e to a92e38a Compare July 13, 2023 00:56
@hecrj hecrj force-pushed the feat/viewport-translation branch from a92e38a to a9987cb Compare July 13, 2023 00:56
Copy link
Member Author

@tarkah tarkah left a comment

Choose a reason for hiding this comment

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

Looks good, @hecrj. Thanks!

@hecrj
Copy link
Member

hecrj commented Jul 13, 2023

Eventually, we should probably expose a variant of the Scrollable widget that lets the user completely manage the offset.

I believe `Offset::absolute` guarantees the offset never exceeds the
maximum scrolling boundaries already.
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.

Cool! Thanks again 🙇

@hecrj hecrj added feature New feature or request widget labels Jul 13, 2023
@hecrj hecrj added this to the 0.10.0 milestone Jul 13, 2023
@tarkah
Copy link
Member Author

tarkah commented Jul 13, 2023

Eventually, we should probably expose a variant of the Scrollable widget that lets the user completely manage the offset.

At what point should we prefer widget operations over managed app state?

The same can probably be said for text input widget.

As for composing widgets, a parent widget can still do widget operations within on_event right? It's not just the application that can do these.

@hecrj
Copy link
Member

hecrj commented Jul 13, 2023

At what point should we prefer widget operations over managed app state?

When commands are regularly needed to keep a widget in sync with some application state, then that's a sign there are multiple sources of truth.

With managed app state you can derive any scrolling state from your application state without necessarily storing offsets at all.

The same can probably be said for text input widget.

Yes, a text input with a fully managed cursor has a bunch of valid use cases.

As for composing widgets, a parent widget can still do widget operations within on_event right? It's not just the application that can do these.

Yes, although I'm not sure that'd be very useful. You'd want the unmanaged widget to depend on the managed one, and the managed one won't expose operations ideally.

@hecrj hecrj enabled auto-merge July 13, 2023 01:10
@hecrj hecrj merged commit fca8373 into iced-rs:master Jul 13, 2023
9 checks passed
@tarkah
Copy link
Member Author

tarkah commented Jul 13, 2023

Agreed! The need for managed state satisfies both use cases.

The text input seems like a good test case for this since it really can go either way.

Would you prefer two separate widgets, or one widget that exposes a managed builder for taking state provided by the application and internally it can switch between that or tree state?

@hecrj
Copy link
Member

hecrj commented Jul 13, 2023

Two separate widgets; one built on top of the other. Logic will be simpler this way, and we can expose specific APIs for each.

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

Successfully merging this pull request may close these issues.

2 participants