From 9e426d33c73448106abd49259ae4893e9e74fca5 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev <a.prigorshnev@gmail.com> Date: Tue, 8 Jun 2021 13:22:49 +0400 Subject: [PATCH] FEATURE: Don't show the draft checkmark when drafts are saved (#13292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't want to show the draft checkmark in the composer when drafts are saved, as it’s a little bit distracting to see it keeps appearing and disappearing. Only in the case of error does it need to show anything, we will be showing a "drafts offline" warning as we did it before. An important detail is that the warning was appearing and disappearing all the time too. Now, the warning won’t be flashing while a user is typing, it’ll be disappearing only when the draft was eventually saved. --- .../discourse/app/models/composer.js | 28 +--------- .../discourse/app/templates/composer.hbs | 4 +- .../acceptance/composer-draft-saving-test.js | 53 +++++++++++++++++++ .../tests/acceptance/composer-test.js | 2 - .../stylesheets/common/base/compose.scss | 5 -- 5 files changed, 56 insertions(+), 36 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/acceptance/composer-draft-saving-test.js diff --git a/app/assets/javascripts/discourse/app/models/composer.js b/app/assets/javascripts/discourse/app/models/composer.js index 5fee7738421..5b8f6c68154 100644 --- a/app/assets/javascripts/discourse/app/models/composer.js +++ b/app/assets/javascripts/discourse/app/models/composer.js @@ -1,6 +1,6 @@ import EmberObject, { set } from "@ember/object"; import { and, equal, not, or, reads } from "@ember/object/computed"; -import { cancel, later, next, throttle } from "@ember/runloop"; +import { next, throttle } from "@ember/runloop"; import discourseComputed, { observes, on, @@ -113,7 +113,6 @@ const Composer = RestModel.extend({ unlistTopic: false, noBump: false, draftSaving: false, - draftSaved: false, draftForceSave: false, archetypes: reads("site.archetypes"), @@ -1190,16 +1189,10 @@ const Composer = RestModel.extend({ } this.setProperties({ - draftSaved: false, draftSaving: true, draftConflictUser: null, }); - if (this._clearingStatus) { - cancel(this._clearingStatus); - this._clearingStatus = null; - } - let data = this.serialize(_draft_serializer); if (data.postId && !isEmpty(this.originalText)) { @@ -1224,7 +1217,7 @@ const Composer = RestModel.extend({ }); } else { this.setProperties({ - draftSaved: true, + draftStatus: null, draftConflictUser: null, draftForceSave: false, }); @@ -1276,23 +1269,6 @@ const Composer = RestModel.extend({ this.set("draftSaving", false); }); }, - - @observes("title", "reply") - dataChanged() { - const draftStatus = this.draftStatus; - - if (draftStatus && !this._clearingStatus) { - this._clearingStatus = later( - this, - () => { - this.setProperties({ draftStatus: null, draftConflictUser: null }); - this._clearingStatus = null; - this.setProperties({ draftSaving: false, draftSaved: false }); - }, - Ember.Test ? 0 : 1000 - ); - } - }, }); Composer.reopenClass({ diff --git a/app/assets/javascripts/discourse/app/templates/composer.hbs b/app/assets/javascripts/discourse/app/templates/composer.hbs index 955df36efe8..7779c0e1aef 100644 --- a/app/assets/javascripts/discourse/app/templates/composer.hbs +++ b/app/assets/javascripts/discourse/app/templates/composer.hbs @@ -175,14 +175,12 @@ </div> {{/if}} <div class={{if isUploading "hidden"}} id="draft-status"> - {{#if model.draftSaving}}<div class="spinner small"></div>{{/if}} - {{#if model.draftSaved}}{{d-icon "check" class="save-animation"}}{{/if}} {{#if model.draftStatus}} <span title={{model.draftStatus}}> {{#if model.draftConflictUser}} {{avatar model.draftConflictUser imageSize="small"}} {{d-icon "user-edit"}} {{else}} - {{d-icon "sync-alt"}} + {{d-icon "exclamation-triangle"}} {{/if}} {{#unless site.mobileView}} {{model.draftStatus}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-draft-saving-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-draft-saving-test.js new file mode 100644 index 00000000000..44f6b4d87f9 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-draft-saving-test.js @@ -0,0 +1,53 @@ +import { + acceptance, + exists, + query, +} from "discourse/tests/helpers/qunit-helpers"; +import { click, fillIn, visit } from "@ember/test-helpers"; +import { test } from "qunit"; +import I18n from "I18n"; + +acceptance("Composer - Draft saving", function (needs) { + needs.user(); + + const draftThatWillBeSaved = "This_will_be_saved_successfully"; + + needs.pretender((server, helper) => { + server.post("/draft.json", (request) => { + const success = request.requestBody.includes(draftThatWillBeSaved); + return success + ? helper.response({ success: true }) + : helper.response(500, {}); + }); + }); + + test("Shows a warning if a draft wasn't saved", async function (assert) { + await visit("/t/internationalization-localization/280"); + await click(".topic-post:nth-of-type(1) button.show-more-actions"); + await click(".topic-post:nth-of-type(1) button.edit"); + + await fillIn(".d-editor-input", draftThatWillBeSaved); + + assert.notOk( + exists("div#draft-status span"), + "the draft was saved, there's no warning" + ); + + await fillIn(".d-editor-input", "This won't be saved because of error"); + assert.equal( + query("div#draft-status span").innerText.trim(), + I18n.t("composer.drafts_offline"), + "the draft wasn't saved, a warning is rendered" + ); + assert.ok( + exists("div#draft-status svg.d-icon-exclamation-triangle"), + "an exclamation icon is rendered" + ); + + await fillIn(".d-editor-input", draftThatWillBeSaved); + assert.notOk( + exists("div#draft-status span"), + "the draft was saved again, the warning has disappeared" + ); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js index cd0bb88cf25..5440a5aa2ce 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js @@ -698,8 +698,6 @@ acceptance("Composer", function (needs) { ), "mode should have changed" ); - - assert.ok(queryAll(".save-animation"), "save animation should show"); } finally { toggleCheckDraftPopup(false); } diff --git a/app/assets/stylesheets/common/base/compose.scss b/app/assets/stylesheets/common/base/compose.scss index a37e875194a..67e7165b61b 100644 --- a/app/assets/stylesheets/common/base/compose.scss +++ b/app/assets/stylesheets/common/base/compose.scss @@ -482,11 +482,6 @@ div.ac-wrap { } } -.save-animation { - -webkit-animation: transformer 5s forwards; - animation: transformer 5s forwards; -} - @-webkit-keyframes transformer { 90% { -webkit-filter: opacity(1);