Skip to content

bugfix for wx backend: release mouse on loss of focus and before trying to recapture #7752

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

Merged
merged 1 commit into from
Jan 24, 2017

Conversation

newville
Copy link
Contributor

@newville newville commented Jan 6, 2017

This PR replaces #7652 to address #7636 in which wx._core.PyAssertionError would be raised with a message of

CaptureMouse(): Recapturing the mouse in the same window?

For more background, see the discussion in the above links and also at https://groups.google.com/forum/#!topic/wxpython-users/-NrlWBMNkwM .

WxPython "mouse capture" events are quite different on Windows and Posix, and there could be OS-specific code to achieve the same desired behavior (that the mouse can move out of a mpl canvas and back in without errors and without losing mouse events). The solution here is simple enough and works (AFAICT) on all platforms.

@codecov-io
Copy link

Current coverage is 62.11% (diff: 0.00%)

Merging #7752 into master will decrease coverage by <.01%

@@             master      #7752   diff @@
==========================================
  Files           174        174          
  Lines         56028      56034     +6   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          34805      34805          
- Misses        21223      21229     +6   
  Partials          0          0          

Powered by Codecov. Last update 1fa4dd7...7cedabc

@tacaswell tacaswell added this to the 2.0.1 (next bug fix release) milestone Jan 7, 2017
@tacaswell
Copy link
Member

attn @dhyams

Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

This appears to do what the previous PR does.

@@ -1023,20 +1026,31 @@ def _onKeyUp(self, evt):
evt.Skip()
FigureCanvasBase.key_release_event(self, key, guiEvent=evt)

def _set_capture(self, capture=True):
Copy link
Member

Choose a reason for hiding this comment

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

I don't see it being called anywhere without a value, so maybe the default value isn't necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would be gained by removing the default value?

Copy link
Member

Choose a reason for hiding this comment

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

Enforced explicitness at all callers (if that's what's required).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@QuLogic Sorry, I do not understand the meaning of your response -- If what is required?

The call signature is part of the documentation. Turning a keyword arg into a positional arg makes the code less readable. Won't change.

@QuLogic QuLogic changed the title bugfix for wx backend: release mouse on loss of focus and before trying to recapture [MRG+1] bugfix for wx backend: release mouse on loss of focus and before trying to recapture Jan 20, 2017
@NelleV NelleV merged commit 03c1eb5 into matplotlib:master Jan 24, 2017
@NelleV
Copy link
Member

NelleV commented Jan 24, 2017

Thanks @newville

@NelleV
Copy link
Member

NelleV commented Jan 24, 2017

backported to 2.0.x by 5a895da

@QuLogic QuLogic changed the title [MRG+1] bugfix for wx backend: release mouse on loss of focus and before trying to recapture bugfix for wx backend: release mouse on loss of focus and before trying to recapture Jan 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants