-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 manual scrolling #81
Conversation
Now the center ratio is used inside the ScrollArea
Added the |
Update the scroll demo (made it similar to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Thanks for taking your time to work on this.
I left a lot of comments. Most contentious will probably be center_factor
vs Align
. This is basically a tradeoff between flexibility and simplicity, and I'm keen to hear your thoughts on it!
egui/src/ui.rs
Outdated
/// The scroll centering is based on the `center_factor`: | ||
/// * 0.0 - top | ||
/// * 0.5 - middle | ||
/// * 1.0 - bottom |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// * 1.0 - bottom | |
/// * 1.0 - bottom | |
/// | |
/// ``` | |
/// # let mut ui = egui::Ui::__test(); | |
/// egui::ScrollArea::auto_sized().show(ui, |ui| { | |
/// for i in 0..1000 { | |
/// if ui.button(format!("Button {}", i)).clicked { | |
/// ui.scroll_to_here(0.5); | |
/// } | |
/// } | |
/// }); | |
/// ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...though in this case, this would make more sense:
let response = ui.button(format!("Button {}", i));
if response.clicked {
response.scroll_to_me(0.5);
}
Is there any other more plausible use case of this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the code example from the scroll_to_cursor
to use the scroll_bottom
example similar from the demo. Makes more sense for this case. Added the button example for the scroll_to_me
.
egui/src/ui.rs
Outdated
/// * 0.5 - middle | ||
/// * 1.0 - bottom | ||
pub fn scroll_to_here(&mut self, center_factor: f32) { | ||
let scroll_target = self.min_rect().bottom(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will scroll to the bottom of the current size of the Ui
, not to the current cursor position (a Ui
can be resized to something large before adding elements to it).
I think scroll_to_cursor
and let scroll_target = self.region.cursor.y;
is better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point 👍 Didn't knew about that.
Another minor addition: when clicking a button to scroll to the top of that button, I think it would make sense to add a small offset (e.g. |
egui/src/containers/scroll_area.rs
Outdated
let height_offset = content_ui.clip_rect().height() * center_ratio; | ||
let top = content_ui.min_rect().top(); | ||
let offset_y = scroll_target - top - height_offset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let height_offset = content_ui.clip_rect().height() * center_ratio; | |
let top = content_ui.min_rect().top(); | |
let offset_y = scroll_target - top - height_offset; | |
let offset_y = scroll_target - lerp(content_ui.clip_rect().y_range(), center_ratio); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not work, as the clip_rect
is always the same, but I updated the code to be more easy to understand. I hope.
pub(crate) fn scroll_center_factor(&self) -> f32 { | ||
match self { | ||
Self::Min => 0.0, | ||
Self::Center => 0.5, | ||
Self::Max => 1.0, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is the best place to add this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the right place! Maybe could be called simply to_factor
/// ui.scroll_to_cursor(Align::bottom()); | ||
/// } | ||
/// }); | ||
/// ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good example 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome - this works great!
Thank you!
Fixes #74