From 58b0e945bd955b0cf700f23ea6ea35d62bd79815 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 21 Feb 2019 10:13:37 +0800 Subject: [PATCH] UX: Lightbox support for image uploader. (#7034) --- .../components/image-uploader.js.es6 | 66 ++++++++++++++++--- .../templates/components/image-uploader.hbs | 26 +++++--- .../stylesheets/common/base/upload.scss | 22 +------ app/controllers/uploads_controller.rb | 13 ++++ app/models/upload.rb | 6 ++ app/serializers/upload_serializer.rb | 3 +- config/locales/client.en.yml | 2 +- config/routes.rb | 1 + lib/cooked_post_processor.rb | 4 +- spec/requests/uploads_controller_spec.rb | 47 +++++++++++++ .../components/image-uploader-test.js.es6 | 16 ++--- .../helpers/create-pretender.js.es6 | 9 +++ 12 files changed, 160 insertions(+), 55 deletions(-) diff --git a/app/assets/javascripts/discourse/components/image-uploader.js.es6 b/app/assets/javascripts/discourse/components/image-uploader.js.es6 index 9ebb3f92906..1f73661861a 100644 --- a/app/assets/javascripts/discourse/components/image-uploader.js.es6 +++ b/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() { - this.toggleProperty("infoHidden"); + toggleLightbox() { + 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() { diff --git a/app/assets/javascripts/discourse/templates/components/image-uploader.hbs b/app/assets/javascripts/discourse/templates/components/image-uploader.hbs index 0014d71a4ee..d0769d0c62f 100644 --- a/app/assets/javascripts/discourse/templates/components/image-uploader.hbs +++ b/app/assets/javascripts/discourse/templates/components/image-uploader.hbs @@ -9,18 +9,28 @@ {{/if}} - {{#if imageBaseName}} - {{d-button icon="info-circle" class="btn image-uploader-info-btn no-text" - title="upload_selector.filename" - action=(action "toggleInfo")}} + {{#if imageUrl}} + {{d-button icon="discourse-expand" + title='expand' + class="btn image-uploader-lightbox-btn no-text" + action=(action "toggleLightbox")}} {{/if}} {{i18n 'upload_selector.uploading'}} {{uploadProgress}}% - {{#if showInfo}} -
- {{imageBaseName}} -
+ + {{#if imageUrl}} + + +
+ + {{imageWidth}}x{{imageHeight}} {{imageFilesize}} + +
+ {{/if}} diff --git a/app/assets/stylesheets/common/base/upload.scss b/app/assets/stylesheets/common/base/upload.scss index f7f3d8c64c6..d52ec8cf41a 100644 --- a/app/assets/stylesheets/common/base/upload.scss +++ b/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; diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 481d5d9c29b..a3af31cb8a9 100644 --- a/app/controllers/uploads_controller.rb +++ b/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 diff --git a/app/models/upload.rb b/app/models/upload.rb index 7f58ed20bb6..3cf5fb6b468 100644 --- a/app/models/upload.rb +++ b/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 = [] diff --git a/app/serializers/upload_serializer.rb b/app/serializers/upload_serializer.rb index 9e2098b8155..5aeee2261f2 100644 --- a/app/serializers/upload_serializer.rb +++ b/app/serializers/upload_serializer.rb @@ -9,5 +9,6 @@ class UploadSerializer < ApplicationSerializer :thumbnail_height, :extension, :short_url, - :retain_hours + :retain_hours, + :human_filesize end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index aedce12f3ce..5c23e09ff5a 100644 --- a/config/locales/client.en.yml +++ b/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" diff --git a/config/routes.rb b/config/routes.rb index cf38a68cd59..2ea0ce89e26 100644 --- a/config/routes.rb +++ b/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" diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 300e4817559..ca1ea035711 100644 --- a/lib/cooked_post_processor.rb +++ b/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) diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index 9128d8515ec..0e2ff0fd45b 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/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 diff --git a/test/javascripts/components/image-uploader-test.js.es6 b/test/javascripts/components/image-uploader-test.js.es6 index a2003094a89..9f61761b685 100644 --- a/test/javascripts/components/image-uploader-test.js.es6 +++ b/test/javascripts/components/image-uploader-test.js.es6 @@ -17,18 +17,12 @@ componentTest("with image", { "it displays the trash icon" ); - assert.equal( - this.$(".image-uploader-info").length, - 0, - "it does not display the image info" - ); - - await click(".image-uploader-info-btn"); + await click(".image-uploader-lightbox-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" ); } }); diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6 index 02f84cab4c2..9bf0cf85b95 100644 --- a/test/javascripts/helpers/create-pretender.js.es6 +++ b/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(