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

core: Add option to focus a view on mouse click even if modifiers are pressed #896

Merged
merged 3 commits into from
Nov 25, 2020

Conversation

dkondor
Copy link
Contributor

@dkondor dkondor commented Nov 23, 2020

This addresses the second part of my issue #891 by allowing a view to be focused by a modifier + mouse click. In my opinion, this behavior is more consistent. Also, this ensures that the clicked view receives the modifier state (since it is always focused).

I've implemented this as an option, with the old behavior being the default.

Depends on WayfireWM/wf-config#36

Currently, click-to-focus only works for mouse clicks without keyboard modifiers. If a modifier is held down, the necessary button binding in the focus handler does not match, so the focus does not change. The view under the mouse receives the mouse click, but not the state of the keyboard modifiers (since it never receives keyboard focus).

This commit adds an option to essentially ignore pressed modifiers for the purpose of focusing the clicked view, so this will work consistently regardless of the state of keyboard modifiers.

Fixes #657

@ammen99
Copy link
Member

ammen99 commented Nov 23, 2020

I would much rather implement it in a different way. Instead of relying on button bindings, just listen for direct button press signals. This would also solve #657

@dkondor
Copy link
Contributor Author

dkondor commented Nov 23, 2020

I would much rather implement it in a different way. Instead of relying on button bindings, just listen for direct button press signals. This would also solve #657

I was thinking of this (but this needs more changes); I'll look into this.

@dkondor dkondor force-pushed the focus_click_modifier branch 2 times, most recently from 69d82d8 to 95483cc Compare November 23, 2020 17:57
@dkondor
Copy link
Contributor Author

dkondor commented Nov 23, 2020

This is actually simpler than I expected. What I actually don't like with this change is that it splits the focusing implementation between core/wm.cpp and core/seat/pointer.cpp -- I wonder if it would be better to keep the main functionality in wm.cpp and call it from pointer.cpp

@ammen99
Copy link
Member

ammen99 commented Nov 23, 2020

This is actually simpler than I expected. What I actually don't like with this change is that it splits the focusing implementation between core/wm.cpp and core/seat/pointer.cpp -- I wonder if it would be better to keep the main functionality in wm.cpp and call it from pointer.cpp

You don't have to do that - there are events that the focus plugin can listen to.

template<class wlr_event_t>

@dkondor dkondor force-pushed the focus_click_modifier branch from 95483cc to 82d97e0 Compare November 24, 2020 02:30
@dkondor dkondor force-pushed the focus_click_modifier branch from 82d97e0 to 8f6cc10 Compare November 24, 2020 02:31
@dkondor
Copy link
Contributor Author

dkondor commented Nov 24, 2020

You don't have to do that - there are events that the focus plugin can listen to.

template<class wlr_event_t>

I see; I've created an alternative version with this.

My problem in this case is that there is no way to not change focus if the click was handled by a plugin; this can mess up things if a plugin wants to manipulate focus. E.g. this is a problem for my mouse gesture plugin. To mitigate this, I've added options to enable / disable focus behavior separately with the middle and right mouse buttons. This solves the problem in my case (since I use a right click to start gestures -- I could also add an option there to change focus), but it might come up with other plugins that try to change focus.

@ammen99
Copy link
Member

ammen99 commented Nov 24, 2020

My problem in this case is that there is no way to not change focus if the click was handled by a plugin; this can mess up things if a plugin wants to manipulate focus.

How is this different from before?

@dkondor
Copy link
Contributor Author

dkondor commented Nov 24, 2020

My problem in this case is that there is no way to not change focus if the click was handled by a plugin; this can mess up things if a plugin wants to manipulate focus.

How is this different from before?

Before, this issue could not come up, since only the left button was matched (it is unlikely that would be set as a plugin activator). With the default behavior, there shouldn't be any change though.

In my previous version, when the focus logic was in pointer.cpp, it could be skipped if a binding matched.

Anyway, it is working for my use case, and for all the official plugins (I don't think any of them tries to set the focus away from the clicked view) and if anyone else has this issue, that could be solved separately.

metadata/core.xml Outdated Show resolved Hide resolved
metadata/core.xml Outdated Show resolved Hide resolved
Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

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

I'd prefer the full names of the bindings.

Otherwise, LGTM.

@dkondor
Copy link
Contributor Author

dkondor commented Nov 25, 2020

I'd prefer the full names of the bindings.
Sure, I've adjusted it

Otherwise, LGTM.
Should I squash everything to one commit?

@ammen99
Copy link
Member

ammen99 commented Nov 25, 2020

Don't worry, I can squash the commits when merging.

@ammen99
Copy link
Member

ammen99 commented Nov 25, 2020

Thanks!

@ammen99 ammen99 merged commit c180c29 into WayfireWM:master Nov 25, 2020
@dkondor dkondor deleted the focus_click_modifier branch November 25, 2020 15:58
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.

make middle-click also focus the view below the mouse
2 participants