REFACTOR: Move Page title / focus / counts logic to service

We had a handful of methods attached to the root `Discourse` object
related to focus and notification counts.

This patch pulls them out into a service called `document-title` for
updating the title, and a component called `d-document` to attach
and listen for browser events related to focus.

It also removes some computed properties and observers in favor of plain
old Javascript objects.

app/assets/javascripts/discourse/app/app.js

@@ -2,8 +2,7 @@
 import Application from "@ember/application";
 import { computed } from "@ember/object";
 import { buildResolver } from "discourse-common/resolver";
-import { bind } from "@ember/runloop";
-import discourseComputed, { observes } from "discourse-common/utils/decorators";
+import discourseComputed from "discourse-common/utils/decorators";
 import { default as getURL, getURLWithCDN } from "discourse-common/lib/get-url";
 import deprecated from "discourse-common/lib/deprecated";
 
@@ -11,10 +10,7 @@ const _pluginCallbacks = [];
 
 const Discourse = Application.extend({
   rootElement: "#main",
-  _docTitle: document.title,
   __widget_helpers: {},
-  hasFocus: null,
-  _boundFocusChange: null,
 
   customEvents: {
     paste: "paste"
@@ -23,24 +19,6 @@ const Discourse = Application.extend({
   reset() {
     this._super(...arguments);
     Mousetrap.reset();
-
-    document.removeEventListener("visibilitychange", this._boundFocusChange);
-    document.removeEventListener("resume", this._boundFocusChange);
-    document.removeEventListener("freeze", this._boundFocusChange);
-
-    this._boundFocusChange = null;
-  },
-
-  ready() {
-    this._super(...arguments);
-    this._boundFocusChange = bind(this, this._focusChanged);
-
-    // Default to true
-    this.set("hasFocus", true);
-
-    document.addEventListener("visibilitychange", this._boundFocusChange);
-    document.addEventListener("resume", this._boundFocusChange);
-    document.addEventListener("freeze", this._boundFocusChange);
   },
 
   getURL(url) {
@@ -61,72 +39,6 @@ const Discourse = Application.extend({
 
   Resolver: buildResolver("discourse"),
 
-  @observes("_docTitle", "hasFocus", "contextCount", "notificationCount")
-  _titleChanged() {
-    let title = this._docTitle || this.SiteSettings.title;
-
-    let displayCount = this.displayCount;
-    let dynamicFavicon = this.currentUser && this.currentUser.dynamic_favicon;
-    if (displayCount > 0 && !dynamicFavicon) {
-      title = `(${displayCount}) ${title}`;
-    }
-
-    document.title = title;
-  },
-
-  @discourseComputed("contextCount", "notificationCount")
-  displayCount() {
-    return this.currentUser &&
-      this.currentUser.get("title_count_mode") === "notifications"
-      ? this.notificationCount
-      : this.contextCount;
-  },
-
-  @observes("contextCount", "notificationCount")
-  faviconChanged() {
-    if (this.currentUser && this.currentUser.get("dynamic_favicon")) {
-      let url = this.SiteSettings.site_favicon_url;
-
-      // Since the favicon is cached on the browser for a really long time, we
-      // append the favicon_url as query params to the path so that the cache
-      // is not used when the favicon changes.
-      if (/^http/.test(url)) {
-        url = getURL("/favicon/proxied?" + encodeURIComponent(url));
-      }
-
-      new window.Favcount(url).set(this.displayCount);
-    }
-  },
-
-  updateContextCount(count) {
-    this.set("contextCount", count);
-  },
-
-  updateNotificationCount(count) {
-    if (!this.hasFocus) {
-      this.set("notificationCount", count);
-    }
-  },
-
-  incrementBackgroundContextCount() {
-    if (!this.hasFocus) {
-      this.set("backgroundNotify", true);
-      this.set("contextCount", (this.contextCount || 0) + 1);
-    }
-  },
-
-  @observes("hasFocus")
-  resetCounts() {
-    if (this.hasFocus && this.backgroundNotify) {
-      this.set("contextCount", 0);
-    }
-    this.set("backgroundNotify", false);
-
-    if (this.hasFocus) {
-      this.set("notificationCount", 0);
-    }
-  },
-
   authenticationComplete(options) {
     // TODO, how to dispatch this to the controller without the container?
     const loginController = this.__container__.lookup("controller:login");
@@ -192,19 +104,7 @@ const Discourse = Application.extend({
       }
       return this.currentAssetVersion;
     }
-  }),
-
-  _focusChanged() {
-    if (document.visibilityState === "hidden") {
-      if (this.hasFocus) {
-        this.set("hasFocus", false);
-        this.appEvents.trigger("discourse:focus-changed", false);
-      }
-    } else if (!this.hasFocus) {
-      this.set("hasFocus", true);
-      this.appEvents.trigger("discourse:focus-changed", true);
-    }
-  }
+  })
 });
 
 export default Discourse;

app/assets/javascripts/discourse/app/components/d-document.js

@@ -0,0 +1,58 @@
+import Component from "@ember/component";
+import { bind } from "@ember/runloop";
+import { inject as service } from "@ember/service";
+
+export default Component.extend({
+  _boundFocusChange: null,
+  tagName: "",
+  documentTitle: service(),
+
+  didInsertElement() {
+    this._super(...arguments);
+
+    this.documentTitle.setTitle(document.title);
+    this._boundFocusChange = bind(this, this._focusChanged);
+    document.addEventListener("visibilitychange", this._boundFocusChange);
+    document.addEventListener("resume", this._boundFocusChange);
+    document.addEventListener("freeze", this._boundFocusChange);
+    this.session.hasFocus = true;
+
+    this.appEvents.on("notifications:changed", this, this._updateNotifications);
+  },
+
+  willDestroyElement() {
+    this._super(...arguments);
+
+    document.removeEventListener("visibilitychange", this._boundFocusChange);
+    document.removeEventListener("resume", this._boundFocusChange);
+    document.removeEventListener("freeze", this._boundFocusChange);
+    this._boundFocusChange = null;
+
+    this.appEvents.off(
+      "notifications:changed",
+      this,
+      this._updateNotifications
+    );
+  },
+
+  _updateNotifications() {
+    if (!this.currentUser) {
+      return;
+    }
+
+    this.documentTitle.updateNotificationCount(
+      this.currentUser.unread_notifications +
+        this.currentUser.unread_high_priority_notifications
+    );
+  },
+
+  _focusChanged() {
+    if (document.visibilityState === "hidden") {
+      if (this.session.hasFocus) {
+        this.documentTitle.setFocus(false);
+      }
+    } else if (!this.hasFocus) {
+      this.documentTitle.setFocus(true);
+    }
+  }
+});

app/assets/javascripts/discourse/app/components/discovery-topics-list.js

@@ -3,10 +3,12 @@ import Component from "@ember/component";
 import { on, observes } from "discourse-common/utils/decorators";
 import LoadMore from "discourse/mixins/load-more";
 import UrlRefresh from "discourse/mixins/url-refresh";
+import { inject as service } from "@ember/service";
 
 const DiscoveryTopicsListComponent = Component.extend(UrlRefresh, LoadMore, {
   classNames: ["contents"],
   eyelineSelector: ".topic-list-item",
+  documentTitle: service(),
 
   @on("didInsertElement")
   @observes("model")
@@ -26,7 +28,7 @@ const DiscoveryTopicsListComponent = Component.extend(UrlRefresh, LoadMore, {
 
   @observes("incomingCount")
   _updateTitle() {
-    Discourse.updateContextCount(this.incomingCount);
+    this.documentTitle.updateContextCount(this.incomingCount);
   },
 
   saveScrollPosition() {
@@ -35,7 +37,7 @@ const DiscoveryTopicsListComponent = Component.extend(UrlRefresh, LoadMore, {
 
   actions: {
     loadMore() {
-      Discourse.updateContextCount(0);
+      this.documentTitle.updateContextCount(0);
       this.model.loadMore().then(hasMoreResults => {
         schedule("afterRender", () => this.saveScrollPosition());
         if (hasMoreResults && $(window).height() >= $(document).height()) {

app/assets/javascripts/discourse/app/controllers/topic.js

@@ -24,6 +24,7 @@ import TopicTimer from "discourse/models/topic-timer";
 import { Promise } from "rsvp";
 import { escapeExpression } from "discourse/lib/utilities";
 import { AUTO_DELETE_PREFERENCES } from "discourse/models/bookmark";
+import { inject as service } from "@ember/service";
 
 let customPostMessageCallbacks = {};
 
@@ -59,6 +60,7 @@ export default Controller.extend(bufferedProperty("model"), {
   username_filters: null,
   filter: null,
   quoteState: null,
+  documentTitle: service(),
 
   canRemoveTopicFeaturedLink: and(
     "canEditTopicFeaturedLink",
@@ -1350,7 +1352,7 @@ export default Controller.extend(bufferedProperty("model"), {
           case "created": {
             postStream.triggerNewPostInStream(data.id).then(() => refresh());
             if (this.get("currentUser.id") !== data.user_id) {
-              Discourse.incrementBackgroundContextCount();
+              this.documentTitle.incrementBackgroundContextCount();
             }
             break;
           }

app/assets/javascripts/discourse/app/initializers/page-tracking.js

@@ -18,8 +18,9 @@ export default {
     router.on("routeDidChange", cleanDOM);
 
     let appEvents = container.lookup("service:app-events");
+    let documentTitle = container.lookup("service:document-title");
 
-    startPageTracking(router, appEvents);
+    startPageTracking(router, appEvents, documentTitle);
 
     // Out of the box, Discourse tries to track google analytics
     // if it is present

app/assets/javascripts/discourse/app/initializers/title-notifications.js

@@ -1,33 +0,0 @@
-export default {
-  name: "title-notifications",
-  after: "message-bus",
-
-  initialize(container) {
-    const user = container.lookup("current-user:main");
-    if (!user) return; // must be logged in
-
-    this.container = container;
-
-    container
-      .lookup("service:app-events")
-      .on("notifications:changed", this, "_updateTitle");
-  },
-
-  teardown(container) {
-    container
-      .lookup("service:app-events")
-      .off("notifications:changed", this, "_updateTitle");
-
-    this.container = null;
-  },
-
-  _updateTitle() {
-    const user = this.container.lookup("current-user:main");
-    if (!user) return; // must be logged in
-
-    const notifications =
-      user.unread_notifications + user.unread_high_priority_notifications;
-
-    Discourse.updateNotificationCount(notifications);
-  }
-};

app/assets/javascripts/discourse/app/lib/page-tracker.js

@@ -18,7 +18,7 @@ export function resetPageTracking() {
   cache = {};
 }
 
-export function startPageTracking(router, appEvents) {
+export function startPageTracking(router, appEvents, documentTitle) {
   if (_started) {
     return;
   }
@@ -34,11 +34,9 @@ export function startPageTracking(router, appEvents) {
     // Refreshing the title is debounced, so we need to trigger this in the
     // next runloop to have the correct title.
     next(() => {
-      let title = Discourse.get("_docTitle");
-
       appEvents.trigger("page:changed", {
         url,
-        title,
+        title: documentTitle.getTitle(),
         currentRouteName: router.currentRouteName,
         replacedOnlyQueryParams
       });

app/assets/javascripts/discourse/app/lib/screen-track.js

@@ -240,7 +240,7 @@ export default class {
       this.flush();
     }
 
-    if (Discourse.get("hasFocus")) {
+    if (this.session.hasFocus) {
       this._topicTime += diff;
 
       this._onscreen.forEach(

app/assets/javascripts/discourse/app/models/session.js

@@ -4,7 +4,9 @@ import Singleton from "discourse/mixins/singleton";
 // A data model representing current session data. You can put transient
 // data here you might want later. It is not stored or serialized anywhere.
 const Session = RestModel.extend({
-  init: function() {
+  hasFocus: null,
+
+  init() {
     this.set("highestSeenByTopic", {});
   }
 });

app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js

@@ -69,6 +69,7 @@ export default {
     const session = Session.current();
     app.register("session:main", session, { instantiate: false });
     ALL_TARGETS.forEach(t => app.inject(t, "session", "session:main"));
+    app.inject("service", "session", "session:main");
 
     const screenTrack = new ScreenTrack(
       topicTrackingState,

app/assets/javascripts/discourse/app/routes/application.js

@@ -13,6 +13,7 @@ import { findAll } from "discourse/models/login-method";
 import { getOwner } from "discourse-common/lib/get-owner";
 import { userPath } from "discourse/lib/url";
 import Composer from "discourse/models/composer";
+import { inject as service } from "@ember/service";
 
 function unlessReadOnly(method, message) {
   return function() {
@@ -27,6 +28,7 @@ function unlessReadOnly(method, message) {
 const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
   siteTitle: setting("title"),
   shortSiteDescription: setting("short_site_description"),
+  documentTitle: service(),
 
   actions: {
     toggleAnonymous() {
@@ -53,7 +55,7 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
       ) {
         tokens.push(this.shortSiteDescription);
       }
-      Discourse.set("_docTitle", tokens.join(" - "));
+      this.documentTitle.setTitle(tokens.join(" - "));
     },
 
     // Ember doesn't provider a router `willTransition` event so let's make one

app/assets/javascripts/discourse/app/services/document-title.js

@@ -0,0 +1,105 @@
+import Service from "@ember/service";
+import { inject as service } from "@ember/service";
+import getURL from "discourse-common/lib/get-url";
+
+export default Service.extend({
+  appEvents: service(),
+  contextCount: null,
+  notificationCount: null,
+  _title: null,
+  _backgroundNotify: null,
+
+  init() {
+    this._super(...arguments);
+    this.reset();
+  },
+
+  reset() {
+    this.contextCount = 0;
+    this.notificationCount = 0;
+    this._title = null;
+    this._backgroundNotify = null;
+  },
+
+  getTitle() {
+    return this._title;
+  },
+
+  setTitle(title) {
+    this._title = title;
+    this._renderTitle();
+  },
+
+  setFocus(focus) {
+    let { session } = this;
+
+    session.hasFocus = focus;
+
+    if (session.hasFocus && this._backgroundNotify) {
+      this.updateContextCount(0);
+    }
+    this._backgroundNotify = false;
+
+    if (session.hasFocus) {
+      this.notificationCount = 0;
+    }
+    this.appEvents.trigger("discourse:focus-changed", session.hasFocus);
+    this._renderTitle();
+  },
+
+  updateContextCount(count) {
+    this.contextCount = count;
+    this._renderTitle();
+  },
+
+  updateNotificationCount(count) {
+    if (!this.session.hasFocus) {
+      this.notificationCount = count;
+      this._renderFavicon();
+      this._renderTitle();
+    }
+  },
+
+  incrementBackgroundContextCount() {
+    if (!this.session.hasFocus) {
+      this._backgroundNotify = true;
+      this.contextCount += 1;
+      this._renderFavicon();
+      this._renderTitle();
+    }
+  },
+
+  _displayCount() {
+    return this.currentUser &&
+      this.currentUser.title_count_mode === "notifications"
+      ? this.notificationCount
+      : this.contextCount;
+  },
+
+  _renderTitle() {
+    let title = this._title || this.siteSettings.title;
+
+    let displayCount = this._displayCount();
+    let dynamicFavicon = this.currentUser && this.currentUser.dynamic_favicon;
+    if (displayCount > 0 && !dynamicFavicon) {
+      title = `(${displayCount}) ${title}`;
+    }
+
+    document.title = title;
+  },
+
+  _renderFavicon() {
+    if (this.currentUser && this.currentUser.dynamic_favicon) {
+      let url = this.siteSettings.site_favicon_url;
+
+      // Since the favicon is cached on the browser for a really long time, we
+      // append the favicon_url as query params to the path so that the cache
+      // is not used when the favicon changes.
+      if (/^http/.test(url)) {
+        url = getURL("/favicon/proxied?" + encodeURIComponent(url));
+      }
+
+      new window.Favcount(url).set(this._displayCount());
+    }
+  }
+});

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

@@ -1,3 +1,4 @@
+{{d-document}}
 {{plugin-outlet name="above-site-header"}}
 {{site-header canSignUp=canSignUp
               showCreateAccount=(route-action "showCreateAccount")

test/javascripts/controllers/topic-test.js

@@ -10,7 +10,8 @@ moduleFor("controller:topic", "controller:topic", {
   needs: [
     "controller:composer",
     "controller:application",
-    "service:app-events"
+    "service:app-events",
+    "service:document-title"
   ],
   beforeEach() {
     this.registry.injection("controller", "appEvents", "service:app-events");

test/javascripts/lib/discourse-test.js

@@ -1,64 +0,0 @@
-import { logIn, updateCurrentUser } from "helpers/qunit-helpers";
-
-QUnit.module("lib:discourse");
-
-QUnit.test("title counts are updated correctly", assert => {
-  Discourse.set("hasFocus", true);
-  Discourse.set("contextCount", 0);
-  Discourse.set("notificationCount", 0);
-
-  Discourse.set("_docTitle", "Test Title");
-
-  assert.equal(document.title, "Test Title", "title is correct");
-
-  Discourse.updateNotificationCount(5);
-  assert.equal(document.title, "Test Title", "title doesn't change with focus");
-
-  Discourse.incrementBackgroundContextCount();
-  assert.equal(document.title, "Test Title", "title doesn't change with focus");
-
-  Discourse.set("hasFocus", false);
-
-  Discourse.updateNotificationCount(5);
-  assert.equal(
-    document.title,
-    "Test Title",
-    "notification count ignored for anon"
-  );
-
-  Discourse.incrementBackgroundContextCount();
-  assert.equal(
-    document.title,
-    "(1) Test Title",
-    "title changes when incremented for anon"
-  );
-
-  logIn();
-  updateCurrentUser({ dynamic_favicon: false });
-
-  Discourse.set("hasFocus", true);
-  Discourse.set("hasFocus", false);
-
-  Discourse.incrementBackgroundContextCount();
-  assert.equal(
-    document.title,
-    "Test Title",
-    "title doesn't change when incremented for logged in"
-  );
-
-  Discourse.updateNotificationCount(3);
-  assert.equal(
-    document.title,
-    "(3) Test Title",
-    "title includes notification count for logged in user"
-  );
-
-  Discourse.set("hasFocus", false);
-  Discourse.set("hasFocus", true);
-
-  assert.equal(
-    document.title,
-    "Test Title",
-    "counter dissappears after focus, and doesn't reappear until another notification arrives"
-  );
-});

test/javascripts/lib/screen-track-test.js

@@ -1,76 +0,0 @@
-import TopicTrackingState from "discourse/models/topic-tracking-state";
-import Session from "discourse/models/session";
-import ScreenTrack from "discourse/lib/screen-track";
-import pretender from "helpers/create-pretender";
-
-let clock;
-
-QUnit.module("lib:screen-track", {
-  beforeEach() {
-    clock = sinon.useFakeTimers(new Date(2012, 11, 31, 12, 0).getTime());
-  },
-
-  afterEach() {
-    clock.restore();
-  }
-});
-
-// skip for now test leaks state
-QUnit.skip("Correctly flushes posts as needed", assert => {
-  const timings = [];
-
-  pretender.post("/topics/timings", t => {
-    timings.push(t);
-    return [200, {}, ""];
-  });
-
-  const trackingState = TopicTrackingState.create();
-  const siteSettings = {
-    flush_timings_secs: 60
-  };
-
-  const currentUser = { id: 1, username: "sam" };
-
-  const tracker = new ScreenTrack(
-    trackingState,
-    siteSettings,
-    Session.current(),
-    currentUser
-  );
-
-  const topicController = null;
-
-  Discourse.set("hasFocus", true);
-
-  tracker.reset();
-  tracker.start(1, topicController);
-
-  tracker.setOnscreen([1, 2, 3], [1, 2, 3]);
-
-  clock.tick(1050);
-  clock.tick(1050);
-
-  // no ajax yet
-  assert.equal(0, timings.length);
-
-  tracker.setOnscreen([1, 2, 3, 4], [1, 2, 3]);
-
-  clock.tick(1050);
-  clock.tick(1050);
-
-  // we should be rushed now cause there is a new thing on the screen
-  assert.equal(1, timings.length);
-
-  const req =
-    "timings%5B1%5D=3000&timings%5B2%5D=3000&timings%5B3%5D=3000&timings%5B4%5D=1000&topic_time=3000&topic_id=1";
-  assert.equal(timings[0].requestBody, req);
-
-  tracker.stop();
-
-  assert.equal(2, timings.length);
-
-  const req2 =
-    "timings%5B1%5D=1200&timings%5B2%5D=1200&timings%5B3%5D=1200&timings%5B4%5D=1200&topic_time=1200&topic_id=1";
-
-  assert.equal(timings[1].requestBody, req2);
-});

test/javascripts/services/document-title-test.js

@@ -0,0 +1,66 @@
+import { discourseModule } from "helpers/qunit-helpers";
+import { currentUser } from "helpers/qunit-helpers";
+
+discourseModule("service:document-title", {
+  beforeEach() {
+    this.documentTitle = this.container.lookup("service:document-title");
+    this.documentTitle.currentUser = null;
+    this.container.lookup("session:main").hasFocus = true;
+  },
+  afterEach() {
+    this.documentTitle.reset();
+  }
+});
+
+QUnit.test("it updates the document title", function(assert) {
+  this.documentTitle.setTitle("Test Title");
+  assert.equal(document.title, "Test Title", "title is correct");
+});
+
+QUnit.test(
+  "it doesn't display notification counts for anonymous users",
+  function(assert) {
+    this.documentTitle.setTitle("test notifications");
+    this.documentTitle.updateNotificationCount(5);
+    assert.equal(document.title, "test notifications");
+    this.documentTitle.setFocus(false);
+    this.documentTitle.updateNotificationCount(6);
+    assert.equal(document.title, "test notifications");
+  }
+);
+
+QUnit.test("it displays notification counts for logged in users", function(
+  assert
+) {
+  this.documentTitle.currentUser = currentUser();
+  this.documentTitle.currentUser.dynamic_favicon = false;
+  this.documentTitle.setTitle("test notifications");
+  this.documentTitle.updateNotificationCount(5);
+  assert.equal(document.title, "test notifications");
+  this.documentTitle.setFocus(false);
+  this.documentTitle.updateNotificationCount(6);
+  assert.equal(document.title, "(6) test notifications");
+  this.documentTitle.setFocus(true);
+  assert.equal(document.title, "test notifications");
+});
+
+QUnit.test(
+  "it doesn't increment background context counts when focused",
+  function(assert) {
+    this.documentTitle.setTitle("background context");
+    this.documentTitle.setFocus(true);
+    this.documentTitle.incrementBackgroundContextCount();
+    assert.equal(document.title, "background context");
+  }
+);
+
+QUnit.test("it increments background context counts when not focused", function(
+  assert
+) {
+  this.documentTitle.setTitle("background context");
+  this.documentTitle.setFocus(false);
+  this.documentTitle.incrementBackgroundContextCount();
+  assert.equal(document.title, "(1) background context");
+  this.documentTitle.setFocus(true);
+  assert.equal(document.title, "background context");
+});