key events handler return value to True to stop propagation #6397

Merged
merged 2 commits into from May 12, 2016

Conversation

Projects
None yet
5 participants
Member

fariza commented May 11, 2016

Fixing #6349

Right now only stopping the key press and key release event from propagating after callback.
I am in doubt about the other events, should be stopped also?

@tacaswell how does it work in Qt?

@fariza fariza key events handler return value to False to stop propagation
c996ee6

mdboom added the needs_review label May 11, 2016

fariza changed the title from key events handler return value to False to stop propagation to key events handler return value to True to stop propagation May 11, 2016

Owner

tacaswell commented May 11, 2016

Qt does not have this notion of a 'callback' chain. Each signal can propagate to 0 or more slots, but they are, my understanding, all independently executed.

@QuLogic QuLogic commented on an outdated diff May 11, 2016

lib/matplotlib/backends/backend_gtk.py
@@ -300,14 +300,14 @@ def key_press_event(self, widget, event):
key = self._get_key(event)
if _debug: print("hit", key)
FigureCanvasBase.key_press_event(self, key, guiEvent=event)
- return False # finish event propagation?
+ return True # stoping event propagation
@QuLogic

QuLogic May 11, 2016

Member

"stopping", not "stoping", but actually, just use "stop".

Contributor

dopplershift commented May 11, 2016

@tacaswell Qt's signals/slots mechanism is separate from its event handling (like keyboard events). Qt's event handling does have the notion of stopping the propagating of events.

@fariza fariza comment correction
cc15c7c
Owner

tacaswell commented May 12, 2016

On a bit further reading, the notion of event propagation is handled by the method event in QObject which by default dispatches to a set of eventtypeEvent methods (all of which are void functions at the c++ layer) which we override. This 'accepts' the event and prevents propagation up to the parent widgets. We could re-enable that propagation by calling the default implementation of any of those methods, but as written of the Qt events are stopped by our handlers.

https://doc.qt.io/qt-4.8/qwidget.html#event

Member

fariza commented May 12, 2016

So with the changes in this PR we are at the same level that Qt

@tacaswell tacaswell merged commit 37a3baf into matplotlib:master May 12, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 69.749%
Details

tacaswell removed the needs_review label May 12, 2016

@tacaswell tacaswell added a commit that referenced this pull request May 12, 2016

@tacaswell tacaswell Merge pull request #6397 from fariza/stop-gtk-key-event-propagation
MNT: key events handler return value to True to stop propagation it gtk
9b39f3e
Owner

tacaswell commented May 12, 2016

backported to 2.x as 9b39f3e

fariza deleted the fariza:stop-gtk-key-event-propagation branch May 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment