Merge pull request #5038 from irgolic/scatterplot-selection-ux

OWScatterPlotBase: Fixup selection UX

Orange/widgets/tests/base.py

@@ -7,7 +7,7 @@
 import numpy as np
 import scipy.sparse as sp
 
-from AnyQt.QtGui import QFont
+from AnyQt.QtGui import QFont, QTextDocumentFragment
 from AnyQt.QtCore import QRectF, QPointF
 from AnyQt.QtTest import QSignalSpy
 from AnyQt.QtWidgets import (
@@ -676,11 +676,9 @@ def test_class_density(self, timeout=DEFAULT_TIMEOUT):
 
     def test_dragging_tooltip(self):
         """Dragging tooltip depends on data being jittered"""
-        text = self.widget.graph.tiptexts[0]
+        text = QTextDocumentFragment.fromHtml(self.widget.graph.tiptexts[0]).toPlainText()
         self.send_signal(self.widget.Inputs.data, Table("heart_disease"))
         self.assertEqual(self.widget.graph.tip_textitem.toPlainText(), text)
-        self.widget.graph.controls.jitter_size.setValue(1)
-        self.assertGreater(self.widget.graph.tip_textitem.toPlainText(), text)
 
     def test_sparse_data(self, timeout=DEFAULT_TIMEOUT):
         """Test widget for sparse data"""

Orange/widgets/utils/plot/owplotgui.py

@@ -279,6 +279,7 @@ def __init__(self, gui, text, orientation, buttons, parent, nomargin=False):
                         self.buttons.update(s.buttons)
                         self.groups[buttons[i+1]] = s
                         i = j
+                        self.layout().addStretch()
                         break
                     else:
                         state_buttons.append(buttons[j])
@@ -291,7 +292,6 @@ def __init__(self, gui, text, orientation, buttons, parent, nomargin=False):
             else:
                 self.buttons[buttons[i][0]] = gui.tool_button(buttons[i], self)
             i = i + 1
-        self.layout().addStretch()
 
     def select_state(self, state):
         # SELECT_RECTANGLE = SELECT

Orange/widgets/visualize/owscatterplotgraph.py

@@ -12,7 +12,7 @@
 from AnyQt.QtGui import QColor, QPen, QBrush, QPainterPath, QTransform, \
     QPainter
 from AnyQt.QtWidgets import QApplication, QToolTip, QGraphicsTextItem, \
-    QGraphicsRectItem
+    QGraphicsRectItem, QGraphicsItemGroup
 
 import pyqtgraph as pg
 from pyqtgraph.graphicsItems.ScatterPlotItem import Symbols
@@ -540,6 +540,7 @@ def __init__(self, scatter_widget, parent=None, view_box=ViewBox):
         gui.OWComponent.__init__(self, scatter_widget)
 
         self.subset_is_shown = False
+        self.jittering_suspended = False
 
         self.view_box = view_box(self)
         _axis = {"left": AxisItem("left"), "bottom": AxisItem("bottom")}
@@ -557,7 +558,8 @@ def __init__(self, scatter_widget, parent=None, view_box=ViewBox):
         self.labels = []
 
         self.master = scatter_widget
-        self._create_drag_tooltip(self.plot_widget.scene())
+        tooltip = self._create_drag_tooltip()
+        self.view_box.setDragTooltip(tooltip)
 
         self.selection = None  # np.ndarray
 
@@ -596,15 +598,17 @@ def _create_legend(self, anchor):
         legend.restoreAnchor(anchor)
         return legend
 
-    def _create_drag_tooltip(self, scene):
+    def _create_drag_tooltip(self):
         tip_parts = [
-            (Qt.ShiftModifier, "Shift: Add group"),
-            (Qt.ShiftModifier + Qt.ControlModifier,
-             "Shift-{}: Append to group".
+            (Qt.ControlModifier,
+             "{}: Append to group".
              format("Cmd" if sys.platform == "darwin" else "Ctrl")),
+            (Qt.ShiftModifier, "Shift: Add group"),
             (Qt.AltModifier, "Alt: Remove")
         ]
-        all_parts = ", ".join(part for _, part in tip_parts)
+        all_parts = "<center>" + \
+                    ", ".join(part for _, part in tip_parts) + \
+                    "</center>"
         self.tiptexts = {
             int(modifier): all_parts.replace(part, "<b>{}</b>".format(part))
             for modifier, part in tip_parts
@@ -613,35 +617,39 @@ def _create_drag_tooltip(self, scene):
 
         self.tip_textitem = text = QGraphicsTextItem()
         # Set to the longest text
-        text.setHtml(self.tiptexts[Qt.ShiftModifier + Qt.ControlModifier])
+        text.setHtml(self.tiptexts[Qt.ControlModifier])
         text.setPos(4, 2)
         r = text.boundingRect()
+        text.setTextWidth(r.width())
         rect = QGraphicsRectItem(0, 0, r.width() + 8, r.height() + 4)
         rect.setBrush(QColor(224, 224, 224, 212))
         rect.setPen(QPen(Qt.NoPen))
         self.update_tooltip()
 
-        scene.drag_tooltip = scene.createItemGroup([rect, text])
-        scene.drag_tooltip.hide()
+        tooltip_group = QGraphicsItemGroup()
+        tooltip_group.addToGroup(rect)
+        tooltip_group.addToGroup(text)
+        return tooltip_group
 
     def update_tooltip(self, modifiers=Qt.NoModifier):
-        modifiers &= Qt.ShiftModifier + Qt.ControlModifier + Qt.AltModifier
-        text = self.tiptexts.get(int(modifiers), self.tiptexts[0])
-        self.tip_textitem.setHtml(text + self._get_jittering_tooltip())
-
-    def _get_jittering_tooltip(self):
-        warn_jittered = ""
-        if self.jitter_size:
-            warn_jittered = \
-                '<br/><br/>' \
-                '<span style="background-color: red; color: white; ' \
-                'font-weight: 500;">' \
-                '&nbsp;Warning: Selection is applied to unjittered data&nbsp;' \
-                '</span>'
-        return warn_jittered
+        text = self.tiptexts[0]
+        for mod in [Qt.ControlModifier,
+                    Qt.ShiftModifier,
+                    Qt.AltModifier]:
+            if modifiers & mod:
+                text = self.tiptexts.get(int(mod))
+                break
+        self.tip_textitem.setHtml(text)
+
+    def suspend_jittering(self):
+        self.jittering_suspended = True
+        self.update_jittering()
+
+    def unsuspend_jittering(self):
+        self.jittering_suspended = False
+        self.update_jittering()
 
     def update_jittering(self):
-        self.update_tooltip()
         x, y = self.get_coordinates()
         if x is None or len(x) == 0 or self.scatterplot_item is None:
             return
@@ -810,7 +818,7 @@ def jitter_coordinates(self, x, y):
         Display coordinates to random positions within ellipses with
         radiuses of `self.jittter_size` percents of spans
         """
-        if self.jitter_size == 0:
+        if self.jitter_size == 0 or self.jittering_suspended:
             return x, y
         return self._jitter_data(x, y)
 
@@ -1550,12 +1558,12 @@ def select_by_indices(self, indices):
         if self.selection is None:
             self.selection = np.zeros(self.n_valid, dtype=np.uint8)
         keys = QApplication.keyboardModifiers()
-        if keys & Qt.AltModifier:
-            self.selection_remove(indices)
-        elif keys & Qt.ShiftModifier and keys & Qt.ControlModifier:
+        if keys & Qt.ControlModifier:
             self.selection_append(indices)
         elif keys & Qt.ShiftModifier:
             self.selection_new_group(indices)
+        elif keys & Qt.AltModifier:
+            self.selection_remove(indices)
         else:
             self.selection_select(indices)
 
@@ -1565,7 +1573,7 @@ def selection_select(self, indices):
         self._update_after_selection()
 
     def selection_append(self, indices):
-        self.selection[indices] = np.max(self.selection)
+        self.selection[indices] = max(np.max(self.selection), 1)
         self._update_after_selection()
 
     def selection_new_group(self, indices):

Orange/widgets/visualize/utils/plotutils.py

@@ -158,6 +158,13 @@ def __init__(self, graph, enable_menu=False):
     def _dragtip_pos():
         return 10, 10
 
+    def setDragTooltip(self, tooltip):
+        scene = self.scene()
+        scene.addItem(tooltip)
+        tooltip.setPos(*self._dragtip_pos())
+        tooltip.hide()
+        scene.drag_tooltip = tooltip
+
     def updateScaleBox(self, p1, p2):
         """
         Overload to use ViewBox.mapToView instead of mapRectFromParent
@@ -184,6 +191,12 @@ def safe_update_scale_box(self, buttonDownPos, currentPos):
             y += 1
         self.updateScaleBox(buttonDownPos, pg.Point(x, y))
 
+    def _updateDragtipShown(self, enabled):
+        scene = self.scene()
+        dragtip = scene.drag_tooltip
+        if enabled != dragtip.isVisible():
+            dragtip.setVisible(enabled)
+
     # noinspection PyPep8Naming,PyMethodOverriding
     def mouseDragEvent(self, ev, axis=None):
         def get_mapped_rect():
@@ -196,16 +209,15 @@ def select():
             ev.accept()
             if ev.button() == Qt.LeftButton:
                 self.safe_update_scale_box(ev.buttonDownPos(), ev.pos())
-                scene = self.scene()
-                dragtip = scene.drag_tooltip
                 if ev.isFinish():
-                    dragtip.hide()
+                    self._updateDragtipShown(False)
+                    self.graph.unsuspend_jittering()
                     self.rbScaleBox.hide()
                     value_rect = get_mapped_rect()
                     self.graph.select_by_rectangle(value_rect)
                 else:
-                    dragtip.setPos(*self._dragtip_pos())
-                    dragtip.show()  # although possibly already shown
+                    self._updateDragtipShown(True)
+                    self.graph.suspend_jittering()
                     self.safe_update_scale_box(ev.buttonDownPos(), ev.pos())
 
         def zoom():
@@ -270,7 +282,7 @@ def scaleHistory(self, d):
     def mouseClickEvent(self, ev):
         if ev.button() == Qt.RightButton:  # undo zoom
             self.scaleHistory(-1)
-        else:
+        elif ev.modifiers() == Qt.NoModifier:
             ev.accept()
             self.graph.unselect_all()