Drag-and-drop upload for Maniphest

Summary:
This needs a bunch of UI polish (critically, it's totally undiscoverable) but it
basically works correctly. I'll clean it up in some followups.

Test Plan:
Uploaded some files via drag-and-drop, made comments, etc.

Reviewed By: aran
Reviewers: tomo, aran, jungejason, tuomaspelkonen
CC: anjali, aran
Differential Revision: 332

src/__celerity_resource_map__.php

@@ -81,7 +81,7 @@ celerity_register_resource_map(array(
   ),
   'aphront-panel-view-css' =>
   array(
-    'uri' => '/res/5ca2f692/rsrc/css/aphront/panel-view.css',
+    'uri' => '/res/e0139b9c/rsrc/css/aphront/panel-view.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -381,6 +381,16 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/js/application/differential/behavior-show-all-comments.js',
   ),
+  0 =>
+  array(
+    'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js',
+    'type' => 'js',
+    'requires' =>
+    array(
+      0 => 'javelin-install',
+    ),
+    'disk' => '/rsrc/js/javelin/docs/Base.js',
+  ),
   'javelin-behavior-differential-show-more' =>
   array(
     'uri' => '/res/9cbf1c9c/rsrc/js/application/differential/behavior-show-more.js',
@@ -456,6 +466,18 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/js/application/maniphest/behavior-transaction-controls.js',
   ),
+  'javelin-behavior-maniphest-transaction-drag-and-drop' =>
+  array(
+    'uri' => '/res/5bf1f40c/rsrc/js/application/maniphest/behavior-transaction-drag-and-drop.js',
+    'type' => 'js',
+    'requires' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-dom',
+      2 => 'phabricator-drag-and-drop-file-upload',
+    ),
+    'disk' => '/rsrc/js/application/maniphest/behavior-transaction-drag-and-drop.js',
+  ),
   'javelin-behavior-maniphest-transaction-expand' =>
   array(
     'uri' => '/res/966410de/rsrc/js/application/maniphest/behavior-transaction-expand.js',
@@ -533,16 +555,6 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/js/javelin/lib/DOM.js',
   ),
-  0 =>
-  array(
-    'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js',
-    'type' => 'js',
-    'requires' =>
-    array(
-      0 => 'javelin-install',
-    ),
-    'disk' => '/rsrc/js/javelin/docs/Base.js',
-  ),
   'javelin-event' =>
   array(
     'uri' => '/res/25c7c9e8/rsrc/js/javelin/core/Event.js',
@@ -768,7 +780,7 @@ celerity_register_resource_map(array(
   ),
   'maniphest-transaction-detail-css' =>
   array(
-    'uri' => '/res/927f4430/rsrc/css/application/maniphest/transaction-detail.css',
+    'uri' => '/res/14758b00/rsrc/css/application/maniphest/transaction-detail.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -853,6 +865,20 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/css/application/directory/phabricator-directory.css',
   ),
+  'phabricator-drag-and-drop-file-upload' =>
+  array(
+    'uri' => '/res/711efc61/rsrc/js/application/core/DragAndDropFileUpload.js',
+    'type' => 'js',
+    'requires' =>
+    array(
+      0 => 'javelin-install',
+      1 => 'javelin-util',
+      2 => 'javelin-request',
+      3 => 'javelin-dom',
+      4 => 'javelin-uri',
+    ),
+    'disk' => '/rsrc/js/application/core/DragAndDropFileUpload.js',
+  ),
   'phabricator-object-selector-css' =>
   array(
     'uri' => '/res/ced4098a/rsrc/css/application/objectselector/object-selector.css',
@@ -933,30 +959,6 @@ celerity_register_resource_map(array(
       'uri' => '/res/pkg/03ef179e/diffusion.pkg.css',
       'type' => 'css',
     ),
-    '0ab8eff6' =>
-    array (
-      'name' => 'core.pkg.css',
-      'symbols' =>
-      array (
-        0 => 'phabricator-core-css',
-        1 => 'phabricator-core-buttons-css',
-        2 => 'phabricator-standard-page-view',
-        3 => 'aphront-dialog-view-css',
-        4 => 'aphront-form-view-css',
-        5 => 'aphront-panel-view-css',
-        6 => 'aphront-side-nav-view-css',
-        7 => 'aphront-table-view-css',
-        8 => 'aphront-crumbs-view-css',
-        9 => 'aphront-tokenizer-control-css',
-        10 => 'aphront-typeahead-control-css',
-        11 => 'aphront-list-filter-view-css',
-        12 => 'phabricator-directory-css',
-        13 => 'phabricator-remarkup-css',
-        14 => 'syntax-highlighting-css',
-      ),
-      'uri' => '/res/pkg/0ab8eff6/core.pkg.css',
-      'type' => 'css',
-    ),
     '122a6b6d' =>
     array (
       'name' => 'workflow.pkg.js',
@@ -1002,6 +1004,30 @@ celerity_register_resource_map(array(
       'uri' => '/res/pkg/33f413ef/typeahead.pkg.js',
       'type' => 'js',
     ),
+    'ac3f56cc' =>
+    array (
+      'name' => 'core.pkg.css',
+      'symbols' =>
+      array (
+        0 => 'phabricator-core-css',
+        1 => 'phabricator-core-buttons-css',
+        2 => 'phabricator-standard-page-view',
+        3 => 'aphront-dialog-view-css',
+        4 => 'aphront-form-view-css',
+        5 => 'aphront-panel-view-css',
+        6 => 'aphront-side-nav-view-css',
+        7 => 'aphront-table-view-css',
+        8 => 'aphront-crumbs-view-css',
+        9 => 'aphront-tokenizer-control-css',
+        10 => 'aphront-typeahead-control-css',
+        11 => 'aphront-list-filter-view-css',
+        12 => 'phabricator-directory-css',
+        13 => 'phabricator-remarkup-css',
+        14 => 'syntax-highlighting-css',
+      ),
+      'uri' => '/res/pkg/ac3f56cc/core.pkg.css',
+      'type' => 'css',
+    ),
     'ce1d51a9' =>
     array (
       'name' => 'javelin.pkg.js',
@@ -1038,15 +1064,15 @@ celerity_register_resource_map(array(
   ),
   'reverse' =>
   array (
-    'aphront-crumbs-view-css' => '0ab8eff6',
+    'aphront-crumbs-view-css' => 'ac3f56cc',
-    'aphront-dialog-view-css' => '0ab8eff6',
+    'aphront-dialog-view-css' => 'ac3f56cc',
-    'aphront-form-view-css' => '0ab8eff6',
+    'aphront-form-view-css' => 'ac3f56cc',
-    'aphront-list-filter-view-css' => '0ab8eff6',
+    'aphront-list-filter-view-css' => 'ac3f56cc',
-    'aphront-panel-view-css' => '0ab8eff6',
+    'aphront-panel-view-css' => 'ac3f56cc',
-    'aphront-side-nav-view-css' => '0ab8eff6',
+    'aphront-side-nav-view-css' => 'ac3f56cc',
-    'aphront-table-view-css' => '0ab8eff6',
+    'aphront-table-view-css' => 'ac3f56cc',
-    'aphront-tokenizer-control-css' => '0ab8eff6',
+    'aphront-tokenizer-control-css' => 'ac3f56cc',
-    'aphront-typeahead-control-css' => '0ab8eff6',
+    'aphront-typeahead-control-css' => 'ac3f56cc',
     'differential-changeset-view-css' => '1ac25e8a',
     'differential-core-view-css' => '1ac25e8a',
     'differential-revision-add-comment-css' => '1ac25e8a',
@@ -1081,11 +1107,11 @@ celerity_register_resource_map(array(
     'javelin-util' => 'ce1d51a9',
     'javelin-vector' => 'ce1d51a9',
     'javelin-workflow' => '122a6b6d',
-    'phabricator-core-buttons-css' => '0ab8eff6',
+    'phabricator-core-buttons-css' => 'ac3f56cc',
-    'phabricator-core-css' => '0ab8eff6',
+    'phabricator-core-css' => 'ac3f56cc',
-    'phabricator-directory-css' => '0ab8eff6',
+    'phabricator-directory-css' => 'ac3f56cc',
-    'phabricator-remarkup-css' => '0ab8eff6',
+    'phabricator-remarkup-css' => 'ac3f56cc',
-    'phabricator-standard-page-view' => '0ab8eff6',
+    'phabricator-standard-page-view' => 'ac3f56cc',
-    'syntax-highlighting-css' => '0ab8eff6',
+    'syntax-highlighting-css' => 'ac3f56cc',
   ),
 ));

src/__phutil_library_map__.php

@@ -312,6 +312,7 @@ phutil_register_library_map(array(
     'PhabricatorFile' => 'applications/files/storage/file',
     'PhabricatorFileController' => 'applications/files/controller/base',
     'PhabricatorFileDAO' => 'applications/files/storage/base',
+    'PhabricatorFileDropUploadController' => 'applications/files/controller/dropupload',
     'PhabricatorFileImageMacro' => 'applications/files/storage/imagemacro',
     'PhabricatorFileListController' => 'applications/files/controller/list',
     'PhabricatorFileMacroDeleteController' => 'applications/files/controller/macrodelete',
@@ -749,6 +750,7 @@ phutil_register_library_map(array(
     'PhabricatorFile' => 'PhabricatorFileDAO',
     'PhabricatorFileController' => 'PhabricatorController',
     'PhabricatorFileDAO' => 'PhabricatorLiskDAO',
+    'PhabricatorFileDropUploadController' => 'PhabricatorFileController',
     'PhabricatorFileImageMacro' => 'PhabricatorFileDAO',
     'PhabricatorFileListController' => 'PhabricatorFileController',
     'PhabricatorFileMacroDeleteController' => 'PhabricatorFileController',

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -52,6 +52,7 @@ class AphrontDefaultApplicationConfiguration
       '/file/' => array(
         '$' => 'PhabricatorFileListController',
         'upload/$' => 'PhabricatorFileUploadController',
+        'dropupload/$' => 'PhabricatorFileDropUploadController',
         '(?P<view>info)/(?P<phid>[^/]+)/' => 'PhabricatorFileViewController',
         '(?P<view>view)/(?P<phid>[^/]+)/' => 'PhabricatorFileViewController',
         '(?P<view>download)/(?P<phid>[^/]+)/' => 'PhabricatorFileViewController',

src/applications/files/controller/dropupload/PhabricatorFileDropUploadController.php

@@ -0,0 +1,42 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class PhabricatorFileDropUploadController extends PhabricatorFileController {
+
+  public function processRequest() {
+    $request = $this->getRequest();
+
+    $data = file_get_contents('php://input');
+    $name = $request->getStr('name');
+
+    $file = PhabricatorFile::newFromFileData(
+      $data,
+      array(
+        'name' => $request->getStr('name'),
+      ));
+
+    return id(new AphrontAjaxResponse())->setContent(
+      array(
+        'name' => $file->getName(),
+        'phid' => $file->getPHID(),
+        'size' => $file->getByteSize(),
+        'uri'  => $file->getViewURI(),
+      ));
+  }
+
+}

src/applications/files/controller/dropupload/__init__.php

@@ -0,0 +1,16 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/response/ajax');
+phutil_require_module('phabricator', 'applications/files/controller/base');
+phutil_require_module('phabricator', 'applications/files/storage/file');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('PhabricatorFileDropUploadController.php');

src/applications/maniphest/controller/taskdetail/ManiphestTaskDetailController.php

@@ -285,6 +285,13 @@ class ManiphestTaskDetailController extends ManiphestController {
           ->setName('comments')
           ->setValue($draft_text)
           ->setID('transaction-comments'))
+      ->appendChild(
+        id(new AphrontFormMarkupControl())
+          ->setLabel('Attached Files')
+          ->setValue(
+            '<div id="file-list">'.
+              'None'.
+            '</div>'))
       ->appendChild(
         id(new AphrontFormSubmitControl())
           ->setValue('Avast!'));
@@ -328,11 +335,22 @@ class ManiphestTaskDetailController extends ManiphestController {
       'map'      => $control_map,
     ));
 
+    $id = celerity_generate_unique_node_id();
 
     $comment_panel = new AphrontPanelView();
     $comment_panel->appendChild($comment_form);
-    $comment_panel->setHeader('Weigh In');
+    $comment_panel->setID($id);
     $comment_panel->addClass('aphront-panel-accent');
+    $comment_panel->setHeader('Leap Into Action');
+
+    Javelin::initBehavior(
+      'maniphest-transaction-drag-and-drop',
+      array(
+        'target'          => $id,
+        'activatedClass'  => 'aphront-panel-view-drag-and-drop',
+        'uri'             => '/file/dropupload/',
+        'list'            => 'file-list',
+      ));
 
     $preview_panel =
       '<div class="aphront-panel-preview">

src/applications/maniphest/controller/taskdetail/__init__.php

@@ -22,6 +22,7 @@ phutil_require_module('phabricator', 'infrastructure/celerity/api');
 phutil_require_module('phabricator', 'infrastructure/javelin/api');
 phutil_require_module('phabricator', 'view/form/base');
 phutil_require_module('phabricator', 'view/form/control/file');
+phutil_require_module('phabricator', 'view/form/control/markup');
 phutil_require_module('phabricator', 'view/form/control/select');
 phutil_require_module('phabricator', 'view/form/control/submit');
 phutil_require_module('phabricator', 'view/form/control/textarea');

src/applications/maniphest/controller/transactionsave/ManiphestTransactionSaveController.php

@@ -27,8 +27,56 @@ class ManiphestTransactionSaveController extends ManiphestController {
       return new Aphront404Response();
     }
 
+    $transactions = array();
+
     $action = $request->getStr('action');
 
+    // If we have drag-and-dropped files, attach them first in a separate
+    // transaction. These can come in on any transaction type, which is why we
+    // handle them separately.
+    $files = array();
+
+    // Look for drag-and-drop uploads first.
+    $file_phids = $request->getArr('files');
+    if ($file_phids) {
+      $files = id(new PhabricatorFile())->loadAllWhere(
+        'phid in (%Ls)',
+        $file_phids);
+    }
+
+    // This means "attach a file" even though we store other types of data
+    // as 'attached'.
+    if ($action == ManiphestTransactionType::TYPE_ATTACH) {
+      if (!empty($_FILES['file'])) {
+        $err = idx($_FILES['file'], 'error');
+        if ($err != UPLOAD_ERR_NO_FILE) {
+          $file = PhabricatorFile::newFromPHPUpload($_FILES['file']);
+          $files[] = $file;
+        }
+      }
+    }
+
+    // If we had explicit or drag-and-drop files, create a transaction
+    // for those before we deal with whatever else might have happened.
+    $file_transaction = null;
+    if ($files) {
+      $files = mpull($files, 'getPHID', 'getPHID');
+      $new = $task->getAttached();
+      foreach ($files as $phid) {
+        if (empty($new[PhabricatorPHIDConstants::PHID_TYPE_FILE])) {
+          $new[PhabricatorPHIDConstants::PHID_TYPE_FILE] = array();
+        }
+        $new[PhabricatorPHIDConstants::PHID_TYPE_FILE][$phid] = array();
+      }
+      $transaction = new ManiphestTransaction();
+      $transaction
+        ->setAuthorPHID($user->getPHID())
+        ->setTransactionType(ManiphestTransactionType::TYPE_ATTACH);
+      $transaction->setNewValue($new);
+      $transactions[] = $transaction;
+      $file_transaction = $transaction;
+    }
+
     $transaction = new ManiphestTransaction();
     $transaction
       ->setAuthorPHID($user->getPHID())
@@ -36,8 +84,6 @@ class ManiphestTransactionSaveController extends ManiphestController {
       ->setTransactionType($action);
 
     switch ($action) {
-      case ManiphestTransactionType::TYPE_NONE:
-        break;
       case ManiphestTransactionType::TYPE_STATUS:
         $transaction->setNewValue($request->getStr('resolution'));
         break;
@@ -63,32 +109,22 @@ class ManiphestTransactionSaveController extends ManiphestController {
       case ManiphestTransactionType::TYPE_PRIORITY:
         $transaction->setNewValue($request->getInt('priority'));
         break;
+      case ManiphestTransactionType::TYPE_NONE:
       case ManiphestTransactionType::TYPE_ATTACH:
-        // This means "attach a file" even though we store other types of data
+        // If we have a file transaction, just get rid of this secondary
-        // as 'attached'.
+        // transaction and put the comments on it instead.
-        $phid = null;
+        if ($file_transaction) {
-        if (!empty($_FILES['file'])) {
+          $file_transaction->setComments($transaction->getComments());
-          $err = idx($_FILES['file'], 'error');
+          $transaction = null;
-          if ($err != UPLOAD_ERR_NO_FILE) {
-            $file = PhabricatorFile::newFromPHPUpload($_FILES['file']);
-            $phid = $file->getPHID();
         }
-        }
-        if ($phid) {
-          $new = $task->getAttached();
-          if (empty($new[PhabricatorPHIDConstants::PHID_TYPE_FILE])) {
-            $new[PhabricatorPHIDConstants::PHID_TYPE_FILE] = array();
-          }
-          $new[PhabricatorPHIDConstants::PHID_TYPE_FILE][$phid] = array();
-        }
-
-        $transaction->setNewValue($new);
         break;
       default:
         throw new Exception('unknown action');
     }
 
-    $transactions = array($transaction);
+    if ($transaction) {
+      $transactions[] = $transaction;
+    }
 
     switch ($action) {
       case ManiphestTransactionType::TYPE_OWNER:
@@ -140,6 +176,8 @@ class ManiphestTransactionSaveController extends ManiphestController {
         break;
     }
 
+
+
     $editor = new ManiphestTransactionEditor();
     $editor->applyTransactions($task, $transactions);
 

src/view/layout/panel/AphrontPanelView.php

@@ -26,6 +26,7 @@ final class AphrontPanelView extends AphrontView {
   private $header;
   private $width;
   private $classes = array();
+  private $id;
 
   public function setCreateButton($create_button, $href) {
     $this->addButton(
@@ -60,6 +61,11 @@ final class AphrontPanelView extends AphrontView {
     return $this;
   }
 
+  public function setID($id) {
+    $this->id = $id;
+    return $this;
+  }
+
   public function render() {
     if ($this->header !== null) {
       $header = '<h1>'.$this->header.'</h1>';
@@ -85,12 +91,13 @@ final class AphrontPanelView extends AphrontView {
       $classes[] = 'aphront-panel-width-'.$this->width;
     }
 
-    return
+    return phutil_render_tag(
-      '<div class="'.implode(' ', $classes).'">'.
+      'div',
-        $buttons.
+      array(
-        $header.
+        'class' => implode(' ', $classes),
-        $table.
+        'id'    => $this->id,
-      '</div>';
+      ),
+      $buttons.$header.$table);
   }
 
 }

webroot/rsrc/css/aphront/panel-view.css

@@ -62,3 +62,13 @@
 .aphront-panel-flush {
   margin: 0;
 }
+
+.aphront-panel-view-drag-and-drop {
+  background: #99ff99;
+  border-color: #669966;
+}
+
+.aphront-panel-view-drag-and-drop .aphront-form-view {
+  background: #ccffcc;
+  border-color: #669966;
+}

webroot/rsrc/css/application/maniphest/transaction-detail.css

@@ -67,8 +67,9 @@
 }
 
 .maniphest-transaction-comments {
+  border-top: 1px solid #e6e6e6;
   padding: 4px 1em;
-  background: #f3f3f3;
+  background: #fcfcfc;
 }
 
 .maniphest-change-table {

webroot/rsrc/js/application/core/DragAndDropFileUpload.js

@@ -0,0 +1,107 @@
+/**
+ * @requires javelin-install
+ *           javelin-util
+ *           javelin-request
+ *           javelin-dom
+ *           javelin-uri
+ * @provides phabricator-drag-and-drop-file-upload
+ * @javelin
+ */
+
+JX.install('PhabricatorDragAndDropFileUpload', {
+
+  construct : function(node) {
+    this._node = node;
+  },
+
+  events : ['willUpload', 'didUpload'],
+
+  members : {
+    _node : null,
+    _depth : 0,
+    _updateDepth : function(delta) {
+      this._depth += delta;
+      JX.DOM.alterClass(
+        this._node,
+        this.getActivatedClass(),
+        (this._depth > 0));
+    },
+
+    start : function() {
+
+      // TODO: move this to JX.DOM.contains()?
+      function contains(container, child) {
+        do {
+          if (child === container) {
+            return true;
+          }
+          child = child.parentNode;
+        } while (child);
+
+        return false;
+      }
+
+      // We track depth so that the _node may have children inside of it and
+      // not become unselected when they are dragged over.
+      JX.DOM.listen(
+        this._node,
+        'dragenter',
+        null,
+        JX.bind(this, function(e) {
+          if (contains(this._node, e.getTarget())) {
+            this._updateDepth(1);
+          }
+        }));
+
+      JX.DOM.listen(
+        this._node,
+        'dragleave',
+        null,
+        JX.bind(this, function(e) {
+          if (contains(this._node, e.getTarget())) {
+            this._updateDepth(-1);
+          }
+        }));
+
+      JX.DOM.listen(
+        this._node,
+        'dragover',
+        null,
+        function(e) {
+          e.kill();
+        });
+
+      JX.DOM.listen(
+        this._node,
+        'drop',
+        null,
+        JX.bind(this, function(e) {
+          e.kill();
+
+          var files = e.getRawEvent().dataTransfer.files;
+          for (var ii = 0; ii < files.length; ii++) {
+            var file = files[ii];
+
+            this.invoke('willUpload', file);
+
+            var up_uri = JX.$U(this.getURI())
+              .setQueryParam('name', file.name)
+              .toString();
+
+            new JX.Request(up_uri, JX.bind(this, function(r) {
+                this.invoke('didUpload', r);
+              }))
+              .setFile(file)
+              .send();
+          }
+
+          // Force depth to 0.
+          this._updateDepth(-this._depth);
+        }));
+    }
+  },
+  properties: {
+    URI : null,
+    activatedClass : null
+  }
+});

webroot/rsrc/js/application/maniphest/behavior-transaction-drag-and-drop.js

@@ -0,0 +1,39 @@
+/**
+ * @provides javelin-behavior-maniphest-transaction-drag-and-drop
+ * @requires javelin-behavior
+ *           javelin-dom
+ *           phabricator-drag-and-drop-file-upload
+ */
+
+JX.behavior('maniphest-transaction-drag-and-drop', function(config) {
+
+  var files = [];
+
+  var drop = new JX.PhabricatorDragAndDropFileUpload(JX.$(config.target))
+    .setActivatedClass(config.activatedClass)
+    .setURI(config.uri);
+
+  drop.listen('didUpload', function(f) {
+    files.push(f);
+    redraw();
+  });
+
+  drop.start();
+
+  function redraw() {
+    var items = [];
+    for (var ii = 0; ii < files.length; ii++) {
+      items.push(JX.$N('div', {}, files[ii].name));
+      items.push(JX.$N(
+        'input',
+        {
+          type: "hidden",
+          name: "files[" + files[ii].phid + "]",
+          value: files[ii].phid
+        }));
+    }
+    JX.DOM.setContent(JX.$(config.list), items);
+  }
+
+});
+