From d470e4fade93fbc37f13b6b6d3e60f5f5108472d Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Fri, 19 Mar 2021 09:19:15 -0400 Subject: [PATCH] FEATURE: Allow users to save draft and close composer (#12439) We previously included this option conditionally when users were replying or creating a new topic while they had content already in the composer. This makes the dialog always include three buttons: - Close and discard - Close and save draft for later - Keed editing This also changes how the backend notifies the frontend when there is a current draft topic. This is now sent via the `has_topic_draft` property in the current user serializer. --- .../discourse/app/components/d-modal-body.js | 3 +- .../discourse/app/components/d-modal.js | 5 ++ .../discourse/app/controllers/composer.js | 50 +++++++++---------- .../app/controllers/discard-draft.js | 33 +++--------- .../app/controllers/navigation/categories.js | 9 ---- .../app/controllers/navigation/default.js | 7 --- .../discourse/app/controllers/tag-show.js | 5 -- .../discourse/app/routes/discourse.js | 36 +++++-------- .../app/routes/discovery-categories.js | 5 +- .../discourse/app/routes/discovery.js | 5 +- .../discourse/app/routes/tag-show.js | 7 ++- .../app/templates/components/d-modal.hbs | 5 +- .../app/templates/modal/discard-draft.hbs | 9 ++-- .../app/templates/navigation/categories.hbs | 2 +- .../app/templates/navigation/category.hbs | 2 +- .../app/templates/navigation/default.hbs | 2 +- .../discourse/app/templates/tags/show.hbs | 2 +- .../tests/acceptance/composer-test.js | 44 ++++++++++++---- app/assets/stylesheets/common/base/modal.scss | 6 ++- app/models/draft.rb | 6 +++ app/serializers/current_user_serializer.rb | 9 ++++ config/locales/client.en.yml | 23 +++------ .../current_user_serializer_spec.rb | 28 +++++++++++ 23 files changed, 155 insertions(+), 148 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/d-modal-body.js b/app/assets/javascripts/discourse/app/components/d-modal-body.js index ec5385958b8..fada62686d1 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal-body.js +++ b/app/assets/javascripts/discourse/app/components/d-modal-body.js @@ -56,7 +56,8 @@ export default Component.extend({ "fixed", "subtitle", "rawSubtitle", - "dismissable" + "dismissable", + "headerClass" ) ); }, diff --git a/app/assets/javascripts/discourse/app/components/d-modal.js b/app/assets/javascripts/discourse/app/components/d-modal.js index e74f12d566b..c94633fe26f 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal.js +++ b/app/assets/javascripts/discourse/app/components/d-modal.js @@ -23,6 +23,7 @@ export default Component.extend({ title: null, subtitle: null, role: "dialog", + headerClass: null, init() { this._super(...arguments); @@ -129,6 +130,10 @@ export default Component.extend({ this.set("dismissable", true); } + if (data.headerClass) { + this.set("headerClass", data.headerClass); + } + if (this.element) { const autofocusInputs = this.element.querySelectorAll( ".modal-body input[autofocus]" diff --git a/app/assets/javascripts/discourse/app/controllers/composer.js b/app/assets/javascripts/discourse/app/controllers/composer.js index cd260683d62..53fffb5bb14 100644 --- a/app/assets/javascripts/discourse/app/controllers/composer.js +++ b/app/assets/javascripts/discourse/app/controllers/composer.js @@ -18,6 +18,7 @@ import discourseComputed, { import DiscourseURL from "discourse/lib/url"; import Draft from "discourse/models/draft"; import I18n from "I18n"; +import { iconHTML } from "discourse-common/lib/icon-library"; import { Promise } from "rsvp"; import bootbox from "bootbox"; import { buildQuote } from "discourse/lib/quote"; @@ -545,9 +546,7 @@ export default Controller.extend({ }, cancel() { - const differentDraftContext = - this.get("topic.id") !== this.get("model.topic.id"); - this.cancelComposer(differentDraftContext); + this.cancelComposer(); }, save(ignore, event) { @@ -903,13 +902,7 @@ export default Controller.extend({ } } - // If it's a different draft, cancel it and try opening again. - const differentDraftContext = - opts.post && composerModel.topic - ? composerModel.topic.id !== opts.post.topic_id - : true; - - return this.cancelComposer(differentDraftContext) + return this.cancelComposer() .then(() => this.open(opts)) .then(resolve, reject); } @@ -1037,18 +1030,19 @@ export default Controller.extend({ return false; }, - destroyDraft() { + destroyDraft(draftSequence = null) { const key = this.get("model.draftKey"); if (key) { - if (key === "new_topic") { - this.send("clearTopicDraft"); + if (key === Composer.NEW_TOPIC_KEY) { + this.currentUser.set("has_topic_draft", false); } if (this._saveDraftPromise) { return this._saveDraftPromise.then(() => this.destroyDraft()); } - return Draft.clear(key, this.get("model.draftSequence")).then(() => + const sequence = draftSequence || this.get("model.draftSequence"); + return Draft.clear(key, sequence).then(() => this.appEvents.trigger("draft:destroyed", key) ); } else { @@ -1078,9 +1072,12 @@ export default Controller.extend({ { label: I18n.t("drafts.abandon.yes_value"), class: "btn-danger", + icon: iconHTML("far-trash-alt"), callback: () => { - data.draft = null; - resolve(data); + this.destroyDraft(data.draft_sequence).finally(() => { + data.draft = null; + resolve(data); + }); }, }, ]); @@ -1091,22 +1088,20 @@ export default Controller.extend({ } }, - cancelComposer(differentDraft = false) { + cancelComposer() { this.skipAutoSave = true; if (this._saveDraftDebounce) { cancel(this._saveDraftDebounce); } - let promise = new Promise((resolve, reject) => { + let promise = new Promise((resolve) => { if (this.get("model.hasMetaData") || this.get("model.replyDirty")) { const controller = showModal("discard-draft", { model: this.model, modalClass: "discard-draft-modal", - title: "post.abandon.title", }); controller.setProperties({ - differentDraft, onDestroyDraft: () => { this.destroyDraft() .then(() => { @@ -1118,15 +1113,16 @@ export default Controller.extend({ }); }, onSaveDraft: () => { - // cancel composer without destroying draft on new draft context - if (differentDraft) { - this.model.clearState(); - this.close(); - resolve(); + this._saveDraft(); + if (this.model.draftKey === Composer.NEW_TOPIC_KEY) { + this.currentUser.set("has_topic_draft", true); } - - reject(); + this.model.clearState(); + this.close(); + resolve(); }, + // needed to resume saving drafts if composer stays open + onDismissModal: () => resolve(), }); } else { // it is possible there is some sort of crazy draft with no body ... just give up on it diff --git a/app/assets/javascripts/discourse/app/controllers/discard-draft.js b/app/assets/javascripts/discourse/app/controllers/discard-draft.js index 32633aec29d..eecd880f546 100644 --- a/app/assets/javascripts/discourse/app/controllers/discard-draft.js +++ b/app/assets/javascripts/discourse/app/controllers/discard-draft.js @@ -1,40 +1,19 @@ import Controller from "@ember/controller"; import ModalFunctionality from "discourse/mixins/modal-functionality"; -import discourseComputed from "discourse-common/utils/decorators"; export default Controller.extend(ModalFunctionality, { - differentDraft: null, - - @discourseComputed() - keyPrefix() { - return this.model.action === "edit" ? "post.abandon_edit" : "post.abandon"; - }, - - @discourseComputed("keyPrefix") - descriptionKey(keyPrefix) { - return `${keyPrefix}.confirm`; - }, - - @discourseComputed("keyPrefix") - discardKey(keyPrefix) { - return `${keyPrefix}.yes_value`; - }, - - @discourseComputed("keyPrefix", "differentDraft") - saveKey(keyPrefix, differentDraft) { - return differentDraft - ? `${keyPrefix}.no_save_draft` - : `${keyPrefix}.no_value`; - }, - actions: { - _destroyDraft() { + destroyDraft() { this.onDestroyDraft(); this.send("closeModal"); }, - _saveDraft() { + saveDraftAndClose() { this.onSaveDraft(); this.send("closeModal"); }, + dismissModal() { + this.onDismissModal(); + this.send("closeModal"); + }, }, }); diff --git a/app/assets/javascripts/discourse/app/controllers/navigation/categories.js b/app/assets/javascripts/discourse/app/controllers/navigation/categories.js index a1be94b13b4..a36e1d837cc 100644 --- a/app/assets/javascripts/discourse/app/controllers/navigation/categories.js +++ b/app/assets/javascripts/discourse/app/controllers/navigation/categories.js @@ -1,15 +1,6 @@ import NavigationDefaultController from "discourse/controllers/navigation/default"; -import discourseComputed from "discourse-common/utils/decorators"; import { inject } from "@ember/controller"; export default NavigationDefaultController.extend({ discoveryCategories: inject("discovery/categories"), - - @discourseComputed( - "discoveryCategories.model", - "discoveryCategories.model.draft" - ) - draft() { - return this.get("discoveryCategories.model.draft"); - }, }); diff --git a/app/assets/javascripts/discourse/app/controllers/navigation/default.js b/app/assets/javascripts/discourse/app/controllers/navigation/default.js index 28a3530650a..2e6fd002a2f 100644 --- a/app/assets/javascripts/discourse/app/controllers/navigation/default.js +++ b/app/assets/javascripts/discourse/app/controllers/navigation/default.js @@ -1,13 +1,6 @@ import Controller, { inject as controller } from "@ember/controller"; import FilterModeMixin from "discourse/mixins/filter-mode"; -import discourseComputed from "discourse-common/utils/decorators"; export default Controller.extend(FilterModeMixin, { discovery: controller(), - discoveryTopics: controller("discovery/topics"), - - @discourseComputed("discoveryTopics.model", "discoveryTopics.model.draft") - draft: function () { - return this.get("discoveryTopics.model.draft"); - }, }); diff --git a/app/assets/javascripts/discourse/app/controllers/tag-show.js b/app/assets/javascripts/discourse/app/controllers/tag-show.js index bde05d5718b..f3544c5c6be 100644 --- a/app/assets/javascripts/discourse/app/controllers/tag-show.js +++ b/app/assets/javascripts/discourse/app/controllers/tag-show.js @@ -29,11 +29,6 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, { q: null, showInfo: false, - @discourseComputed("list", "list.draft") - createTopicLabel(list, listDraft) { - return listDraft ? "topic.open_draft" : "topic.create"; - }, - @discourseComputed( "canCreateTopic", "category", diff --git a/app/assets/javascripts/discourse/app/routes/discourse.js b/app/assets/javascripts/discourse/app/routes/discourse.js index 8ce296fc63c..3625b6ef52c 100644 --- a/app/assets/javascripts/discourse/app/routes/discourse.js +++ b/app/assets/javascripts/discourse/app/routes/discourse.js @@ -1,4 +1,5 @@ import Composer from "discourse/models/composer"; +import Draft from "discourse/models/draft"; import Route from "@ember/routing/route"; import { once } from "@ember/runloop"; import { seenUser } from "discourse/lib/user-presence"; @@ -42,23 +43,6 @@ const DiscourseRoute = Route.extend({ refreshTitle() { once(this, this._refreshTitleOnce); }, - - clearTopicDraft() { - // perhaps re-delegate this to root controller in all cases? - // TODO also poison the store so it does not come back from the - // dead - if (this.get("controller.list.draft")) { - this.set("controller.list.draft", null); - } - - if (this.controllerFor("discovery/categories").get("model.draft")) { - this.controllerFor("discovery/categories").set("model.draft", null); - } - - if (this.controllerFor("discovery/topics").get("model.draft")) { - this.controllerFor("discovery/topics").set("model.draft", null); - } - }, }, redirectIfLoginRequired() { @@ -68,20 +52,24 @@ const DiscourseRoute = Route.extend({ } }, - openTopicDraft(model) { + openTopicDraft() { const composer = this.controllerFor("composer"); if ( composer.get("model.action") === Composer.CREATE_TOPIC && - composer.get("model.draftKey") === model.draft_key + composer.get("model.draftKey") === Composer.NEW_TOPIC_KEY ) { composer.set("model.composeState", Composer.OPEN); } else { - composer.open({ - action: Composer.CREATE_TOPIC, - draft: model.draft, - draftKey: model.draft_key, - draftSequence: model.draft_sequence, + Draft.get(Composer.NEW_TOPIC_KEY).then((data) => { + if (data.draft) { + composer.open({ + action: Composer.CREATE_TOPIC, + draft: data.draft, + draftKey: Composer.NEW_TOPIC_KEY, + draftSequence: data.draft_sequence, + }); + } }); } }, diff --git a/app/assets/javascripts/discourse/app/routes/discovery-categories.js b/app/assets/javascripts/discourse/app/routes/discovery-categories.js index b496f99e7ef..eeb402c3bf1 100644 --- a/app/assets/javascripts/discourse/app/routes/discovery-categories.js +++ b/app/assets/javascripts/discourse/app/routes/discovery-categories.js @@ -118,9 +118,8 @@ const DiscoveryCategoriesRoute = DiscourseRoute.extend(OpenComposer, { }, createTopic() { - const model = this.controllerFor("discovery/categories").get("model"); - if (model.draft) { - this.openTopicDraft(model); + if (this.get("currentUser.has_topic_draft")) { + this.openTopicDraft(); } else { this.openComposer(this.controllerFor("discovery/categories")); } diff --git a/app/assets/javascripts/discourse/app/routes/discovery.js b/app/assets/javascripts/discourse/app/routes/discovery.js index 0a983301fa9..8c69cf10f25 100644 --- a/app/assets/javascripts/discourse/app/routes/discovery.js +++ b/app/assets/javascripts/discourse/app/routes/discovery.js @@ -59,9 +59,8 @@ export default DiscourseRoute.extend(OpenComposer, { }, createTopic() { - const model = this.controllerFor("discovery/topics").get("model"); - if (model.draft) { - this.openTopicDraft(model); + if (this.get("currentUser.has_topic_draft")) { + this.openTopicDraft(); } else { this.openComposer(this.controllerFor("discovery/topics")); } diff --git a/app/assets/javascripts/discourse/app/routes/tag-show.js b/app/assets/javascripts/discourse/app/routes/tag-show.js index bc07ea23e3d..6ffe1e582f4 100644 --- a/app/assets/javascripts/discourse/app/routes/tag-show.js +++ b/app/assets/javascripts/discourse/app/routes/tag-show.js @@ -180,11 +180,10 @@ export default DiscourseRoute.extend(FilterModeMixin, { }, createTopic() { - const controller = this.controllerFor("tag.show"); - - if (controller.get("list.draft")) { - this.openTopicDraft(controller.get("list")); + if (this.get("currentUser.has_topic_draft")) { + this.openTopicDraft(); } else { + const controller = this.controllerFor("tag.show"); const composerController = this.controllerFor("composer"); composerController .open({ diff --git a/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs b/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs index dc314151b37..6b57bd1c214 100644 --- a/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs @@ -1,7 +1,7 @@