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.

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")
+  router: service(),
-  currentPathChanged() {
+
+  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: {

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 = [

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({});
-export default Controller.extend({
-  router: service(),
-});

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",

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({});
-export default Controller.extend({
-  router: service(),
-});

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"),

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")

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"),

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")

app/assets/javascripts/discourse/app/templates/components/group-navigation.hbs

@@ -1,4 +1,4 @@
-{{#mobile-nav class="group-nav" desktopClass="nav nav-pills" currentPath=currentPath}}
+{{#mobile-nav class="group-nav" desktopClass="nav nav-pills"}}
   {{#if site.mobileView}}
     <li>
       {{#link-to "groups.index"}}

app/assets/javascripts/discourse/app/templates/group/activity.hbs

@@ -1,5 +1,5 @@
 <section class="user-secondary-navigation">
-  {{#mobile-nav class="activity-nav" desktopClass="action-list activity-list nav-stacked" currentPath=router._router.currentPath}}
+  {{#mobile-nav class="activity-nav" desktopClass="action-list activity-list nav-stacked"}}
     {{#if model.can_see_members}}
       {{group-activity-filter filter="posts" categoryId=category_id}}
       {{group-activity-filter filter="topics" categoryId=category_id}}

app/assets/javascripts/discourse/app/templates/group/manage.hbs

@@ -1,5 +1,5 @@
 <section class="user-secondary-navigation">
-  {{#mobile-nav class="activity-nav" desktopClass="action-list activity-list nav-stacked" currentPath=router._router.currentPath}}
+  {{#mobile-nav class="activity-nav" desktopClass="action-list activity-list nav-stacked"}}
     {{#each tabs as |tab|}}
       <li>
         {{#link-to tab.route model.name}}

app/assets/javascripts/discourse/app/templates/group/messages.hbs

@@ -1,5 +1,5 @@
 <section class="user-secondary-navigation">
-  {{#mobile-nav class="messages-nav" desktopClass="nav-stacked action-list" currentPath=router._router.currentPath}}
+  {{#mobile-nav class="messages-nav" desktopClass="nav-stacked action-list"}}
     <li>
       {{#link-to "group.messages.inbox" model.name}}
         {{i18n "user.messages.inbox"}}

app/assets/javascripts/discourse/app/templates/preferences.hbs

@@ -1,5 +1,5 @@
 {{#d-section pageClass="user-preferences" class="user-secondary-navigation"}}
-  {{#mobile-nav class="preferences-nav" desktopClass="preferences-list action-list nav-stacked" currentPath=router._router.currentPath}}
+  {{#mobile-nav class="preferences-nav" desktopClass="preferences-list action-list nav-stacked"}}
     <li class="nav-account">
       {{#link-to "preferences.account"}}
         {{i18n "user.preferences_nav.account"}}

app/assets/javascripts/discourse/app/templates/user.hbs

@@ -225,7 +225,7 @@
     </section>
     <div class='user-content-wrapper'>
       <section class="user-primary-navigation">
-        {{#mobile-nav class='main-nav' desktopClass="nav nav-pills user-nav" currentPath=currentPath}}
+        {{#mobile-nav class='main-nav' desktopClass="nav nav-pills user-nav"}}
           {{#unless model.profile_hidden}}
             <li class="summary">{{#link-to 'user.summary'}}{{i18n 'user.summary.title'}}{{/link-to}}</li>
             <li class="activity">{{#link-to 'userActivity'}}{{i18n 'user.activity_stream'}}{{/link-to}}</li>

app/assets/javascripts/discourse/app/templates/user/activity.hbs

@@ -1,6 +1,6 @@
 {{#d-section pageClass="user-activity" class="user-secondary-navigation" scrollTop="false"}}
   <nav role="navigation">
-    {{#mobile-nav class="activity-nav" desktopClass="action-list activity-list nav-stacked" currentPath=router._router.currentPath}}
+    {{#mobile-nav class="activity-nav" desktopClass="action-list activity-list nav-stacked"}}
       {{#d-navigation-item route="userActivity.index"}}{{i18n "user.filters.all"}}{{/d-navigation-item}}
       {{#d-navigation-item route="userActivity.topics"}}{{i18n "user_action_groups.4"}}{{/d-navigation-item}}
       {{#d-navigation-item route="userActivity.replies"}}{{i18n "user_action_groups.5"}}{{/d-navigation-item}}

app/assets/javascripts/discourse/app/templates/user/messages.hbs

@@ -5,7 +5,7 @@
     {{/if}}
   {{/unless}}
 
-  {{#mobile-nav class="messages-nav" desktopClass="nav-stacked action-list" currentPath=currentPath}}
+  {{#mobile-nav class="messages-nav" desktopClass="nav-stacked action-list"}}
     <li class="noGlyph">
       {{#link-to "userPrivateMessages.index" model}}
         {{i18n "user.messages.inbox"}}

app/assets/javascripts/discourse/app/templates/user/notifications.hbs

@@ -1,5 +1,5 @@
 {{#d-section pageClass="user-notifications" class="user-secondary-navigation"}}
-  {{#mobile-nav class="notifications-nav" desktopClass="notification-list action-list nav-stacked" currentPath=currentPath}}
+  {{#mobile-nav class="notifications-nav" desktopClass="notification-list action-list nav-stacked"}}
     <li>
       {{#link-to "userNotifications.index"}}
         {{i18n "user.filters.all"}}

plugins/styleguide/assets/javascripts/discourse/templates/styleguide/molecules/navigation-bar.hbs

@@ -3,7 +3,7 @@
 {{/styleguide-example}}
 
 {{#styleguide-example title=".user-main .nav-pills"}}
-  {{#mobile-nav class="main-nav" desktopClass="nav nav-pills user-nav" currentPath=currentPath}}
+  {{#mobile-nav class="main-nav" desktopClass="nav nav-pills user-nav"}}
     {{#each dummy.navItems as |ni|}}
       <li><a href={{ni.href}} class={{if ni.styleGuideActive "active"}}>{{ni.displayName}}</a></li>
     {{/each}}

plugins/styleguide/assets/javascripts/discourse/templates/styleguide/molecules/navigation-stacked.hbs

@@ -1,5 +1,5 @@
 {{#styleguide-example title=".nav-stacked" class="half-size"}}
-  {{#mobile-nav class="preferences-nav" desktopClass="preferences-list action-list nav-stacked" currentPath=application.currentPath}}
+  {{#mobile-nav class="preferences-nav" desktopClass="preferences-list action-list nav-stacked"}}
     {{#each dummy.navItems as |ni|}}
       <li><a href={{ni.href}} class={{if ni.styleGuideActive "active"}}>{{ni.displayName}}</a></li>
     {{/each}}
@@ -8,7 +8,7 @@
 
 {{#styleguide-example title=".user-navigation .nav-stacked" class="half-size"}}
   {{#d-section class="user-navigation"}}
-    {{#mobile-nav class="preferences-nav" desktopClass="preferences-list action-list nav-stacked" currentPath=application.currentPath}}
+    {{#mobile-nav class="preferences-nav" desktopClass="preferences-list action-list nav-stacked"}}
       {{#each dummy.navItems as |ni|}}
         <li><a href={{ni.href}} class={{if ni.styleGuideActive "active"}}>{{ni.displayName}}</a></li>
       {{/each}}