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

Calculate actual popover position for mouse event detection #63

Merged
merged 4 commits into from
Dec 17, 2017

Conversation

davidmhewitt
Copy link
Member

Fixes #62

The allocation of the Gtk.Box containing the indicator popover is used to determine if a click was inside the popover or not when deciding whether or not that click event should close the currently open popover.

Because an Allocation is relative to its parent this was always (0, 0) causing there to be a box the size of the current indicator popover to be in the top left that prevented you from clicking to close indicators. So, this PR does some maths to move the allocation calculation underneath the actual popover relative to the mouse co-ordinates.

I considered removing the mouse bounds check altogether as items inside the popover still receive click events properly. But removing it causes the application menu to be closed when clicking a blank space inside it, so not ideal.

Copy link
Collaborator

@donadigo donadigo left a comment

Choose a reason for hiding this comment

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

One question.

current_indicator.get_parent ().get_allocation (out container_allocation);

int wingpanel_width;
current_indicator.get_root_window ().get_geometry (null, null, out wingpanel_width, null);
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 we just use the already available owner field here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review, @donadigo

Since get_root_window returns a Gdk.Window, not a Gtk.Window. We still need the call to get_root_window. However, we still get the same resulting window if we get it through the owner and it may be slightly more efficient, so I've push a commit that switches to that.

Thanks.

Copy link
Collaborator

@donadigo donadigo left a comment

Choose a reason for hiding this comment

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

Okay, looks good. I actually didn't understand the part about getting the root window, but now I do. Both versions in this case are correct. You can revert the latest commit if you want. This to be tested by someone else with multi-monitor setup to confirm it's working as intended.

@donadigo donadigo merged commit 99d68c1 into master Dec 17, 2017
@donadigo donadigo deleted the fix-top-left-click-to-close branch December 17, 2017 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants