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

don't change focus to on_click under other on_click #379

Merged
merged 1 commit into from
Mar 17, 2024

Conversation

jrmoulton
Copy link
Collaborator

@dzhou121

I've been trying to fix the issue where having an on_click handler on top of another on_click handler will move focus to the bottom one and it will receive the click event instead of the one on top.

This change fixes the issue but I'm not sure I completely understand it.

The problem comes because both views will get the pointer down and since the bottom one gets it last it gets the focus for the pointer up.

Review on this would be good

Fixes #295

@jrmoulton
Copy link
Collaborator Author

whoops. pushed to wrong branch

@@ -318,7 +321,9 @@ impl WindowHandle {
}
}
}
if was_focused != cx.app_state.focus {
if (was_focused != cx.app_state.focus) && !handled {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this is not needed because the PointerDown event propagation was stopped. Can you please test if it fixes the issue without this check?

Copy link
Collaborator Author

@jrmoulton jrmoulton Mar 15, 2024

Choose a reason for hiding this comment

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

This is the part I don't completely understand what is happening. If I remove that check then anytime that an event returns Stop (even a different event handler such as PointerDown) the focus will be switched away on pointer down

@jrmoulton
Copy link
Collaborator Author

Figured it out. There was another issue. If a view has focus and then there is a pointer down event inside of that view (on a child) that returns EventPropagation::Stop the focus will have been cleared without giving the focus to any other view.

This checks on pointer down if the event is still inside of the old was_focused and nothing else has gained focus it gives focus back to the view that previously had focus.

@dzhou121 dzhou121 merged commit 0bf0a29 into lapce:main Mar 17, 2024
7 checks passed
@jrmoulton jrmoulton deleted the two-on-click branch March 17, 2024 21:10
dzhou121 added a commit that referenced this pull request Apr 22, 2024
dzhou121 added a commit that referenced this pull request Apr 22, 2024
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.

Views on top of inputs don't fire on_clicks
2 participants