diff --git a/app/assets/javascripts/discourse/app/app.js b/app/assets/javascripts/discourse/app/app.js index 836ad694fdd..4a17b540a73 100644 --- a/app/assets/javascripts/discourse/app/app.js +++ b/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; diff --git a/app/assets/javascripts/discourse/app/components/d-document.js b/app/assets/javascripts/discourse/app/components/d-document.js new file mode 100644 index 00000000000..ec022e856f8 --- /dev/null +++ b/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); + } + } +}); diff --git a/app/assets/javascripts/discourse/app/components/discovery-topics-list.js b/app/assets/javascripts/discourse/app/components/discovery-topics-list.js index 28ab1efab59..f84761fb697 100644 --- a/app/assets/javascripts/discourse/app/components/discovery-topics-list.js +++ b/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()) { diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index 49467d5a30e..93077ca6ff4 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/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; } diff --git a/app/assets/javascripts/discourse/app/initializers/page-tracking.js b/app/assets/javascripts/discourse/app/initializers/page-tracking.js index 71fa6f25aea..df34d9f0676 100644 --- a/app/assets/javascripts/discourse/app/initializers/page-tracking.js +++ b/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 diff --git a/app/assets/javascripts/discourse/app/initializers/title-notifications.js b/app/assets/javascripts/discourse/app/initializers/title-notifications.js deleted file mode 100644 index c7ae1d5f06b..00000000000 --- a/app/assets/javascripts/discourse/app/initializers/title-notifications.js +++ /dev/null @@ -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); - } -}; diff --git a/app/assets/javascripts/discourse/app/lib/page-tracker.js b/app/assets/javascripts/discourse/app/lib/page-tracker.js index 3246f14ee81..f0e58900665 100644 --- a/app/assets/javascripts/discourse/app/lib/page-tracker.js +++ b/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 }); diff --git a/app/assets/javascripts/discourse/app/lib/screen-track.js b/app/assets/javascripts/discourse/app/lib/screen-track.js index ccfd9d8b3fe..e3a8a8b3c7e 100644 --- a/app/assets/javascripts/discourse/app/lib/screen-track.js +++ b/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( diff --git a/app/assets/javascripts/discourse/app/models/session.js b/app/assets/javascripts/discourse/app/models/session.js index 517cf769cb7..1ded733a429 100644 --- a/app/assets/javascripts/discourse/app/models/session.js +++ b/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", {}); } }); diff --git a/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js b/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js index fe6c7b638cb..98d59a811d2 100644 --- a/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js +++ b/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, diff --git a/app/assets/javascripts/discourse/app/routes/application.js b/app/assets/javascripts/discourse/app/routes/application.js index cd9c6dd4a0b..9cdf53b47c7 100644 --- a/app/assets/javascripts/discourse/app/routes/application.js +++ b/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 diff --git a/app/assets/javascripts/discourse/app/services/document-title.js b/app/assets/javascripts/discourse/app/services/document-title.js new file mode 100644 index 00000000000..1cccb468d87 --- /dev/null +++ b/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()); + } + } +}); diff --git a/app/assets/javascripts/discourse/app/templates/application.hbs b/app/assets/javascripts/discourse/app/templates/application.hbs index aeb8c91232a..f7dfdd9a89c 100644 --- a/app/assets/javascripts/discourse/app/templates/application.hbs +++ b/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") diff --git a/test/javascripts/controllers/topic-test.js b/test/javascripts/controllers/topic-test.js index 411d70d38be..08796ef7d26 100644 --- a/test/javascripts/controllers/topic-test.js +++ b/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"); diff --git a/test/javascripts/lib/discourse-test.js b/test/javascripts/lib/discourse-test.js deleted file mode 100644 index 4b2395c68ea..00000000000 --- a/test/javascripts/lib/discourse-test.js +++ /dev/null @@ -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" - ); -}); diff --git a/test/javascripts/lib/screen-track-test.js b/test/javascripts/lib/screen-track-test.js deleted file mode 100644 index 9574e972a73..00000000000 --- a/test/javascripts/lib/screen-track-test.js +++ /dev/null @@ -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); -}); diff --git a/test/javascripts/services/document-title-test.js b/test/javascripts/services/document-title-test.js new file mode 100644 index 00000000000..ec031f336f9 --- /dev/null +++ b/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"); +});