Allow commenting on non-viewable lightbox files

Summary: Removes the viewable restriction on embedded files. Builds a basic lightbox UI for commenting.

Test Plan:
Add psd, pdf to Maniphest task, clicked on download, comment, left comment. Closed box.

{F1943726}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T3612

Differential Revision: https://secure.phabricator.com/D16917

resources/celerity/map.php

@@ -9,8 +9,8 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '0b64e988',
     'conpherence.pkg.js' => '6249a1cf',
-    'core.pkg.css' => '3aeb7eb6',
+    'core.pkg.css' => '92233377',
-    'core.pkg.js' => 'f9083b80',
+    'core.pkg.js' => '519f84e8',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => 'a4ba74b5',
     'differential.pkg.js' => '634399e9',
@@ -132,7 +132,7 @@ return array(
     'rsrc/css/phui/phui-chart.css' => '6bf6f78e',
     'rsrc/css/phui/phui-cms.css' => 'be43c8a8',
     'rsrc/css/phui/phui-comment-form.css' => '4ecc56ef',
-    'rsrc/css/phui/phui-comment-panel.css' => '20ab1e5b',
+    'rsrc/css/phui/phui-comment-panel.css' => '5659325f',
     'rsrc/css/phui/phui-crumbs-view.css' => '195ac419',
     'rsrc/css/phui/phui-curtain-view.css' => '947bf1a4',
     'rsrc/css/phui/phui-document-pro.css' => 'c354e312',
@@ -151,7 +151,7 @@ return array(
     'rsrc/css/phui/phui-info-panel.css' => '27ea50a1',
     'rsrc/css/phui/phui-info-view.css' => 'ec92802a',
     'rsrc/css/phui/phui-invisible-character-view.css' => '6993d9f0',
-    'rsrc/css/phui/phui-lightbox.css' => 'e01994f2',
+    'rsrc/css/phui/phui-lightbox.css' => '04367b4f',
     'rsrc/css/phui/phui-list.css' => '9da2aa00',
     'rsrc/css/phui/phui-object-box.css' => '6b487c57',
     'rsrc/css/phui/phui-object-item-list-view.css' => '87278fa0',
@@ -505,7 +505,7 @@ return array(
     'rsrc/js/core/behavior-hovercard.js' => 'bcaccd64',
     'rsrc/js/core/behavior-keyboard-pager.js' => 'a8da01f0',
     'rsrc/js/core/behavior-keyboard-shortcuts.js' => '01fca1f0',
-    'rsrc/js/core/behavior-lightbox-attachments.js' => '35da14ae',
+    'rsrc/js/core/behavior-lightbox-attachments.js' => '2674e4fa',
     'rsrc/js/core/behavior-line-linker.js' => '1499a8cb',
     'rsrc/js/core/behavior-more.js' => 'a80d0378',
     'rsrc/js/core/behavior-object-selector.js' => 'e0ec7f2f',
@@ -651,7 +651,7 @@ return array(
     'javelin-behavior-history-install' => '7ee2b591',
     'javelin-behavior-icon-composer' => '8499b6ab',
     'javelin-behavior-launch-icon-composer' => '48086888',
-    'javelin-behavior-lightbox-attachments' => '35da14ae',
+    'javelin-behavior-lightbox-attachments' => '2674e4fa',
     'javelin-behavior-line-chart' => 'e4232876',
     'javelin-behavior-load-blame' => '42126667',
     'javelin-behavior-maniphest-batch-editor' => '782ab6e7',
@@ -847,7 +847,7 @@ return array(
     'phui-chart-css' => '6bf6f78e',
     'phui-cms-css' => 'be43c8a8',
     'phui-comment-form-css' => '4ecc56ef',
-    'phui-comment-panel-css' => '20ab1e5b',
+    'phui-comment-panel-css' => '5659325f',
     'phui-crumbs-view-css' => '195ac419',
     'phui-curtain-view-css' => '947bf1a4',
     'phui-document-summary-view-css' => '9ca48bdf',
@@ -869,7 +869,7 @@ return array(
     'phui-info-view-css' => 'ec92802a',
     'phui-inline-comment-view-css' => '5953c28e',
     'phui-invisible-character-view-css' => '6993d9f0',
-    'phui-lightbox-css' => 'e01994f2',
+    'phui-lightbox-css' => '04367b4f',
     'phui-list-view-css' => '9da2aa00',
     'phui-object-box-css' => '6b487c57',
     'phui-object-item-list-view-css' => '87278fa0',
@@ -1095,9 +1095,6 @@ return array(
       'javelin-install',
       'javelin-dom',
     ),
-    '20ab1e5b' => array(
-      'phui-timeline-view-css',
-    ),
     '21df4ff5' => array(
       'javelin-install',
       'javelin-workboard-card',
@@ -1109,6 +1106,15 @@ return array(
       'javelin-workflow',
       'javelin-util',
     ),
+    '2674e4fa' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-dom',
+      'javelin-mask',
+      'javelin-util',
+      'phuix-icon-view',
+      'phabricator-busy',
+    ),
     '2926fff2' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1171,15 +1177,6 @@ return array(
       'javelin-behavior-device',
       'javelin-vector',
     ),
-    '35da14ae' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-dom',
-      'javelin-mask',
-      'javelin-util',
-      'phuix-icon-view',
-      'phabricator-busy',
-    ),
     '3ab51e2c' => array(
       'javelin-behavior',
       'javelin-behavior-device',
@@ -1364,6 +1361,9 @@ return array(
       'phabricator-drag-and-drop-file-upload',
       'javelin-workboard-board',
     ),
+    '5659325f' => array(
+      'phui-timeline-view-css',
+    ),
     '58dea2fa' => array(
       'javelin-install',
       'javelin-util',

src/applications/files/controller/PhabricatorFileLightboxController.php

@@ -36,11 +36,15 @@ final class PhabricatorFileLightboxController
       ->withTransactionTypes(array(PhabricatorTransactions::TYPE_COMMENT));
     $timeline = $this->buildTransactionTimeline($file, $transactions);
 
-    if ($timeline->isTimelineEmpty()) {
-      //
-    }
     $comment_form = $this->renderCommentForm($file);
 
+    $info = phutil_tag(
+      'div',
+      array(
+        'class' => 'phui-comment-panel-header',
+      ),
+      $file->getName());
+
     require_celerity_resource('phui-comment-panel-css');
     $content = phutil_tag(
       'div',
@@ -48,6 +52,7 @@ final class PhabricatorFileLightboxController
         'class' => 'phui-comment-panel',
       ),
       array(
+        $info,
         $timeline,
         $comment_form,
       ));

src/view/layout/PhabricatorFileLinkView.php

@@ -88,14 +88,9 @@ final class PhabricatorFileLinkView extends AphrontView {
     require_celerity_resource('phabricator-remarkup-css');
     require_celerity_resource('phui-lightbox-css');
 
-    $sigil       = null;
+    $mustcapture = true;
-    $meta        = null;
+    $sigil = 'lightboxable';
-    $mustcapture = false;
+    $meta = $this->getMetadata();
-    if ($this->getFileViewable()) {
-      $mustcapture = true;
-      $sigil = 'lightboxable';
-      $meta = $this->getMetadata();
-    }
 
     $class = 'phabricator-remarkup-embed-layout-link';
     if ($this->getCustomClass()) {

src/view/page/PhabricatorStandardPageView.php

@@ -268,9 +268,6 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
         }
       }
 
-      $default_img_uri =
-        celerity_get_resource_uri(
-          'rsrc/image/icon/fatcow/document_black.png');
       $icon = id(new PHUIIconView())
         ->setIcon('fa-download');
       $lightbox_id = celerity_generate_unique_node_id();
@@ -296,7 +293,6 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
         'lightbox-attachments',
         array(
           'lightbox_id'     => $lightbox_id,
-          'defaultImageUri' => $default_img_uri,
           'downloadForm'    => $download_form,
         ));
     }

webroot/rsrc/css/phui/phui-comment-panel.css

@@ -16,6 +16,12 @@
   color: {$lightbluetext};
 }
 
+.phui-comment-panel-header {
+  font-weight: bold;
+  padding: 12px 16px 0;
+  word-break: break-word;
+}
+
 .phui-comment-panel .phui-timeline-view .phui-timeline-event-view {
   margin-left: 40px;
 }

webroot/rsrc/css/phui/phui-lightbox.css

@@ -22,6 +22,7 @@
   right: 0;
   bottom: 0;
   left: 0;
+  text-align: center;
 }
 
 .lightbox-attachment.comment-panel-open .lightbox-image-frame {
@@ -42,9 +43,28 @@
 .lightbox-comment-frame {
   position: absolute;
   top: -19999px;
-  bottom: -19999px;
+  bottom: 0;
+  right: 0;
   opacity: 0;
-  transition: all 0.2s;
+  transition: all 0.3s;
+}
+
+.lightbox-attachment .lightbox-icon-frame {
+  top: 44%;
+  left: calc(50% - 160px);
+  position: fixed;
+  display: block;
+  height: 120px;
+  width: 320px;
+}
+
+.lightbox-attachment.comment-panel-open .lightbox-icon-frame {
+  left: calc(50% - 340px);
+}
+
+.lightbox-attachment .phui-lightbox-file-icon {
+  font-size: 64px;
+  color: {$darkbluetext};
 }
 
 .comment-panel-open .lightbox-comment-frame {
@@ -63,8 +83,8 @@
 }
 
 .lightbox-attachment .attachment-name {
-  width: 100%;
+  line-height: 32px;
-  line-height: 30px;
+  font-size: {$biggerfontsize};
   text-align: center;
 }
 

webroot/rsrc/js/core/behavior-lightbox-attachments.js

@@ -79,37 +79,44 @@ JX.behavior('lightbox-attachments', function (config) {
     }
 
     var img_uri = '';
+    var img = '';
     var extra_status = '';
-    var name_element = '';
     // for now, this conditional is always true
     // revisit if / when we decide to add non-images to lightbox view
     if (target_data.viewable) {
       img_uri = target_data.uri;
+      var alt_name = '';
+      if (typeof target_data.name != 'undefined') {
+        alt_name = target_data.name;
+      }
+
+      img =
+        JX.$N('img',
+          {
+            className : 'loading',
+            alt : alt_name
+          }
+        );
     } else {
-      img_uri = config.defaultImageUri;
+      var imgIcon = new JX.PHUIXIconView()
-      extra_status = ' Image may not be representative of actual attachment.';
+        .setIcon('fa-file-text-o phui-lightbox-file-icon')
-      name_element =
+        .getNode();
+      var nameElement =
         JX.$N('div',
           {
             className : 'attachment-name'
           },
           target_data.name
         );
+      img =
+        JX.$N('div',
+          {
+            className : 'lightbox-icon-frame',
+          },
+          [ imgIcon, nameElement ]
+        );
     }
 
-    var alt_name = '';
-    if (typeof target_data.name != 'undefined') {
-      alt_name = target_data.name;
-    }
-
-    var img =
-      JX.$N('img',
-        {
-          className : 'loading',
-          alt : alt_name
-        }
-      );
-
     var imgFrame =
       JX.$N('div',
         {
@@ -146,7 +153,7 @@ JX.behavior('lightbox-attachments', function (config) {
         },
         [
           m_url,
-          ' Image ' + current + ' of ' + total + '.' + extra_status
+          ' Image ' + current + ' of ' + total + '.'
         ]
       );
 
@@ -184,7 +191,6 @@ JX.behavior('lightbox-attachments', function (config) {
        [statusSpan, closeButton, commentButton, downloadSpan]
       );
     JX.DOM.appendContent(lightbox, statusHTML);
-    JX.DOM.appendContent(lightbox, name_element);
     JX.DOM.listen(closeButton, 'click', null, closeLightBox);
 
     var leftIcon = '';
@@ -238,13 +244,15 @@ JX.behavior('lightbox-attachments', function (config) {
 
     document.body.appendChild(lightbox);
 
-    JX.Busy.start();
+    if (img_uri) {
-    img.onload = function() {
+      JX.Busy.start();
-      JX.DOM.alterClass(img, 'loading', false);
+      img.onload = function() {
-      JX.Busy.done();
+        JX.DOM.alterClass(img, 'loading', false);
-    };
+        JX.Busy.done();
+      };
 
-    img.src = img_uri;
+      img.src = img_uri;
+    }
     loadComments(target_data.phid);
   }