From fa66d1fa82d6c768f265d3c3601110dca672a205 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 2 Sep 2021 17:31:15 +1000 Subject: [PATCH] FIX: Make bindMobileUploadButton explicit for upload mixins (#14220) When using ComposerUpload and/or ComposerUploadUppy, we were always calling bindMobileUploadButton. However with more composer-like interfaces being developed, we need this to be optional, as not everywhere will have a separate mobile upload button to bind to. Also makes it so the composer extending the ComposerUpload mixins is responsible for explicitly unbinding the mobile upload button if it needs to. --- .../discourse/app/components/composer-editor.js | 3 +++ .../discourse/app/mixins/composer-upload-uppy.js | 6 ------ .../javascripts/discourse/app/mixins/composer-upload.js | 8 ++++---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index c319408bbc3..eb2d0798538 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -219,6 +219,8 @@ export default Component.extend(ComposerUpload, { } this._bindUploadTarget(); + this._bindMobileUploadButton(); + this.appEvents.trigger("composer:will-open"); }, @@ -607,6 +609,7 @@ export default Component.extend(ComposerUpload, { @on("willDestroyElement") _composerClosed() { + this._unbindMobileUploadButton(); this.appEvents.trigger("composer:will-close"); next(() => { // need to wait a bit for the "slide down" transition of the composer diff --git a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js index 885c86b3f9b..3636c058690 100644 --- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js +++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js @@ -46,11 +46,6 @@ export default Mixin.create({ @on("willDestroyElement") _unbindUploadTarget() { - this.mobileUploadButton?.removeEventListener( - "click", - this.mobileUploadButtonEventListener - ); - this.fileInputEl?.removeEventListener( "change", this.fileInputEventListener @@ -86,7 +81,6 @@ export default Mixin.create({ this._unbindUploadTarget(); this._bindFileInputChangeListener(); this._bindPasteListener(); - this._bindMobileUploadButton(); this._uppyInstance = new Uppy({ id: this.uppyId, diff --git a/app/assets/javascripts/discourse/app/mixins/composer-upload.js b/app/assets/javascripts/discourse/app/mixins/composer-upload.js index c3ada1ccc0b..01ab335ddaf 100644 --- a/app/assets/javascripts/discourse/app/mixins/composer-upload.js +++ b/app/assets/javascripts/discourse/app/mixins/composer-upload.js @@ -327,8 +327,6 @@ export default Mixin.create({ } }); }); - - this._bindMobileUploadButton(); }, _bindMobileUploadButton() { @@ -345,13 +343,15 @@ export default Mixin.create({ } }, - @on("willDestroyElement") - _unbindUploadTarget() { + _unbindMobileUploadButton() { this.mobileUploadButton?.removeEventListener( "click", this.mobileUploadButtonEventListener ); + }, + @on("willDestroyElement") + _unbindUploadTarget() { this._validUploads = 0; const $uploadTarget = $(this.element); try {