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

ZStack fix #2291

Merged
merged 4 commits into from
Nov 29, 2022
Merged

ZStack fix #2291

merged 4 commits into from
Nov 29, 2022

Conversation

xarvic
Copy link
Collaborator

@xarvic xarvic commented Nov 25, 2022

This PR makes it possible for a parent widget to use mouse events (for an example scrolling) while hovering over a ZStack.

TODOs:

  • decide whether Event::Wheel is a follow up event when the widget is active.

…handled is called

- add event.is_pointer_event()
@xarvic xarvic linked an issue Nov 25, 2022 that may be closed by this pull request
Copy link
Collaborator

@jneem jneem left a comment

Choose a reason for hiding this comment

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

Thanks! I guess figuring out the status of wheel isn't super critical, since mouse up/down are much more important.

This conversation has made me realize, though, that I don't really understand exactly when it's appropriate to set_handled...

ctx.set_handled();
if event.is_pointer_event() && previous_hot {
if layer.child.is_active() {
ctx.set_handled();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need this set_handled? Given your point about set_handled affecting the rest of the widget tree, it seems maybe better to avoid it?

Copy link
Collaborator Author

@xarvic xarvic Nov 28, 2022

Choose a reason for hiding this comment

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

Do you mean replace it with the obstructed flag or just remove it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a good chance I'm missing something, but I was thinking just remove it. Does that work?

I'm expecting that this child is active but not hot, and it will know how to correctly handle mouse events based on that, even if they aren't marked as handled.

Copy link
Collaborator Author

@xarvic xarvic Nov 28, 2022

Choose a reason for hiding this comment

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

I'm expecting that this child is active but not hot, and it will know how to correctly handle mouse events based on that, even if they aren't marked as handled.

The child only knows that it is not hot because it received a follow up event (Mouse event while active is set) which was marked as ´handled´.

In any case we either need set_handled or obstructed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't the child do ctx.is_hot()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is about the Pod not the widget. The Pod sees a mouse event inside its bounds. Therefore it sets its hot state to true. The only exception is that active and hot are both set. In that case the Pod sets hot to false but still propagates the event.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You mean handled and not active, right? So in that case I understand why you need set_handled here, thanks for the explanation! I still think it isn't ideal because (as you explained in some other thread) it affects the global widget tree and not just the subtree under the zstack. But I don't think figuring it out needs to block this pr...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ups my mistake. I meant active and set_handled. In this case hot is set to false even if the curser is inside the widgets bounds

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I still think it isn't ideal because (as you explained in some other thread) it affects the global widget tree and not just the subtree under the zstack. But I don't think figuring it out needs to block this pr...

I aggree. The only thing which is still brocken is scrolling while pressing a mousebutton. That is not the most impprtant usecase. I think we can merge this PR as it is.

@jneem jneem added the 0.8 label Nov 26, 2022
@xStrom xStrom added the S-waiting-on-author waits for changes from the submitter label Nov 27, 2022
@xarvic
Copy link
Collaborator Author

xarvic commented Nov 28, 2022

Thanks! I guess figuring out the status of wheel isn't super critical, since mouse up/down are much more important.

Event::Wheel is now a pointer event, which is probably the easiest solution.

@jneem jneem removed the S-waiting-on-author waits for changes from the submitter label Nov 29, 2022
@xarvic xarvic merged commit e3b77cd into linebender:master Nov 29, 2022
xarvic added a commit to xarvic/druid that referenced this pull request Jan 15, 2023
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.

Scroll bug when using ZStack
3 participants