🐛 fix broken sidebar after successful import (#658)

closes TryGhost/Ghost#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

ghost/admin/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');

ghost/admin/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

ghost/admin/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) => {

ghost/admin/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() {

ghost/admin/app/templates/application.hbs

@@ -5,7 +5,13 @@
 
     <div class="gh-viewport {{if autoNav 'gh-autonav'}} {{if showSettingsMenu 'settings-menu-expanded'}} {{if showMobileMenu 'mobile-menu-expanded'}}">
         {{#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}}

ghost/admin/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"}}
-        <div class="gh-nav-menu-icon" style={{navMenuIcon}}></div>
+        <div class="gh-nav-menu-icon" style={{iconStyle}}></div>
         <div class="gh-nav-menu-details">
             <div class="gh-nav-menu-details-blog">{{config.blogTitle}}</div>
             <div class="gh-nav-menu-details-user">{{session.user.name}}</div>