From bf3260faea09899ad1ddcf9ea1d7be3dc65e842d Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 6 Apr 2022 12:48:13 +1000 Subject: [PATCH] DEV: Use pick-files-button in composer-editor and clean up (#16375) A while ago in 27b97e4 the pick-files-input was added but only used once for data-explorer. This commit uses it for the composer-editor, and cleans it up to be usable either via uppy handling the uploads or with this component handling the uploads. This can then be used in other places in the app and also for plugins. --- .../app/components/composer-editor.js | 24 +---- .../app/components/pick-files-button.js | 94 ++++++++++--------- .../templates/components/composer-editor.hbs | 6 +- .../components/pick-files-button.hbs | 12 ++- .../components/pick-files-button-tests.js | 92 ------------------ 5 files changed, 60 insertions(+), 168 deletions(-) delete mode 100644 app/assets/javascripts/discourse/tests/integration/components/pick-files-button-tests.js diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index 71312232091..f836b7ff784 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -1,8 +1,4 @@ -import { - authorizedExtensions, - authorizesAllExtensions, - authorizesOneOrMoreImageExtensions, -} from "discourse/lib/uploads"; +import { authorizesOneOrMoreImageExtensions } from "discourse/lib/uploads"; import { alias } from "@ember/object/computed"; import { BasePlugin } from "@uppy/core"; import { resolveAllShortUrls } from "pretty-text/upload-short-url"; @@ -201,24 +197,6 @@ export default Component.extend(ComposerUploadUppy, { }); }, - @discourseComputed() - acceptsAllFormats() { - return ( - this.capabilities.isIOS || - authorizesAllExtensions(this.currentUser.staff, this.siteSettings) - ); - }, - - @discourseComputed() - acceptedFormats() { - const extensions = authorizedExtensions( - this.currentUser.staff, - this.siteSettings - ); - - return extensions.map((ext) => `.${ext}`).join(); - }, - @bind _afterMentionComplete(value) { this.composer.set("reply", value); diff --git a/app/assets/javascripts/discourse/app/components/pick-files-button.js b/app/assets/javascripts/discourse/app/components/pick-files-button.js index 9ab6465bcd8..56aae7ed42c 100644 --- a/app/assets/javascripts/discourse/app/components/pick-files-button.js +++ b/app/assets/javascripts/discourse/app/components/pick-files-button.js @@ -1,25 +1,46 @@ import Component from "@ember/component"; -import { action } from "@ember/object"; -import { empty } from "@ember/object/computed"; -import computed, { bind } from "discourse-common/utils/decorators"; -import I18n from "I18n"; import bootbox from "bootbox"; +import { isBlank } from "@ember/utils"; +import { + authorizedExtensions, + authorizesAllExtensions, +} from "discourse/lib/uploads"; +import { action } from "@ember/object"; +import discourseComputed, { bind } from "discourse-common/utils/decorators"; +import I18n from "I18n"; +// This picker is intended to be used with UppyUploadMixin or with +// ComposerUploadUppy, which is why there are no change events registered +// for the input. They are handled by the uppy mixins directly. +// +// However, if you provide an onFilesPicked action to this component, the change +// binding will still be added, and the file type will be validated here. This +// is sometimes useful if you need to do something outside the uppy upload with +// the file, such as directly using JSON or CSV data from a file in JS. export default Component.extend({ + fileInputId: null, + fileInputClass: null, classNames: ["pick-files-button"], - acceptedFileTypes: null, - acceptAnyFile: empty("acceptedFileTypes"), + acceptedFormatsOverride: null, + allowMultiple: false, + showButton: false, didInsertElement() { this._super(...arguments); - const fileInput = this.element.querySelector("input"); - this.set("fileInput", fileInput); - fileInput.addEventListener("change", this.onChange, false); + + if (this.onFilesPicked) { + const fileInput = this.element.querySelector("input"); + this.set("fileInput", fileInput); + fileInput.addEventListener("change", this.onChange, false); + } }, willDestroyElement() { this._super(...arguments); - this.fileInput.removeEventListener("change", this.onChange); + + if (this.onFilesPicked) { + this.fileInput.removeEventListener("change", this.onChange); + } }, @bind @@ -28,33 +49,27 @@ export default Component.extend({ this._filesPicked(files); }, - @computed - acceptedFileTypesString() { - if (!this.acceptedFileTypes) { - return null; - } - - return this.acceptedFileTypes.join(","); + @discourseComputed() + acceptsAllFormats() { + return ( + this.capabilities.isIOS || + authorizesAllExtensions(this.currentUser.staff, this.siteSettings) + ); }, - @computed - acceptedExtensions() { - if (!this.acceptedFileTypes) { - return null; + @discourseComputed() + acceptedFormats() { + // the acceptedFormatsOverride can be a list of extensions or mime types + if (!isBlank(this.acceptedFormatsOverride)) { + return this.acceptedFormatsOverride; } - return this.acceptedFileTypes - .filter((type) => type.startsWith(".")) - .map((type) => type.substring(1)); - }, + const extensions = authorizedExtensions( + this.currentUser.staff, + this.siteSettings + ); - @computed - acceptedMimeTypes() { - if (!this.acceptedFileTypes) { - return null; - } - - return this.acceptedFileTypes.filter((type) => !type.startsWith(".")); + return extensions.map((ext) => `.${ext}`).join(); }, @action @@ -79,25 +94,18 @@ export default Component.extend({ _haveAcceptedTypes(files) { for (const file of files) { - if ( - !(this._hasAcceptedExtension(file) || this._hasAcceptedMimeType(file)) - ) { + if (!this._hasAcceptedExtensionOrType(file)) { return false; } } return true; }, - _hasAcceptedExtension(file) { + _hasAcceptedExtensionOrType(file) { const extension = this._fileExtension(file.name); return ( - !this.acceptedExtensions || this.acceptedExtensions.includes(extension) - ); - }, - - _hasAcceptedMimeType(file) { - return ( - !this.acceptedMimeTypes || this.acceptedMimeTypes.includes(file.type) + this.acceptedFormats.includes(`.${extension}`) || + this.acceptedFormats.includes(file.type) ); }, diff --git a/app/assets/javascripts/discourse/app/templates/components/composer-editor.hbs b/app/assets/javascripts/discourse/app/templates/components/composer-editor.hbs index 77aac380bcb..767b0a854ae 100644 --- a/app/assets/javascripts/discourse/app/templates/components/composer-editor.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/composer-editor.hbs @@ -23,9 +23,5 @@ {{/d-editor}} {{#if allowUpload}} - {{#if acceptsAllFormats}} - - {{else}} - - {{/if}} + {{pick-files-button fileInputId="file-uploader" allowMultiple=true}} {{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/components/pick-files-button.hbs b/app/assets/javascripts/discourse/app/templates/components/pick-files-button.hbs index 409a3becbd7..dc6bc33f6ab 100644 --- a/app/assets/javascripts/discourse/app/templates/components/pick-files-button.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/pick-files-button.hbs @@ -1,6 +1,8 @@ -{{d-button action=(action "openSystemFilePicker") label=label icon=icon}} -{{#if acceptAnyFile}} - -{{else}} - +{{#if showButton}} + {{d-button action=(action "openSystemFilePicker") label=label icon=icon}} +{{/if}} +{{#if acceptsAllFormats}} + +{{else}} + {{/if}} diff --git a/app/assets/javascripts/discourse/tests/integration/components/pick-files-button-tests.js b/app/assets/javascripts/discourse/tests/integration/components/pick-files-button-tests.js deleted file mode 100644 index db622c99b43..00000000000 --- a/app/assets/javascripts/discourse/tests/integration/components/pick-files-button-tests.js +++ /dev/null @@ -1,92 +0,0 @@ -import componentTest, { - setupRenderingTest, -} from "discourse/tests/helpers/component-test"; -import { discourseModule } from "discourse/tests/helpers/qunit-helpers"; -import hbs from "htmlbars-inline-precompile"; -import { triggerEvent } from "@ember/test-helpers"; -import sinon from "sinon"; -import bootbox from "bootbox"; - -function createBlob(mimeType, extension) { - const blob = new Blob(["content"], { - type: mimeType, - }); - blob.name = `filename${extension}`; - return blob; -} - -discourseModule( - "Integration | Component | pick-files-button", - function (hooks) { - const expectedExtension = ".json"; - const expectedMimeType = "text/json"; - - setupRenderingTest(hooks); - - hooks.beforeEach(function () { - this.set("acceptedFileTypes", [expectedExtension, expectedMimeType]); - this.set("onFilesPicked", () => {}); - }); - - componentTest("it doesn't show alert if a file has a supported MIME type", { - skip: true, - template: hbs` - {{pick-files-button - acceptedFileTypes=this.acceptedFileTypes - onFilesPicked=this.onFilesPicked}}`, - - async test(assert) { - sinon.stub(bootbox, "alert"); - - const wrongExtension = ".txt"; - const file = createBlob(expectedMimeType, wrongExtension); - - await triggerEvent("input[type='file']", "change", { files: [file] }); - - assert.ok(bootbox.alert.notCalled); - }, - }); - - componentTest("it doesn't show alert if a file has a supported extension", { - skip: true, - template: hbs` - {{pick-files-button - acceptedFileTypes=this.acceptedFileTypes - onFilesPicked=this.onFilesPicked}}`, - - async test(assert) { - sinon.stub(bootbox, "alert"); - - const wrongMimeType = "text/plain"; - const file = createBlob(wrongMimeType, expectedExtension); - - await triggerEvent("input[type='file']", "change", { files: [file] }); - - assert.ok(bootbox.alert.notCalled); - }, - }); - - componentTest( - "it shows alert if a file has an unsupported extension and unsupported MIME type", - { - skip: true, - template: hbs` - {{pick-files-button - acceptedFileTypes=this.acceptedFileTypes - onFilesPicked=this.onFilesPicked}}`, - - async test(assert) { - sinon.stub(bootbox, "alert"); - - const wrongExtension = ".txt"; - const wrongMimeType = "text/plain"; - const file = createBlob(wrongMimeType, wrongExtension); - - await triggerEvent("input[type='file']", "change", { files: [file] }); - - assert.ok(bootbox.alert.calledOnce); - }, - } - ); - } -);