FIX: Abort transition when `transition.from` present on new-topic route (#22253)

Why is this change required?

The `/new-topic` route is a special route which we use to open the
composer by loading a URL. By default, the `new-topic` route is replaced with the
`discovery.latest` route. On a fresh page load, this makes sense since
there is no template for the `new-topic` route to render. However, this
behavior does not make sense if we're transition from another route.
There is no need to replace the current route with the `discovery.latest` when all we want
is to open the composer.

What does this commit do?

This commit fixes the undesirable behaviour described above by aborting
the existing transition to the `new-topic` route if `transition.from` is
present. This indicates that we're navigating from an existing route and
we can just open the composer.
This commit is contained in:
Alan Guo Xiang Tan 2023-06-27 08:03:51 +08:00 committed by GitHub
parent 343f09a152
commit df04a99db9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 49 additions and 25 deletions

View File

@ -117,14 +117,15 @@ export default Component.extend(KeyEnterEscape, {
@observes("composeState", "composer.{action,canEditTopicFeaturedLink}")
_triggerComposerResized() {
schedule("afterRender", () => {
if (!this.element || this.isDestroying || this.isDestroyed) {
return;
}
discourseDebounce(this, this.composerResized, 300);
});
},
composerResized() {
if (!this.element || this.isDestroying || this.isDestroyed) {
return;
}
this.appEvents.trigger("composer:resized");
},

View File

@ -43,11 +43,16 @@ export default DiscourseRoute.extend({
}
});
} else {
this.replaceWith("discovery.latest").then((e) => {
if (this.controllerFor("navigation/default").canCreateTopic) {
this._sendTransition(e, transition);
}
});
if (transition.from) {
transition.abort();
this.send("createNewTopicViaParams");
} else {
this.replaceWith("discovery.latest").then((e) => {
if (this.controllerFor("navigation/default").canCreateTopic) {
this._sendTransition(e, transition);
}
});
}
}
} else {
// User is not logged in

View File

@ -1,11 +1,7 @@
import {
acceptance,
exists,
query,
} from "discourse/tests/helpers/qunit-helpers";
import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers";
import selectKit from "discourse/tests/helpers/select-kit-helper";
import { test } from "qunit";
import { visit } from "@ember/test-helpers";
import { currentURL, visit, waitFor } from "@ember/test-helpers";
acceptance("New Topic - Anonymous", function () {
test("accessing new-topic route when logged out", async function (assert) {
@ -17,26 +13,48 @@ acceptance("New Topic - Anonymous", function () {
acceptance("New Topic - Authenticated", function (needs) {
needs.user();
test("accessing new-topic route when logged in", async function (assert) {
test("accessing new-topic route", async function (assert) {
await visit("/c/1");
try {
await visit("/new-topic");
} catch (error) {
assert.strictEqual(
error.message,
"TransitionAborted",
"it aborts the transition"
);
}
assert.strictEqual(currentURL(), "/c/1");
await waitFor(".composer-fields", { timeout: 5000 });
assert.dom(".composer-fields").exists("it opens the composer");
});
test("accessing new-topic route with title, body and category param", async function (assert) {
await visit(
"/new-topic?title=topic%20title&body=topic%20body&category=bug"
);
assert.ok(exists(".composer-fields"), "it opens composer");
assert.strictEqual(
query("#reply-title").value.trim(),
"topic title",
"it pre-fills topic title"
);
assert.strictEqual(
query(".d-editor-input").value.trim(),
"topic body",
"it pre-fills topic body"
);
assert
.dom("#reply-title")
.hasValue("topic title", "it pre-fills the topic title");
assert
.dom(".d-editor-input")
.hasValue("topic body", "it pre-fills topic body");
assert.strictEqual(
selectKit(".category-chooser").header().value(),
"1",
"it selects desired category"
);
assert.strictEqual(currentURL(), "/c/1");
});
});