UX: Lightbox support for image uploader. (#7034)

app/assets/javascripts/discourse/components/image-uploader.js.es6

@@ -1,9 +1,25 @@
-import computed from "ember-addons/ember-computed-decorators";
+import {
+  default as computed,
+  observes
+} from "ember-addons/ember-computed-decorators";
+
 import UploadMixin from "discourse/mixins/upload";
+import lightbox from "discourse/lib/lightbox";
+import { ajax } from "discourse/lib/ajax";
+import { popupAjaxError } from "discourse/lib/ajax-error";
 
 export default Ember.Component.extend(UploadMixin, {
   classNames: ["image-uploader"],
-  infoHidden: true,
+
+  init() {
+    this._super(...arguments);
+    this._applyLightbox();
+  },
+
+  willDestroyElement() {
+    this._super(...arguments);
+    $("a.lightbox").magnificPopup("close");
+  },
 
   @computed("imageUrl")
   backgroundStyle(imageUrl) {
@@ -20,11 +36,6 @@ export default Ember.Component.extend(UploadMixin, {
     return imageUrl.split("/").slice(-1)[0];
   },
 
-  @computed("infoHidden", "imageBaseName")
-  showInfo(infoHidden, imageBaseName) {
-    return !infoHidden && imageBaseName;
-  },
-
   @computed("backgroundStyle")
   hasBackgroundStyle(backgroundStyle) {
     return !Ember.isEmpty(backgroundStyle.string);
@@ -35,16 +46,51 @@ export default Ember.Component.extend(UploadMixin, {
   },
 
   uploadDone(upload) {
-    this.setProperties({ imageUrl: upload.url, imageId: upload.id });
+    this.setProperties({
+      imageUrl: upload.url,
+      imageId: upload.id,
+      imageFilesize: upload.human_filesize,
+      imageFilename: upload.original_filename,
+      imageWidth: upload.width,
+      imageHeight: upload.height
+    });
+
+    this._applyLightbox();
 
     if (this.onUploadDone) {
       this.onUploadDone(upload);
     }
   },
 
+  _openLightbox() {
+    Ember.run.next(() => this.$("a.lightbox").magnificPopup("open"));
+  },
+
+  _applyLightbox() {
+    if (this.get("imageUrl")) Ember.run.next(() => lightbox(this.$()));
+  },
+
   actions: {
-    toggleInfo() {
+    toggleLightbox() {
-      this.toggleProperty("infoHidden");
+      if (this.get("imageFilename")) {
+        this._openLightbox();
+      } else {
+        ajax(`/uploads/lookup-metadata`, {
+          type: "POST",
+          data: { url: this.get("imageUrl") }
+        })
+          .then(json => {
+            this.setProperties({
+              imageFilename: json.original_filename,
+              imageFilesize: json.human_filesize,
+              imageWidth: json.width,
+              imageHeight: json.height
+            });
+
+            this._openLightbox();
+          })
+          .catch(popupAjaxError);
+      }
     },
 
     trash() {

app/assets/javascripts/discourse/templates/components/image-uploader.hbs

@@ -9,18 +9,28 @@
       <button {{action "trash"}} class="btn btn-danger pad-left no-text">{{d-icon "far-trash-alt"}}</button>
     {{/if}}
 
-    {{#if imageBaseName}}
+    {{#if imageUrl}}
-      {{d-button icon="info-circle" class="btn image-uploader-info-btn no-text"
+      {{d-button icon="discourse-expand"
-                                    title="upload_selector.filename"
+          title='expand'
-                                    action=(action "toggleInfo")}}
+          class="btn image-uploader-lightbox-btn no-text"
+          action=(action "toggleLightbox")}}
     {{/if}}
 
     <span class="btn {{unless uploading 'hidden'}}">{{i18n 'upload_selector.uploading'}} {{uploadProgress}}%</span>
   </div>
 
-  {{#if showInfo}}
+
-    <div class="image-uploader-info">
+  {{#if imageUrl}}
-      <a href={{imageUrl}} target="_blank">{{imageBaseName}}</a>
+    <a class="lightbox"
-    </div>
+       href={{imageUrl}}
+       title={{imageFilename}}
+       rel="nofollow noopener">
+
+      <div class="meta">
+        <span class="informations">
+          {{imageWidth}}x{{imageHeight}} {{imageFilesize}}
+        </span>
+      </div>
+    </a>
   {{/if}}
 </div>

app/assets/stylesheets/common/base/upload.scss

@@ -3,26 +3,6 @@
   background-size: cover;
   position: relative;
 
-  .image-uploader-info {
-    position: absolute;
-    bottom: 0;
-    width: 100%;
-    background: $primary;
-    text-align: center;
-    overflow: hidden;
-    white-space: nowrap;
-    text-overflow: ellipsis;
-    opacity: 0.6;
-
-    a {
-      color: $secondary;
-
-      &:hover {
-        text-decoration: underline;
-      }
-    }
-  }
-
   .image-upload-controls {
     display: flex;
 
@@ -30,7 +10,7 @@
       margin-right: 5px;
     }
 
-    .image-uploader-info-btn {
+    .image-uploader-lightbox-btn {
       background: none;
       margin-right: 0;
       margin-left: auto;

app/controllers/uploads_controller.rb

@@ -86,6 +86,19 @@ class UploadsController < ApplicationController
     end
   end
 
+  def metadata
+    params.require(:url)
+    upload = Upload.get_from_url(params[:url])
+    raise Discourse::NotFound unless upload
+
+    render json: {
+      original_filename: upload.original_filename,
+      width: upload.width,
+      height: upload.height,
+      human_filesize: upload.human_filesize
+    }
+  end
+
   protected
 
   def render_404

app/models/upload.rb

@@ -7,6 +7,8 @@ require_dependency "file_store/local_store"
 require_dependency "base62"
 
 class Upload < ActiveRecord::Base
+  include ActionView::Helpers::NumberHelper
+
   SHA1_LENGTH = 40
 
   belongs_to :user
@@ -208,6 +210,10 @@ class Upload < ActiveRecord::Base
     upload || Upload.find_by("url LIKE ?", "%#{data[1]}")
   end
 
+  def human_filesize
+    number_to_human_size(self.filesize)
+  end
+
   def self.migrate_to_new_scheme(limit = nil)
     problems = []
 

app/serializers/upload_serializer.rb

@@ -9,5 +9,6 @@ class UploadSerializer < ApplicationSerializer
              :thumbnail_height,
              :extension,
              :short_url,
-             :retain_hours
+             :retain_hours,
+             :human_filesize
 end

config/locales/client.en.yml

@@ -206,6 +206,7 @@ en:
         us_west_2: "US West (Oregon)"
 
     edit: "edit the title and category of this topic"
+    expand: "Expand"
     not_implemented: "That feature hasn't been implemented yet, sorry!"
     no_value: "No"
     yes_value: "Yes"
@@ -1569,7 +1570,6 @@ en:
       select_file: "Select File"
       image_link: "link your image will point to"
       default_image_alt_text: image
-      filename: "Filename"
 
     search:
       sort_by: "Sort by"

config/routes.rb

@@ -461,6 +461,7 @@ Discourse::Application.routes.draw do
   get "stylesheets/:name.css" => "stylesheets#show", constraints: { name: /[-a-z0-9_]+/ }
   get "theme-javascripts/:digest.js" => "theme_javascripts#show", constraints: { digest: /\h{40}/ }
 
+  post "uploads/lookup-metadata" => "uploads#metadata"
   post "uploads" => "uploads#create"
   post "uploads/lookup-urls" => "uploads#lookup_urls"
 

lib/cooked_post_processor.rb

@@ -6,8 +6,6 @@ require_dependency 'pretty_text'
 require_dependency 'quote_comparer'
 
 class CookedPostProcessor
-  include ActionView::Helpers::NumberHelper
-
   INLINE_ONEBOX_LOADING_CSS_CLASS = "inline-onebox-loading"
   INLINE_ONEBOX_CSS_CLASS = "inline-onebox"
   LOADING_SIZE = 10
@@ -422,7 +420,7 @@ class CookedPostProcessor
 
     filename = get_filename(upload, img["src"])
     informations = "#{original_width}×#{original_height}"
-    informations << " #{number_to_human_size(upload.filesize)}" if upload
+    informations << " #{upload.human_filesize}" if upload
 
     a["title"] = CGI.escapeHTML(img["title"] || filename)
 

spec/requests/uploads_controller_spec.rb

@@ -283,4 +283,51 @@ describe UploadsController do
       expect(result[0]["url"]).to eq(upload.url)
     end
   end
+
+  describe '#metadata' do
+    let(:upload) { Fabricate(:upload) }
+
+    describe 'when url is missing' do
+      it 'should return the right response' do
+        post "/uploads/lookup-metadata.json"
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    describe 'when not signed in' do
+      it 'should return the right response' do
+        post "/uploads/lookup-metadata.json", params: { url: upload.url }
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    describe 'when signed in' do
+      before do
+        sign_in(Fabricate(:user))
+      end
+
+      describe 'when url is invalid' do
+        it 'should return the right response' do
+          post "/uploads/lookup-metadata.json", params: { url: 'abc' }
+
+          expect(response.status).to eq(404)
+        end
+      end
+
+      it "should return the right response" do
+        post "/uploads/lookup-metadata.json", params: { url: upload.url }
+
+        expect(response.status).to eq(200)
+
+        result = JSON.parse(response.body)
+
+        expect(result["original_filename"]).to eq(upload.original_filename)
+        expect(result["width"]).to eq(upload.width)
+        expect(result["height"]).to eq(upload.height)
+        expect(result["human_filesize"]).to eq(upload.human_filesize)
+      end
+    end
+  end
 end

test/javascripts/components/image-uploader-test.js.es6

@@ -17,18 +17,12 @@ componentTest("with image", {
       "it displays the trash icon"
     );
 
-    assert.equal(
+    await click(".image-uploader-lightbox-btn");
-      this.$(".image-uploader-info").length,
-      0,
-      "it does not display the image info"
-    );
-
-    await click(".image-uploader-info-btn");
 
     assert.equal(
-      this.$(".image-uploader-info").length,
+      $(".mfp-container").length,
       1,
-      "it displays the image info"
+      "it displays the image lightbox"
     );
   }
 });
@@ -50,9 +44,9 @@ componentTest("without image", {
     );
 
     assert.equal(
-      this.$(".image-uploader-info-btn").length,
+      this.$(".image-uploader-lightbox-btn").length,
       0,
-      "it does not display the image info button toggle"
+      "it does not display the button to open image lightbox"
     );
   }
 });

test/javascripts/helpers/create-pretender.js.es6

@@ -536,6 +536,15 @@ export default function() {
       });
     });
 
+    this.post("/uploads/lookup-metadata", () => {
+      return response(200, {
+        imageFilename: "somefile.png",
+        imageFilesize: "10 KB",
+        imageWidth: "1",
+        imageHeight: "1"
+      });
+    });
+
     this.get("/inline-onebox", request => {
       if (
         request.queryParams.urls.includes(