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

[ENH] Widget Insertion #3179

Merged
merged 5 commits into from
Aug 8, 2018
Merged

[ENH] Widget Insertion #3179

merged 5 commits into from
Aug 8, 2018

Conversation

irgolic
Copy link
Member

@irgolic irgolic commented Aug 1, 2018

Feature

Insert a widget in-between two already connected widgets by either:

  • dragging it from the widget selection menu onto the link,
  • right-clicking the link and selecting 'Insert Widget'
Description of changes
  • Added an 'Insert Node' command for purposes of undo/redo
  • Added entry to 'right-click on link' context menu
  • Widget drag from menu -> insert
Includes
  • Code changes
  • Tests
  • Documentation

@codecov-io
Copy link

codecov-io commented Aug 1, 2018

Codecov Report

Merging #3179 into master will increase coverage by 0.1%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #3179     +/-   ##
=========================================
+ Coverage   82.54%   82.64%   +0.1%     
=========================================
  Files         337      342      +5     
  Lines       58431    59016    +585     
=========================================
+ Hits        48229    48776    +547     
- Misses      10202    10240     +38

@@ -91,6 +93,9 @@ def setPath(self, path):
self.__shape = None
super().setPath(path)

def parent(self):
return self.__parent
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the same as parentItem()

@@ -1054,7 +1083,12 @@ def eventFilter(self, obj, event):
log.error("Unknown qualified name '%s'", qname)
else:
pos = event.scenePos()
self.createNewNode(desc, position=(pos.x(), pos.y()))
item = self.__scene.item_at(event.scenePos())
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use self.__scene.item_at(event.scenePos(), items.LinkItem) for better and easier hit detection (you also would not need the new LinkCurveItem.parent method).

SchemeLink(link.source_node, link.source_channel,
new_node, possible_links[0][0][1]), # first link, first entry, output (1)
SchemeLink(new_node, possible_links[1][0][0], # second link, first entry, input (0)
link.sink_node, link.sink_channel))
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic in __init__ should be moved out of the command class. Parameterize the command with the new node, old link and new link pairs.

@@ -69,6 +71,38 @@ def undo(self):
self.scheme.add_link(self.link)


class InsertNodeCommand(QUndoCommand):
def __init__(self, scheme, link, new_node, parent=None):
QUndoCommand.__init__(self, "Remove link", parent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Change the command name/text to 'Insert widget on link' or something similar.

sink_node = link.sink_node

if nodes_are_compatible(source_node.description, new_node_desc) and \
nodes_are_compatible(new_node_desc, sink_node.description):
Copy link
Contributor

Choose a reason for hiding this comment

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

The test here should also be constrained on the existing link.source_channel and link.sink_channel.

E.g. If I use:

screen shot 2018-08-02 at 11 21 41

and drop an Impute widget on the link, I get a

------------------- IncompatibleChannelTypeError Exception --------------------
Traceback (most recent call last):
  File "/Users/aleserjavec/workspace/orange3/Orange/canvas/document/schemeedit.py", line 1102, in eventFilter
    self.tryInsertNode(link, desc, pos)
  File "/Users/aleserjavec/workspace/orange3/Orange/canvas/document/schemeedit.py", line 1066, in tryInsertNode
    new_node, possible_links[0][0][1]),  # first link, first entry, output
  File "/Users/aleserjavec/workspace/orange3/Orange/canvas/scheme/link.py", line 125, in __init__
    % (source_channel.type, sink_channel.type)
Orange.canvas.scheme.errors.IncompatibleChannelTypeError: Cannot connect 'Orange.base.Model' to 'Orange.base.Learner'
-------------------------------------------------------------------------------

The set of possible links below should also be filtered correspondingly.

P.S.
The same also applies to the 'Insert widget` context menu action.

@lanzagar lanzagar added this to the 3.16 milestone Aug 3, 2018
@ales-erjavec
Copy link
Contributor

Can you also implement drop target detection in eventFilter for GraphicsSceneDragEnter, GraphicsSceneDragMove and GraphicsSceneDragLeave events and highlight the drop target.
Something like:

diff --git a/Orange/canvas/document/schemeedit.py b/Orange/canvas/document/schemeedit.py
index 840ece304..ddf453fa6 100644
--- a/Orange/canvas/document/schemeedit.py
+++ b/Orange/canvas/document/schemeedit.py
@@ -132,6 +132,7 @@ class SchemeEditWidget(QWidget):
         self.__possibleMouseItemsMove = False
         self.__itemsMoving = {}
         self.__contextMenuTarget = None
+        self.__dropTarget = None
         self.__quickMenu = None
         self.__quickTip = ""
 
@@ -1066,18 +1067,43 @@ class SchemeEditWidget(QWidget):
 
     def eventFilter(self, obj, event):
         # Filter the scene's drag/drop events.
+        MIME_TYPE = "application/vnv.orange-canvas.registry.qualified-name"
         if obj is self.scene():
             etype = event.type()
             if etype == QEvent.GraphicsSceneDragEnter or \
                     etype == QEvent.GraphicsSceneDragMove:
                 mime_data = event.mimeData()
-                if mime_data.hasFormat(
-                        "application/vnv.orange-canvas.registry.qualified-name"
-                        ):
+                droptarget = None
+                accept = True
+                if mime_data.hasFormat(MIME_TYPE):
+                    qname = bytes(mime_data.data(MIME_TYPE)).decode("ascii")
+                    try:
+                        desc = self.__registry.widget(qname)
+                    except KeyError:
+                        pass
+                    else:
+                        item = self.__scene.item_at(event.scenePos(),
+                                                    items.LinkItem)
+                        link = self.__scene.link_for_item(item) if item else None
+                        if link is not None:
+                            droptarget = item
+                            droptarget.setHoverState(True)
+                            accept = can_insert_node(desc, link)
+                if accept:
                     event.acceptProposedAction()
                 else:
                     event.ignore()
+                if self.__dropTarget is not None and \
+                        self.__dropTarget is not droptarget:
+                    self.__dropTarget.setHoverState(False)
+                    self.__dropTarget = None
+
+                self.__dropTarget = droptarget
                 return True
+            elif etype == QEvent.GraphicsSceneDragLeave:
+                if self.__dropTarget is not None:
+                    self.__dropTarget.setHoverState(False)
+                    self.__dropTarget = None

@ajdapretnar
Copy link
Contributor

Dragging the widget no longer works for me...?

@irgolic
Copy link
Member Author

irgolic commented Aug 8, 2018

Are you dragging a compatible widget to the link? If the widget is incompatible, it's simply added as if dragged to empty canvas.
https://gyazo.com/bcd54f4c9279e804ed43f3629e938530

@ajdapretnar
Copy link
Contributor

Yes. File - Select Columns - Data Table.

@irgolic
Copy link
Member Author

irgolic commented Aug 8, 2018

Resolved.

@irgolic irgolic closed this Aug 8, 2018
@ajdapretnar ajdapretnar reopened this Aug 8, 2018
@ajdapretnar ajdapretnar merged commit aa88f74 into biolab:master Aug 8, 2018
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.

5 participants