REFACTOR: Move Page title / focus / counts logic to service (PR #10347)

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.

GitHub

So much cleaner :heart_eyes:

what is the reason for this happening before super ? Feels like it is so rare, it should be documented if on purpose

Oops! Not intentional. I’ve pushed a fix.

We don’t need the .get here, or you could change the whole line:

this.get("currentUser.title_count_mode") === "notifications"

does it pass linting ? I thought eslint wouldn’t accept to use new without assignment.

That’s a nitpick here but could me try to have the same naming pattern:

setTitle setFocus setContextCount setNotificationCount

(or update instead of set)

getTitle()? setTitle()? What is this, java? :stuck_out_tongue_winking_eye:

That looks like a native getters/setters opportunity. :wink:

  get title() {
    return this._title;
  },

  set title(title) {
    this._title = title;
    this._renderTitle();
  },

(Possibly the same with with updateFocus and updateContextCount below)

Agreed, I feel like we will have to draw some line in the next months, we don’t have any history/patterns on this so far

There is one! (since a year ago!) discourse/app/assets/javascripts/discourse/app/services/emoji-store.js at 8a9e4504fea9edd693ceecf314308cd08e99f0ca · discourse/discourse · GitHub :sweat_smile:

1 Like