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

🪂 Disable drag_to_scroll on frame selection ScrollArea #214

Merged
merged 2 commits into from
Jun 28, 2024

Conversation

tosti007
Copy link
Contributor

@tosti007 tosti007 commented May 23, 2024

Redo of #207

Checklist

  • I have read the Contributor Guide
  • I have read and agree to the Code of Conduct
  • I have added a description of my changes and why I'd like them included in the section below

Description of Changes

In the version 0.27 release of egui, the interaction handling was changed. This caused the scope selection in the flamegraph to be unselectable. By disabling the drag_to_scroll on the related ScrollArea this issue is solved.

The latest scope selection does not suffer from this, as it does not use a ScrollArea.

Note that this does not have any behavioral effect, but instead results in the same behavior of click/drag/scroll when on egui 0.26!

Related Issues

Tested on

Edit tasklist title
Beta Give feedback Tasklist Tested on, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Windows
    Options
  2. Linux
    Options
  3. Mac
    Options
  4. Android
    Options
Loading

Copy link
Contributor

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

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

You can check "Tested on Linux" (via cargo r -p puffin_egui --example eframe). You're right that dragging only scrolls as soon as a scrollbar is necessary, never allowing to select frames anymore.

@Hoodad
Copy link
Contributor

Hoodad commented Jun 25, 2024

FYI I have found the source of the issue but I'm not well versed with egui yet to understand how to fix the issue, see #219. Not sure if we want this in if we can get the root issue resolved?

@MarijnS95
Copy link
Contributor

@Hoodad the root cause is already highlighted in this PR description, I think. The solution is given here as well, all that is required is for this PR to be merged 😉

@Hoodad
Copy link
Contributor

Hoodad commented Jun 25, 2024

Hmm fair enough I didn't consider disabling the drag_to_scroll to solve the problem, but maybe it should be considered to solve it? Have you tested the change on Android as that's where I would expect this change to degrade the experience the most as there wouldn't be any way of scrolling through the frames if I understood the change correctly?

@Hoodad
Copy link
Contributor

Hoodad commented Jun 26, 2024

Okay I just now tested the change on Android as well and it works (the UI is a bit too cramped to use on a small screen) like on desktop you have to scroll by grabbing the scrollbar which I consider good enough.

I wanted to get to the bottom with this and investigate an alternative solution that I got working that changes the logic inside show_frame_list to work without having to set drag_to_scroll to false. But after some testing I cannot see any drawback with this PR and its less code change then compared to my alternative method. So thumbs up from me👍

Copy link
Contributor

@rib rib left a comment

Choose a reason for hiding this comment

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

and testing on Windows this fix seems to work nicely here

@Hoodad
Copy link
Contributor

Hoodad commented Jun 27, 2024

@tosti007 can you or @MarijnS95 fix the merge conflict so we can have the change merged?

@Hoodad Hoodad merged commit 88c8d16 into EmbarkStudios:main Jun 28, 2024
6 checks passed
@tosti007 tosti007 deleted the scroll_no_drag branch July 1, 2024 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🍳 puffin-egui: Incorrect flamegraph frame selection input handling
4 participants