From de82b5eb702b6549778c03bce1ec181581a46f34 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 19 Apr 2017 16:29:03 +0100 Subject: [PATCH] closes https://github.com/TryGhost/Ghost/issues/8307 - unloading the store and refreshing the `session.user` attribute after an import was triggering a rendering edge case where the style was re-computed and a re-render was attempted after the sidebar has been destroyed - rather than binding a style attribute directly to a CP in `gh-nav-menu` we pass the menu icon in (using `settings.settledIcon` - see below) and manually set the style attribute via the `didReceiveAttrs` hook so that outside changes don't trigger re-computations when we don't expect them and so we can still react to icons being uploaded or removed - our usage of `settings.icon` is a bit of an odd situation because it's a link to an external resource that will only resolve correctly after a successful save - if we change `settings.icon` in the local store and the nav menu icon style updates before the save has been completed then the server will give us the old icon. To work around this a `settings.settledIcon` attribute has been added that is only updated when we receive data from the store ensuring that our cache-busting technique works correctly --- app/components/gh-nav-menu.js | 56 +++++++++++++++--------- app/controllers/application.js | 1 + app/controllers/settings/labs.js | 6 ++- app/services/settings.js | 11 ++++- app/templates/application.hbs | 8 +++- app/templates/components/gh-nav-menu.hbs | 2 +- 6 files changed, 58 insertions(+), 26 deletions(-) diff --git a/app/components/gh-nav-menu.js b/app/components/gh-nav-menu.js index 8c1f967c16..40d6a626f8 100644 --- a/app/components/gh-nav-menu.js +++ b/app/components/gh-nav-menu.js @@ -1,35 +1,29 @@ import Component from 'ember-component'; -import {htmlSafe} from 'ember-string'; import injectService from 'ember-service/inject'; -import computed from 'ember-computed'; +import {htmlSafe} from 'ember-string'; import calculatePosition from 'ember-basic-dropdown/utils/calculate-position'; export default Component.extend({ + config: injectService(), + session: injectService(), + ghostPaths: injectService(), + feature: injectService(), + routing: injectService('-routing'), + tagName: 'nav', classNames: ['gh-nav'], classNameBindings: ['open'], open: false, + iconStyle: '', - navMenuIcon: computed('config.blogUrl', 'settings.icon', 'ghostPaths.subdir', 'ghostPaths.url', function () { - let subdirRegExp = new RegExp(`^${this.get('ghostPaths.subdir')}`); - let blogIcon = this.get('settings.icon') ? this.get('settings.icon') : 'favicon.ico'; - let url; - - blogIcon = blogIcon.replace(subdirRegExp, ''); - - url = this.get('ghostPaths.url').join(this.get('config.blogUrl'), blogIcon).replace(/\/$/, ''); - url += `?t=${(new Date()).valueOf()}`; - - return htmlSafe(`background-image: url(${url})`); - }), - - config: injectService(), - settings: injectService(), - session: injectService(), - ghostPaths: injectService(), - feature: injectService(), - routing: injectService('-routing'), + // the menu has a rendering issue (#8307) when the the world is reloaded + // during an import which we have worked around by not binding the icon + // style directly. However we still need to keep track of changing icons + // so that we can refresh when a new icon is uploaded + didReceiveAttrs() { + this._setIconStyle(); + }, mouseEnter() { this.sendAction('onMouseEnter'); @@ -46,6 +40,26 @@ export default Component.extend({ return {horizontalPosition, verticalPosition, style}; }, + _setIconStyle() { + let icon = this.get('icon'); + + if (icon === this._icon) { + return; + } + + let subdirRegExp = new RegExp(`^${this.get('ghostPaths.subdir')}`); + let blogIcon = icon ? icon : 'favicon.ico'; + let iconUrl; + + blogIcon = blogIcon.replace(subdirRegExp, ''); + + iconUrl = this.get('ghostPaths.url').join(this.get('config.blogUrl'), blogIcon).replace(/\/$/, ''); + iconUrl += `?t=${(new Date()).valueOf()}`; + + this.set('iconStyle', htmlSafe(`background-image: url(${iconUrl})`)); + this._icon = icon; + }, + actions: { toggleAutoNav() { this.sendAction('toggleMaximise'); diff --git a/app/controllers/application.js b/app/controllers/application.js index 197a78f687..50f6105104 100644 --- a/app/controllers/application.js +++ b/app/controllers/application.js @@ -5,6 +5,7 @@ import injectService from 'ember-service/inject'; export default Controller.extend({ dropdown: injectService(), session: injectService(), + settings: injectService(), showNavMenu: computed('currentPath', 'session.isAuthenticated', 'session.user.isFulfilled', function () { // we need to defer showing the navigation menu until the session.user diff --git a/app/controllers/settings/labs.js b/app/controllers/settings/labs.js index 915dee4412..8449f9eb1b 100644 --- a/app/controllers/settings/labs.js +++ b/app/controllers/settings/labs.js @@ -98,10 +98,12 @@ export default Controller.extend({ processData: false }); }).then(() => { + let store = this.get('store'); + // Clear the store, so that all the new data gets fetched correctly. - this.store.unloadAll(); + store.unloadAll(); // Reload currentUser and set session - this.set('session.user', this.store.findRecord('user', currentUserId)); + this.set('session.user', store.findRecord('user', currentUserId)); // TODO: keep as notification, add link to view content notifications.showNotification('Import successful.', {key: 'import.upload.success'}); }).catch((response) => { diff --git a/app/services/settings.js b/app/services/settings.js index 0785e55abe..f6e4614254 100644 --- a/app/services/settings.js +++ b/app/services/settings.js @@ -3,6 +3,7 @@ import Service from 'ember-service'; import injectService from 'ember-service/inject'; import RSVP from 'rsvp'; import ValidationEngine from 'ghost-admin/mixins/validation-engine'; +import get from 'ember-metal/get'; // ember-cli-shims doesn't export _ProxyMixin const {_ProxyMixin} = Ember; @@ -17,6 +18,10 @@ export default Service.extend(_ProxyMixin, ValidationEngine, { validationType: 'setting', _loadingPromise: null, + // this is an odd case where we only want to react to changes that we get + // back from the API rather than local updates + settledIcon: '', + // the settings API endpoint is a little weird as it's singular and we have // to pass in all types - if we ever fetch settings without all types then // save we have problems with the missing settings being removed or reset @@ -44,6 +49,7 @@ export default Service.extend(_ProxyMixin, ValidationEngine, { reload() { return this._loadSettings().then((settings) => { this.set('content', settings); + this.set('settledIcon', get(settings, 'icon')); return this; }); }, @@ -55,7 +61,10 @@ export default Service.extend(_ProxyMixin, ValidationEngine, { return false; } - return settings.save(); + return settings.save().then((settings) => { + this.set('settledIcon', get(settings, 'icon')); + return settings; + }); }, rollbackAttributes() { diff --git a/app/templates/application.hbs b/app/templates/application.hbs index fd1152c3ae..bc832257b0 100644 --- a/app/templates/application.hbs +++ b/app/templates/application.hbs @@ -5,7 +5,13 @@
{{#if showNavMenu}} - {{gh-nav-menu open=autoNavOpen toggleMaximise="toggleAutoNav" openAutoNav="openAutoNav" showMarkdownHelp="toggleMarkdownHelpModal" closeMobileMenu="closeMobileMenu"}} + {{gh-nav-menu + open=autoNavOpen + icon=settings.settledIcon + toggleMaximise="toggleAutoNav" + openAutoNav="openAutoNav" + showMarkdownHelp="toggleMarkdownHelpModal" + closeMobileMenu="closeMobileMenu"}} {{/if}} {{#gh-main onMouseEnter="closeAutoNav" data-notification-count=topNotificationCount}} diff --git a/app/templates/components/gh-nav-menu.hbs b/app/templates/components/gh-nav-menu.hbs index 743e87b041..545aee7e36 100644 --- a/app/templates/components/gh-nav-menu.hbs +++ b/app/templates/components/gh-nav-menu.hbs @@ -1,7 +1,7 @@ {{gh-menu-toggle desktopAction="toggleAutoNav" mobileAction="closeMobileMenu"}} {{#gh-basic-dropdown horizontalPosition="right" calculatePosition=userDropdownPosition as |dropdown|}} {{#dropdown.trigger tagName="header" class="gh-nav-menu"}} -
+
{{config.blogTitle}}
{{session.user.name}}