FIX: Allow mobile-nav to work without loading transitions (#12184)

FIX: Allow mobile-nav to work without loading transitions (#12184)

Previously, the {{mobile-nav}} component required a currentRouteName property, passed from the router service. It would observe changes in this property, and update the UI accordingly.

If we change between routes which have the same currentRouteName (e.g. two different group message inboxes), then the currentRouteName does not change and does not trigger the observer. Currently in core, we are relying on the fact that currentRouteName temporarily enters a .loading substate during a transition. This will change when we remove the loading substate in the near future.

This commit refactors {{mobile-nav}} to inject the router directly, and use the routeDidChange event instead of an observer. The change is backwards compatible, but plugins passing the old currentPath property will be shown a deprecation notice.

diff --git a/app/assets/javascripts/discourse/app/components/mobile-nav.js b/app/assets/javascripts/discourse/app/components/mobile-nav.js
index db61688..8648020 100644
--- a/app/assets/javascripts/discourse/app/components/mobile-nav.js
+++ b/app/assets/javascripts/discourse/app/components/mobile-nav.js
@@ -1,6 +1,8 @@
-import { observes, on } from "discourse-common/utils/decorators";
+import { on } from "discourse-common/utils/decorators";
 import Component from "@ember/component";
 import { next } from "@ember/runloop";
+import { inject as service } from "@ember/service";
+import deprecated from "discourse-common/lib/deprecated";
 
 export default Component.extend({
   @on("init")
@@ -12,6 +14,11 @@ export default Component.extend({
         this.set("classNames", classes);
       }
     }
+    if (this.currentPath) {
+      deprecated("{{mobile-nav}} no longer requires the currentPath property", {
+        since: "2.7.0.beta4",
+      });
+    }
   },
 
   tagName: "ul",
@@ -19,13 +26,18 @@ export default Component.extend({
 
   classNames: ["mobile-nav"],
 
-  @observes("currentPath")
-  currentPathChanged() {
+  router: service(),
+
+  currentRouteChanged() {
     this.set("expanded", false);
     next(() => this._updateSelectedHtml());
   },
 
   _updateSelectedHtml() {
+    if (!this.element || this.isDestroying || this.isDestroyed) {
+      return;
+    }
+
     const active = this.element.querySelector(".active");
     if (active && active.innerHTML) {
       this.set("selectedHtml", active.innerHTML);
@@ -36,6 +48,11 @@ export default Component.extend({
     this._super(...arguments);
 
     this._updateSelectedHtml();
+    this.router.on("routeDidChange", this, this.currentRouteChanged);
+  },
+
+  willDestroyElement() {
+    this.router.off("routeDidChange", this, this.currentRouteChanged);
   },
 
   actions: {
diff --git a/app/assets/javascripts/discourse/app/controllers/group-manage.js b/app/assets/javascripts/discourse/app/controllers/group-manage.js
index 28b3da6..de7cea1 100644
--- a/app/assets/javascripts/discourse/app/controllers/group-manage.js
+++ b/app/assets/javascripts/discourse/app/controllers/group-manage.js
@@ -1,10 +1,7 @@
 import Controller from "@ember/controller";
 import discourseComputed from "discourse-common/utils/decorators";
-import { inject as service } from "@ember/service";
 
 export default Controller.extend({
-  router: service(),
-
   @discourseComputed("model.automatic")
   tabs(automatic) {
     const defaultTabs = [
diff --git a/app/assets/javascripts/discourse/app/controllers/group-messages.js b/app/assets/javascripts/discourse/app/controllers/group-messages.js
index cb0c46f..1c0ee08 100644
--- a/app/assets/javascripts/discourse/app/controllers/group-messages.js
+++ b/app/assets/javascripts/discourse/app/controllers/group-messages.js
@@ -1,5 +1,2 @@
 import Controller from "@ember/controller";
-import { inject as service } from "@ember/service";
-export default Controller.extend({
-  router: service(),
-});
+export default Controller.extend({});
diff --git a/app/assets/javascripts/discourse/app/controllers/group.js b/app/assets/javascripts/discourse/app/controllers/group.js
index 3d10fc9..8327c52 100644
--- a/app/assets/javascripts/discourse/app/controllers/group.js
+++ b/app/assets/javascripts/discourse/app/controllers/group.js
@@ -4,8 +4,6 @@ import I18n from "I18n";
 import bootbox from "bootbox";
 import deprecated from "discourse-common/lib/deprecated";
 import discourseComputed from "discourse-common/utils/decorators";
-import { readOnly } from "@ember/object/computed";
-import { inject as service } from "@ember/service";
 
 const Tab = EmberObject.extend({
   init() {
@@ -23,8 +21,6 @@ export default Controller.extend({
   counts: null,
   showing: "members",
   destroying: null,
-  router: service(),
-  currentPath: readOnly("router.currentRouteName"),
 
   @discourseComputed(
     "showMessages",
diff --git a/app/assets/javascripts/discourse/app/controllers/preferences.js b/app/assets/javascripts/discourse/app/controllers/preferences.js
index cb0c46f..1c0ee08 100644
--- a/app/assets/javascripts/discourse/app/controllers/preferences.js
+++ b/app/assets/javascripts/discourse/app/controllers/preferences.js
@@ -1,5 +1,2 @@
 import Controller from "@ember/controller";
-import { inject as service } from "@ember/service";
-export default Controller.extend({
-  router: service(),
-});
+export default Controller.extend({});
diff --git a/app/assets/javascripts/discourse/app/controllers/user-activity.js b/app/assets/javascripts/discourse/app/controllers/user-activity.js
index f2023c2..c62f5bc 100644
--- a/app/assets/javascripts/discourse/app/controllers/user-activity.js
+++ b/app/assets/javascripts/discourse/app/controllers/user-activity.js
@@ -4,12 +4,10 @@ import { alias } from "@ember/object/computed";
 import bootbox from "bootbox";
 import { exportUserArchive } from "discourse/lib/export-csv";
 import { observes } from "discourse-common/utils/decorators";
-import { inject as service } from "@ember/service";
 
 export default Controller.extend({
   application: controller(),
   user: controller(),
-  router: service(),
   userActionType: null,
 
   canDownloadPosts: alias("user.viewingSelf"),
diff --git a/app/assets/javascripts/discourse/app/controllers/user-notifications.js b/app/assets/javascripts/discourse/app/controllers/user-notifications.js
index 369e6f6..3f2b3a4 100644
--- a/app/assets/javascripts/discourse/app/controllers/user-notifications.js
+++ b/app/assets/javascripts/discourse/app/controllers/user-notifications.js
@@ -1,14 +1,10 @@
 import Controller, { inject as controller } from "@ember/controller";
 import discourseComputed, { observes } from "discourse-common/utils/decorators";
 import { ajax } from "discourse/lib/ajax";
-import { readOnly } from "@ember/object/computed";
-import { inject as service } from "@ember/service";
 
 export default Controller.extend({
   application: controller(),
   queryParams: ["filter"],
-  router: service(),
-  currentPath: readOnly("router._router.currentPath"),
   filter: "all",
 
   @observes("model.canLoadMore")
diff --git a/app/assets/javascripts/discourse/app/controllers/user-private-messages.js b/app/assets/javascripts/discourse/app/controllers/user-private-messages.js
index e149557..1288ce2 100644
--- a/app/assets/javascripts/discourse/app/controllers/user-private-messages.js
+++ b/app/assets/javascripts/discourse/app/controllers/user-private-messages.js
@@ -4,12 +4,10 @@ import I18n from "I18n";
 import Topic from "discourse/models/topic";
 import bootbox from "bootbox";
 import discourseComputed from "discourse-common/utils/decorators";
-import { inject as service } from "@ember/service";
 
 export default Controller.extend({
   userTopicsList: controller("user-topics-list"),
   user: controller(),
-  router: service(),
 
   pmView: false,
   viewingSelf: alias("user.viewingSelf"),
diff --git a/app/assets/javascripts/discourse/app/controllers/user.js b/app/assets/javascripts/discourse/app/controllers/user.js
index 7199054..c8c93be 100644
--- a/app/assets/javascripts/discourse/app/controllers/user.js
+++ b/app/assets/javascripts/discourse/app/controllers/user.js
@@ -1,6 +1,6 @@
 import Controller, { inject } from "@ember/controller";
 import EmberObject, { computed, set } from "@ember/object";
-import { alias, and, equal, gt, not, or } from "@ember/object/computed";
+import { and, equal, gt, not, or } from "@ember/object/computed";
 import CanCheckEmails from "discourse/mixins/can-check-emails";
 import User from "discourse/models/user";
 import I18n from "I18n";
@@ -16,7 +16,6 @@ import { inject as service } from "@ember/service";
 export default Controller.extend(CanCheckEmails, {
   router: service(),
   userNotifications: inject("user-notifications"),
-  currentPath: alias("router._router.currentPath"),
   adminTools: optionalService(),
 
   @discourseComputed("model.username")

[... diff too long, it was truncated ...]

GitHub sha: 1844bde5

1 Like

This commit appears in #12184 which was approved by jjaffeux. It was merged by davidtaylorhq.