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

On Windows, Add HitTest event (rebased) #3207

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

On Windows, Add HitTest event (rebased) #3207

wants to merge 4 commits into from

Conversation

Jasper-Bekkers
Copy link
Contributor

@Jasper-Bekkers Jasper-Bekkers commented Nov 1, 2023 •
edited

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

This is just a convenience PR for #2538 that I've rebased. Should fix #2413

@Jasper-Bekkers Jasper-Bekkers changed the title Rebased branch On Windows, Add HitTest event (rebased) Nov 1, 2023
@msiglreith
Copy link
Member

I'm generally in favor of such an API, I have been using sth similar in my own testbed as well for implementing custom decorations (https://github.com/msiglreith/nari/blob/main/nari-studio/src/main.rs#L415-L423).

On Windows, IIRC, some areas (caption mouse tracking, buttons commands) require extra work in the backend to work correctly tho - did it work for you out of the box? As you mentioned in the other PR that you are drawing custom decorations already

@Jasper-Bekkers
Copy link
Contributor Author

Jasper-Bekkers commented Nov 7, 2023 •
edited

On Windows, IIRC, some areas (caption mouse tracking, buttons commands) require extra work in the backend to work correctly tho - did it work for you out of the box? As you mentioned in the other PR that you are drawing custom decorations already

I've made sure the attached example works again, we haven't updated to 0.29 yet (it's a work in progress) so haven't been able to test in our app directly.

We're using egui to draw the UI, so I'd much rather see methods on the window object to achieve this myself (e.g window.hit_test(WindowArea::Caption, Rect { x, y, width, height }) which would require a bit more plumbing in the winit backend but it would fit much better for us due to the immediate mode nature of egui. (Not a hard requirement though, since we can always make this PR work as well). At the moment we'll do this in our wrapper api though.

@Jasper-Bekkers
Copy link
Contributor Author

@msiglreith is there a chance of getting this merged or should I close the PR again (since it's just a rebase of another PR to help along the merge process).

msiglreith
msiglreith approved these changes Nov 17, 2023
View reviewed changes
src/event.rs
/// Sent to a window in order to determine what part of the window corresponds to a particular screen coordinate.
/// This can happen, for example, when the cursor moves, when a mouse button is pressed or released.
///
/// - `x` and `y` are relatvie to the window top-left corner.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// - `x` and `y` are relatvie to the window top-left corner.
/// - `x` and `y` are relative to the window top-left corner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msiglreith msiglreith msiglreith approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HitTest event
2 participants
@Jasper-Bekkers @msiglreith