Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

Fix some small stuff #40

Merged
merged 6 commits into from
Jun 21, 2022
Merged

Fix some small stuff #40

merged 6 commits into from
Jun 21, 2022

Conversation

fkaa
Copy link
Contributor

@fkaa fkaa commented Jun 6, 2022

Fixes panning outside of the editor area, and being able to open a new finder by clicking inside the finder window. Also makes it so the finder area is always on top. It seemed to disappear when clicking twice in a editor area for some reason.

Tested in my program and seems to work.

@setzer22
Copy link
Owner

setzer22 commented Jun 7, 2022

Thanks for this! 🎉 It had been bugging me for a while but I didn't have time to fix it.

The code looks good, but there's a few issues caught by CI.

Other than a few clippy lints, you're using a button_released function that seems to be unreleased. Are you on a patched version of egui? Maybe we can use any_released for the time being, and leave a TODO explaining that this should be using button_released on the next egui release. (EDIT: That won't work, we need to detect a mouse release on the secondary button).

Unfortunately, there doesn't seem to be much we can do to fix this before the next egui release. All the functions or fields we could read from to implement this on our end are private or pub(crate). Should we keep using button_down for the time being?

@fkaa
Copy link
Contributor Author

fkaa commented Jun 10, 2022

Ah sorry, yes I'm using the main branch of egui locally. I'm fine with waiting for the next release.

@setzer22
Copy link
Owner

It doesn't look like next egui version will release anytime soon, so for the time being I'll merge this using secondary_down instead of button_released. It's not exactly correct but all your other changes still work :)

The code in the original PR was correct, but button_released is not
available on egui 0.18 from crates.io, this commit moves back to the old
behavior of using secondary_down instead.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants