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.

Sign up for GitHub

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

Jump to bottom

Fix: scroll_with_delta() for ScrollArea::vertical() and ScrollArea::horizontal() #4893

Closed
wants to merge 4 commits into from
Closed

Fix: scroll_with_delta() for ScrollArea::vertical() and ScrollArea::horizontal() #4893

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e86fd8
Update scroll_area.rs
rustbasic Jul 31, 2024
e621130
Update scroll_area.rs
rustbasic Aug 1, 2024
506e07c
Update scroll_area.rs
rustbasic Aug 1, 2024
05d5fe6
Update scroll_area.rs
rustbasic Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions crates/egui/src/containers/scroll_area.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,22 +793,25 @@ impl Prepared {

let content_size = content_ui.min_size();

let scroll_delta = content_ui
.ctx()
.frame_state_mut(|state| std::mem::take(&mut state.scroll_delta));

for d in 0..2 {
// FrameState::scroll_delta is inverted from the way we apply the delta, so we need to negate it.
let mut delta = -scroll_delta.0[d];
let mut animation = scroll_delta.1;

// We always take both scroll targets regardless of which scroll axes are enabled. This
// is to avoid them leaking to other scroll areas.
let scroll_target = content_ui
.ctx()
.frame_state_mut(|state| state.scroll_target[d].take());

if scroll_enabled[d] {
let (scroll_delta_0, scroll_delta_1) = content_ui.ctx().frame_state_mut(|state| {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could also be solved by using less lines of code if wanted

let (mut delta, mut animation) = content_ui.ctx().frame_state_mut(|state| {
                    (
                        -std::mem::take(&mut state.scroll_delta.0[d]),
                        std::mem::take(&mut state.scroll_delta.1),
                    )
                });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using - here is bad for readability.

It's okay to do let (scroll_delta, scroll_animation), but since it's only used temporarily, it's not a big problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

you are right

(
std::mem::take(&mut state.scroll_delta.0[d]),
std::mem::take(&mut state.scroll_delta.1),
)
});

// FrameState::scroll_delta is inverted from the way we apply the delta, so we need to negate it.
let mut delta = -scroll_delta_0;
let mut animation = scroll_delta_1;

if let Some(target) = scroll_target {
let frame_state::ScrollTarget {
range,
Expand Down
Loading