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 scroll_to operation #1796

Merged
merged 3 commits into from
Apr 20, 2023
Merged

Conversation

tarkah
Copy link
Member

@tarkah tarkah commented Apr 14, 2023

Supersedes #1795

Exposes a new scroll_to operation to allow scrolling a scrollable to a desired "absolute" offset.

@hecrj hecrj added this to the 0.10.0 milestone Apr 17, 2023
@hecrj hecrj added feature New feature or request widget labels Apr 17, 2023
Comment on lines 69 to 74
pub struct CurrentOffset {
/// The [`AbsoluteOffset`] of a [`Scrollable`]
pub absolute: AbsoluteOffset,
/// The [`RelativeOffset`] of a [`Scrollable`]
pub relative: RelativeOffset,
}
Copy link
Member

Choose a reason for hiding this comment

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

There's some redundancy here.

I think it'd be better to have a Window or Viewport type with the already existing private Offset, the content bounds, and the scrollable bounds.

Then we can expose relative_offset and absolute_offset methods in this type that compute the actual values from a single source of truth.

Copy link
Member Author

@tarkah tarkah Apr 17, 2023

Choose a reason for hiding this comment

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

Great suggestion 6ad5e03

@tarkah tarkah requested a review from hecrj April 17, 2023 20:56
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 have made a change to compare the absolute offsets as well in notify_on_scroll, since it's theoretically possible that the relative offset stays the same but the absolute offset doesn't (e.g. if the bounds of the Scrollable change while scrolling). See 8a71140.

I think we can merge! 🚢

@hecrj hecrj enabled auto-merge April 20, 2023 13:53
@hecrj hecrj merged commit 4052ccf into iced-rs:master Apr 20, 2023
@tarkah tarkah deleted the feat/scrollable-scroll-to branch April 20, 2023 14:36
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