From ecce3c81f27447786533a383c2eabe8094cbc18c Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 21 Nov 2022 10:45:19 +0000 Subject: [PATCH] UX: Do not automatically refresh page while composer is open (#19112) We automatically refresh the page 'on the next navigation' whenever a new version of the JS client is available. If the composer is open when this happens then it will be closed and you'll have to reopen the draft. In some circumstances, this refresh can also cause some composer content to be lost. This commit updates the auto-refresh logic so that it doesn't trigger while the composer is open, and adds an acceptance test for the behaviour. --- .../javascripts/discourse/app/lib/url.js | 6 ++- .../tests/acceptance/assets-version-test.js | 51 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 app/assets/javascripts/discourse/tests/acceptance/assets-version-test.js diff --git a/app/assets/javascripts/discourse/app/lib/url.js b/app/assets/javascripts/discourse/app/lib/url.js index 622911c4173..0bd9a28efbf 100644 --- a/app/assets/javascripts/discourse/app/lib/url.js +++ b/app/assets/javascripts/discourse/app/lib/url.js @@ -196,7 +196,7 @@ const DiscourseURL = EmberObject.extend({ return; } - if (Session.currentProp("requiresRefresh")) { + if (Session.currentProp("requiresRefresh") && !this.isComposerOpen) { return this.redirectTo(path); } @@ -409,6 +409,10 @@ const DiscourseURL = EmberObject.extend({ return window.location.origin + (prefix === "/" ? "" : prefix); }, + get isComposerOpen() { + return this.controllerFor("composer")?.visible; + }, + get router() { return this.container.lookup("router:main"); }, diff --git a/app/assets/javascripts/discourse/tests/acceptance/assets-version-test.js b/app/assets/javascripts/discourse/tests/acceptance/assets-version-test.js new file mode 100644 index 00000000000..f20ac400492 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/acceptance/assets-version-test.js @@ -0,0 +1,51 @@ +import { + acceptance, + publishToMessageBus, +} from "discourse/tests/helpers/qunit-helpers"; +import { test } from "qunit"; +import { click, visit } from "@ember/test-helpers"; +import DiscourseURL from "discourse/lib/url"; +import Sinon from "sinon"; + +acceptance("Software update refresh", function (needs) { + needs.user(); + + test("Refreshes page on next navigation", async function (assert) { + const redirectStub = Sinon.stub(DiscourseURL, "redirectTo"); + + await visit("/"); + await click(".nav-item_top a"); + assert.true( + redirectStub.notCalled, + "redirect was not triggered by default" + ); + + await publishToMessageBus("/global/asset-version", "somenewversion"); + + redirectStub.resetHistory(); + await visit("/"); + await click(".nav-item_top a"); + assert.true( + redirectStub.calledWith("/top"), + "redirect was triggered after asset change" + ); + + redirectStub.resetHistory(); + await visit("/"); + await click("#create-topic"); + await click(".nav-item_top a"); + assert.true( + redirectStub.notCalled, + "redirect is not triggered while composer is open" + ); + + redirectStub.resetHistory(); + await visit("/"); + await click(".save-or-cancel .cancel"); + await click(".nav-item_top a"); + assert.true( + redirectStub.calledWith("/top"), + "redirect is triggered on next navigation after composer closed" + ); + }); +});