Don't show workboard action previews if the action won't have any effect

Summary: Ref T10335. When you (for example) drag a "Resolved" task into a column with "Trigger: change status to resolved.", don't show a hint that the action will "Change status to resolved." since this isn't helpful and is somewhat confusing. For now, the only visibility operator is "!=" since all current actions are simple field comparisons, but some actions in the future (like "add subscriber" or "remove project") might need other conditions. Test Plan: Dragged cards in ways that previously provided useless hints: move from column A to column B on a "Group by Priority" board; drag a resolved task to a "Trigger: change status to as resolved" column. Saw a more accurate preview in both cases. Drags which actually cause effects still show the effects correctly. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T10335 Differential Revision: https://secure.phabricator.com/D20300
2024-11-09 16:32:39 +01:00 · 2019-03-19 19:29:33 -07:00 · 2019-03-19 19:29:33 -07:00 · a5b3e33e3c
commit a5b3e33e3c
parent 5dca1569b5
9 changed files with 83 additions and 22 deletions
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@ -408,12 +408,12 @@ return array(
    'rsrc/js/application/phortune/phortune-credit-card-form.js' => 'd12d214f',
    'rsrc/js/application/policy/behavior-policy-control.js' => '0eaa33a9',
    'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '9347f172',
-    'rsrc/js/application/projects/WorkboardBoard.js' => 'ba6e36b0',
+    'rsrc/js/application/projects/WorkboardBoard.js' => '2f893acd',
    'rsrc/js/application/projects/WorkboardCard.js' => '0392a5d8',
    'rsrc/js/application/projects/WorkboardCardTemplate.js' => '2a61f8d4',
    'rsrc/js/application/projects/WorkboardColumn.js' => 'c344eb3c',
    'rsrc/js/application/projects/WorkboardController.js' => '42c7a5a7',
-    'rsrc/js/application/projects/WorkboardDropEffect.js' => '101121be',
+    'rsrc/js/application/projects/WorkboardDropEffect.js' => 'c808589e',
    'rsrc/js/application/projects/WorkboardHeader.js' => '111bfd2d',
    'rsrc/js/application/projects/WorkboardHeaderTemplate.js' => 'ebe83a6b',
    'rsrc/js/application/projects/WorkboardOrderTemplate.js' => '03e8891f',
@ -730,12 +730,12 @@ return array(
    'javelin-view-renderer' => '9aae2b66',
    'javelin-view-visitor' => '308f9fe4',
    'javelin-websocket' => 'fdc13e4e',
-    'javelin-workboard-board' => 'ba6e36b0',
+    'javelin-workboard-board' => '2f893acd',
    'javelin-workboard-card' => '0392a5d8',
    'javelin-workboard-card-template' => '2a61f8d4',
    'javelin-workboard-column' => 'c344eb3c',
    'javelin-workboard-controller' => '42c7a5a7',
-    'javelin-workboard-drop-effect' => '101121be',
+    'javelin-workboard-drop-effect' => 'c808589e',
    'javelin-workboard-header' => '111bfd2d',
    'javelin-workboard-header-template' => 'ebe83a6b',
    'javelin-workboard-order-template' => '03e8891f',
@ -1003,10 +1003,6 @@ return array(
      'javelin-workflow',
      'phuix-icon-view',
    ),
-    '101121be' => array(
-      'javelin-install',
-      'javelin-dom',
-    ),
    '111bfd2d' => array(
      'javelin-install',
    ),
@ -1170,6 +1166,18 @@ return array(
      'phuix-autocomplete',
      'javelin-mask',
    ),
+    '2f893acd' => array(
+      'javelin-install',
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-workflow',
+      'phabricator-draggable-list',
+      'javelin-workboard-column',
+      'javelin-workboard-header-template',
+      'javelin-workboard-card-template',
+      'javelin-workboard-order-template',
+    ),
    '308f9fe4' => array(
      'javelin-install',
      'javelin-util',
@ -1880,18 +1888,6 @@ return array(
      'javelin-uri',
      'phabricator-notification',
    ),
-    'ba6e36b0' => array(
-      'javelin-install',
-      'javelin-dom',
-      'javelin-util',
-      'javelin-stratcom',
-      'javelin-workflow',
-      'phabricator-draggable-list',
-      'javelin-workboard-column',
-      'javelin-workboard-header-template',
-      'javelin-workboard-card-template',
-      'javelin-workboard-order-template',
-    ),
    'bdce4d78' => array(
      'javelin-install',
      'javelin-util',
@ -1955,6 +1951,10 @@ return array(
      'phuix-icon-view',
      'phabricator-busy',
    ),
+    'c808589e' => array(
+      'javelin-install',
+      'javelin-dom',
+    ),
    'c8147a20' => array(
      'javelin-behavior',
      'javelin-dom',
--- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php
+++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php
@ -655,6 +655,8 @@ final class PhabricatorProjectBoardViewController
      $properties[$task->getPHID()] = array(
        'points' => (double)$task->getPoints(),
        'status' => $task->getStatus(),
+        'priority' => (int)$task->getPriority(),
+        'owner' => $task->getOwnerPHID(),
      );
    }

--- a/src/applications/project/icon/PhabricatorProjectDropEffect.php
+++ b/src/applications/project/icon/PhabricatorProjectDropEffect.php
@ -6,6 +6,7 @@ final class PhabricatorProjectDropEffect
  private $icon;
  private $color;
  private $content;
+  private $conditions = array();

  public function setIcon($icon) {
    $this->icon = $icon;
@ -39,7 +40,22 @@ final class PhabricatorProjectDropEffect
      'icon' => $this->getIcon(),
      'color' => $this->getColor(),
      'content' => hsprintf('%s', $this->getContent()),
+      'conditions' => $this->getConditions(),
    );
  }

+  public function addCondition($field, $operator, $value) {
+    $this->conditions[] = array(
+      'field' => $field,
+      'operator' => $operator,
+      'value' => $value,
+    );
+
+    return $this;
+  }
+
+  public function getConditions() {
+    return $this->conditions;
+  }
+
 }
--- a/src/applications/project/order/PhabricatorProjectColumnOwnerOrder.php
+++ b/src/applications/project/order/PhabricatorProjectColumnOwnerOrder.php
@ -171,6 +171,7 @@ final class PhabricatorProjectColumnOwnerOrder
          $this->newEffect()
            ->setIcon($owner_icon)
            ->setColor($owner_color)
+            ->addCondition('owner', '!=', $owner_phid)
            ->setContent($effect_content));
      }

--- a/src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php
+++ b/src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php
@ -68,6 +68,7 @@ final class PhabricatorProjectColumnPriorityOrder
      $drop_effect = $this->newEffect()
        ->setIcon($priority_icon)
        ->setColor($priority_color)
+        ->addCondition('priority', '!=', $priority)
        ->setContent(
          pht(
            'Change priority to %s.',
--- a/src/applications/project/order/PhabricatorProjectColumnStatusOrder.php
+++ b/src/applications/project/order/PhabricatorProjectColumnStatusOrder.php
@ -75,6 +75,7 @@ final class PhabricatorProjectColumnStatusOrder
      $drop_effect = $this->newEffect()
        ->setIcon($status_icon)
        ->setColor($status_color)
+        ->addCondition('status', '!=', $status_key)
        ->setContent(
          pht(
            'Change status to %s.',
--- a/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
+++ b/src/applications/project/trigger/PhabricatorProjectTriggerManiphestStatusRule.php
@ -51,6 +51,7 @@ final class PhabricatorProjectTriggerManiphestStatusRule
      $this->newEffect()
        ->setIcon($status_icon)
        ->setColor($status_color)
+        ->addCondition('status', '!=', $value)
        ->setContent($content),
    );
  }
--- a/webroot/rsrc/js/application/projects/WorkboardBoard.js
+++ b/webroot/rsrc/js/application/projects/WorkboardBoard.js
@ -228,6 +228,17 @@ JX.install('WorkboardBoard', {
        effects = effects.concat(header.getDropEffects());
      }

+      var card_phid = JX.Stratcom.getData(src_node).objectPHID;
+      var card = src_column.getCard(card_phid);
+
+      var visible = [];
+      for (var ii = 0; ii < effects.length; ii++) {
+        if (effects[ii].isEffectVisibleForCard(card)) {
+          visible.push(effects[ii]);
+        }
+      }
+      effects = visible;
+
      if (!effects.length) {
        JX.DOM.remove(node);
        return;
--- a/webroot/rsrc/js/application/projects/WorkboardDropEffect.js
+++ b/webroot/rsrc/js/application/projects/WorkboardDropEffect.js
@ -10,7 +10,8 @@ JX.install('WorkboardDropEffect', {
  properties: {
    icon: null,
    color: null,
-    content: null
+    content: null,
+    conditions: []
  },

  statics: {
@ -18,7 +19,8 @@ JX.install('WorkboardDropEffect', {
      return new JX.WorkboardDropEffect()
        .setIcon(map.icon)
        .setColor(map.color)
-        .setContent(JX.$H(map.content));
+        .setContent(JX.$H(map.content))
+        .setConditions(map.conditions || []);
    }
  },

@ -30,6 +32,32 @@ JX.install('WorkboardDropEffect', {
        .getNode();

      return JX.$N('li', {}, [icon, this.getContent()]);
+    },
+
+    isEffectVisibleForCard: function(card) {
+      var conditions = this.getConditions();
+
+      var properties = card.getProperties();
+      for (var ii = 0; ii < conditions.length; ii++) {
+        var condition = conditions[ii];
+
+        var field = properties[condition.field];
+        var value = condition.value;
+
+        var result = true;
+        switch (condition.operator) {
+          case '!=':
+            result = (field !== value);
+            break;
        }
+
+        if (!result) {
+          return false;
+        }
+      }
+
+      return true;
+    }
+
  }
 });