Conversation
c5a1530 to
bfbdb75
Compare
pyqtgraph/dockarea/DockDrop.py
Outdated
| def __init__(self, allowedAreas=None): | ||
| def __init__(self, parent, allowedAreas=None): | ||
| object.__init__(self) | ||
| self.parent = parent |
There was a problem hiding this comment.
Hi @pijyoi I'm a little worried about the attribute name parent. While DockDrop does not inherit from QObject, many other objects around here do, which have a parent() and setParent() methods. Having an attribute named parent I see potentially causing confusion. I would recommend either adding parent or setParent methods, or changing the attribute name to something else that's analogous.
There was a problem hiding this comment.
It's not satisfactory that DockDrop should hold a reference to its owner class. I was trying to eliminate this reference altogether but DropAreaOverlay needs to be provided with a parent widget.
There was a problem hiding this comment.
I see that ROI.py has a class MouseDragHandler that serves the same purpose as this DockDrop class. Also not implemented as a mix-in.
6d73529 to
b335aca
Compare
|
This LGTM @pijyoi thanks for the PR. Always love to see if/else statements for binding specific codepaths to go away. |
This is a follow-up of #2286.
Problematic areas with the previous code:
The above issues could perhaps be solved by inheriting DockDrop on the left-hand-side instead. But a cleaner approach seems to be to use DockDrop as a non-mixin.