🐛 Fixed unsaved changes modal when using Cmd+S on tag/member screens

no issue

- keep a scratch model on the tag/member controllers rather than inside of the form components
  - allows the controller's `save` task to transfer scratch values to real values before saving
  - means that pressing Cmd+S whilst a field is still focused will save the expected value rather than the old value avoiding unsaved changes modals when trying to leave the screen when you think you've already saved
- fixed route and url not changing after saving a new member
- fixed error when clicking delete tag button
- cleaned up unused `showDeleteTagModal` actions

ghost/admin/app/components/gh-member-settings-form.js

@@ -1,5 +1,4 @@
 import Component from '@ember/component';
-import boundOneWay from 'ghost-admin/utils/bound-one-way';
 import moment from 'moment';
 import {computed} from '@ember/object';
 import {gt, reads} from '@ember/object/computed';
@@ -12,14 +11,9 @@ export default Component.extend({
 
     // Allowed actions
     setProperty: () => {},
-    showDeleteTagModal: () => {},
 
     canEditEmail: reads('member.isNew'),
 
-    scratchName: boundOneWay('member.name'),
-    scratchEmail: boundOneWay('member.email'),
-    scratchNote: boundOneWay('member.note'),
-
     hasMultipleSubscriptions: gt('member.stripe', 1),
 
     subscriptions: computed('member.stripe', function () {
@@ -46,11 +40,6 @@ export default Component.extend({
     actions: {
         setProperty(property, value) {
             this.setProperty(property, value);
-        },
-
-        deleteTag() {
-            this.showDeleteTagModal();
         }
     }
-
 });

ghost/admin/app/components/gh-tag-settings-form.js

@@ -1,7 +1,6 @@
 /* global key */
 import Component from '@ember/component';
 import Ember from 'ember';
-import boundOneWay from 'ghost-admin/utils/bound-one-way';
 import {computed} from '@ember/object';
 import {htmlSafe} from '@ember/string';
 import {inject as service} from '@ember/service';
@@ -18,13 +17,6 @@ export default Component.extend({
 
     // Allowed actions
     setProperty: () => {},
-    showDeleteTagModal: () => {},
-
-    scratchName: boundOneWay('tag.name'),
-    scratchSlug: boundOneWay('tag.slug'),
-    scratchDescription: boundOneWay('tag.description'),
-    scratchMetaTitle: boundOneWay('tag.metaTitle'),
-    scratchMetaDescription: boundOneWay('tag.metaDescription'),
 
     title: computed('tag.isNew', function () {
         if (this.get('tag.isNew')) {
@@ -34,10 +26,10 @@ export default Component.extend({
         }
     }),
 
-    seoTitle: computed('scratchName', 'scratchMetaTitle', function () {
-        let metaTitle = this.scratchMetaTitle || '';
+    seoTitle: computed('scratchTag.{title,metaTitle}', function () {
+        let metaTitle = this.scratchTag.metaTitle || '';
 
-        metaTitle = metaTitle.length > 0 ? metaTitle : this.scratchName;
+        metaTitle = metaTitle.length > 0 ? metaTitle : this.scratchTag.title;
 
         if (metaTitle && metaTitle.length > 70) {
             metaTitle = metaTitle.substring(0, 70).trim();
@@ -48,9 +40,9 @@ export default Component.extend({
         return metaTitle;
     }),
 
-    seoURL: computed('scratchSlug', function () {
+    seoURL: computed('scratchTag.slug', function () {
         let blogUrl = this.get('config.blogUrl');
-        let seoSlug = this.scratchSlug || '';
+        let seoSlug = this.scratchTag.slug || '';
 
         let seoURL = `${blogUrl}/tag/${seoSlug}`;
 
@@ -68,10 +60,10 @@ export default Component.extend({
         return seoURL;
     }),
 
-    seoDescription: computed('scratchDescription', 'scratchMetaDescription', function () {
-        let metaDescription = this.scratchMetaDescription || '';
+    seoDescription: computed('scratchTag.{description,metaDescription}', function () {
+        let metaDescription = this.scratchTag.metaDescription || '';
 
-        metaDescription = metaDescription.length > 0 ? metaDescription : this.scratchDescription;
+        metaDescription = metaDescription.length > 0 ? metaDescription : this.scratchTag.description;
 
         if (metaDescription && metaDescription.length > 156) {
             metaDescription = metaDescription.substring(0, 156).trim();
@@ -114,10 +106,6 @@ export default Component.extend({
 
         closeMeta() {
             this.set('isViewingSubview', false);
-        },
-
-        deleteTag() {
-            this.showDeleteTagModal();
         }
     },
 

ghost/admin/app/controllers/member.js

@@ -1,11 +1,16 @@
 import Controller from '@ember/controller';
+import EmberObject from '@ember/object';
+import boundOneWay from 'ghost-admin/utils/bound-one-way';
 import moment from 'moment';
+import windowProxy from 'ghost-admin/utils/window-proxy';
 import {alias} from '@ember/object/computed';
-import {computed} from '@ember/object';
+import {computed, defineProperty} from '@ember/object';
 import {inject as controller} from '@ember/controller';
 import {inject as service} from '@ember/service';
 import {task} from 'ember-concurrency';
 
+const SCRATCH_PROPS = ['name', 'email', 'note'];
+
 export default Controller.extend({
     members: controller(),
     notifications: service(),
@@ -14,6 +19,12 @@ export default Controller.extend({
 
     member: alias('model'),
 
+    scratchMember: computed('member', function () {
+        let scratchMember = EmberObject.create({member: this.member});
+        SCRATCH_PROPS.forEach(prop => defineProperty(scratchMember, prop, boundOneWay(`member.${prop}`)));
+        return scratchMember;
+    }),
+
     subscribedAt: computed('member.createdAtUTC', function () {
         let memberSince = moment(this.member.createdAtUTC).from(moment());
         let createdDate = moment(this.member.createdAtUTC).format('MMM DD, YYYY');
@@ -73,9 +84,20 @@ export default Controller.extend({
     },
 
     save: task(function* () {
-        let member = this.member;
+        let {member, scratchMember} = this;
+
+        // if Cmd+S is pressed before the field loses focus make sure we're
+        // saving the intended property values
+        let scratchProps = scratchMember.getProperties(SCRATCH_PROPS);
+        member.setProperties(scratchProps);
+
         try {
-            return yield member.save();
+            yield member.save();
+
+            // replace 'member.new' route with 'member' route
+            this.replaceRoute('member', member);
+
+            return member;
         } catch (error) {
             if (error) {
                 this.notifications.showAPIError(error, {key: 'member.save'});
@@ -83,11 +105,6 @@ export default Controller.extend({
         }
     }).drop(),
 
-    _saveMemberProperty(propKey, newValue) {
-        let member = this.member;
-        member.set(propKey, newValue);
-    },
-
     fetchMember: task(function* (memberId) {
         this.set('isLoading', true);
 
@@ -98,6 +115,10 @@ export default Controller.extend({
             this.set('isLoading', false);
             return member;
         });
-    })
+    }),
 
+    _saveMemberProperty(propKey, newValue) {
+        let member = this.member;
+        member.set(propKey, newValue);
+    }
 });

ghost/admin/app/controllers/tag.js

@@ -1,10 +1,15 @@
 import Controller from '@ember/controller';
+import EmberObject from '@ember/object';
+import boundOneWay from 'ghost-admin/utils/bound-one-way';
 import windowProxy from 'ghost-admin/utils/window-proxy';
 import {alias} from '@ember/object/computed';
+import {computed, defineProperty} from '@ember/object';
 import {inject as service} from '@ember/service';
 import {slugify} from '@tryghost/string';
 import {task} from 'ember-concurrency';
 
+const SCRATCH_PROPS = ['name', 'slug', 'description', 'metaTitle', 'metaDescription'];
+
 export default Controller.extend({
     notifications: service(),
     router: service(),
@@ -13,6 +18,12 @@ export default Controller.extend({
 
     tag: alias('model'),
 
+    scratchTag: computed('tag', function () {
+        let scratchTag = EmberObject.create({tag: this.tag});
+        SCRATCH_PROPS.forEach(prop => defineProperty(scratchTag, prop, boundOneWay(`tag.${prop}`)));
+        return scratchTag;
+    }),
+
     actions: {
         setProperty(propKey, value) {
             this._saveTagProperty(propKey, value);
@@ -64,56 +75,21 @@ export default Controller.extend({
         }
     },
 
-    _saveTagProperty(propKey, newValue) {
-        let tag = this.tag;
-        let isNewTag = tag.get('isNew');
-        let currentValue = tag.get(propKey);
-
-        if (newValue) {
-            newValue = newValue.trim();
-        }
-
-        // Quit if there was no change
-        if (newValue === currentValue) {
-            return;
-        }
-
-        tag.set(propKey, newValue);
-
-        // Generate slug based on name for new tag when empty
-        if (propKey === 'name' && !tag.get('slug') && isNewTag) {
-            let slugValue = slugify(newValue);
-            if (/^#/.test(newValue)) {
-                slugValue = 'hash-' + slugValue;
-            }
-            tag.set('slug', slugValue);
-        }
-
-        // TODO: This is required until .validate/.save mark fields as validated
-        tag.get('hasValidated').addObject(propKey);
-    },
-
     save: task(function* () {
-        let tag = this.tag;
-        let isNewTag = tag.get('isNew');
+        let {tag, scratchTag} = this;
+
+        // if Cmd+S is pressed before the field loses focus make sure we're
+        // saving the intended property values
+        let scratchProps = scratchTag.getProperties(SCRATCH_PROPS);
+        tag.setProperties(scratchProps);
+
         try {
-            let savedTag = yield tag.save();
+            yield tag.save();
+
             // replace 'new' route with 'tag' route
-            this.replaceRoute('tag', savedTag);
+            this.replaceRoute('tag', tag);
 
-            // update the URL if the slug changed
-            if (!isNewTag) {
-                let currentPath = window.location.hash;
-
-                let newPath = currentPath.split('/');
-                if (newPath[newPath.length - 1] !== savedTag.get('slug')) {
-                    newPath[newPath.length - 1] = savedTag.get('slug');
-                    newPath = newPath.join('/');
-
-                    windowProxy.replaceState({path: newPath}, '', newPath);
-                }
-            }
-            return savedTag;
+            return tag;
         } catch (error) {
             if (error) {
                 this.notifications.showAPIError(error, {key: 'tag.save'});
@@ -129,5 +105,33 @@ export default Controller.extend({
             this.set('isLoading', false);
             return tag;
         });
-    })
+    }),
+
+    _saveTagProperty(propKey, newValue) {
+        let tag = this.tag;
+        let currentValue = tag.get(propKey);
+
+        if (newValue) {
+            newValue = newValue.trim();
+        }
+
+        // Quit if there was no change
+        if (newValue === currentValue) {
+            return;
+        }
+
+        tag.set(propKey, newValue);
+
+        // Generate slug based on name for new tag when empty
+        if (propKey === 'name' && !tag.get('slug') && tag.isNew) {
+            let slugValue = slugify(newValue);
+            if (/^#/.test(newValue)) {
+                slugValue = 'hash-' + slugValue;
+            }
+            tag.set('slug', slugValue);
+        }
+
+        // TODO: This is required until .validate/.save mark fields as validated
+        tag.get('hasValidated').addObject(propKey);
+    }
 });

ghost/admin/app/templates/components/gh-member-settings-form.hbs

@@ -7,10 +7,9 @@
                 {{gh-text-input
                     id="member-name"
                     name="name"
-                    value=(readonly scratchName)
+                    value=this.scratchMember.name
                     tabindex="1"
-                    input=(action (mut scratchName) value="target.value")
-                    focus-out=(action 'setProperty' 'name' scratchName)}}
+                    focus-out=(action 'setProperty' 'name' this.scratchMember.name)}}
                 {{gh-error-message errors=member.errors property="name"}}
             {{/gh-form-group}}
 
@@ -18,21 +17,20 @@
                 <label for="member-email">Email</label>
                 {{#if canEditEmail}}
                     {{gh-text-input
-                        value=(readonly scratchEmail)
+                        value=this.scratchMember.email
                         id="member-email"
                         name="email"
                         tabindex="2"
                         autocapitalize="off"
                         autocorrect="off"
                         autocomplete="off"
-                        focus-out=(action 'setProperty' 'email' scratchEmail)
-                        input=(action (mut scratchEmail) value="target.value")}}
+                        focus-out=(action 'setProperty' 'email' this.scratchMember.email)}}
                     {{gh-error-message errors=member.errors property="email"}}
                 {{else}}
                     {{gh-text-input
                         name="email-disabled"
                         disabled=true
-                        value=(readonly scratchEmail)}}
+                        value=this.scratchMember.email}}
                 {{/if}}
             {{/gh-form-group}}
         </div>
@@ -44,12 +42,11 @@
                     name="note"
                     class="gh-member-details-textarea"
                     tabindex="3"
-                    value=(readonly scratchNote)
-                    input=(action (mut scratchNote) value="target.value")
-                    focus-out=(action 'setProperty' 'note' scratchNote)
+                    value=this.scratchMember.note
+                    focus-out=(action 'setProperty' 'note' this.scratchMember.note)
                 }}
                 {{gh-error-message errors=member.errors property="note"}}
-                <p>Not visible to member. Maximum: <b>500</b> characters. You’ve used {{gh-count-down-characters scratchNote 500}}</p>
+                <p>Not visible to member. Maximum: <b>500</b> characters. You’ve used {{gh-count-down-characters this.scratchMember.note 500}}</p>
             {{/gh-form-group}}
         </div>
     </div>

ghost/admin/app/templates/components/gh-tag-settings-form.hbs

@@ -6,10 +6,9 @@
         {{gh-text-input
                     id="tag-name"
                     name="name"
-                    value=(readonly scratchName)
+                    value=this.scratchTag.name
                     tabindex="1"
-                    input=(action (mut scratchName) value="target.value")
-                    focus-out=(action 'setProperty' 'name' scratchName)}}
+                    focus-out=(action 'setProperty' 'name' this.scratchTag.name)}}
         <p class="description">Start with # to create internal tags. <a
                 href="https://ghost.org/docs/concepts/tags/#internal-tag" target="_blank" rel="noreferrer">Learn
                 more</a></p>
@@ -19,13 +18,12 @@
         {{#gh-form-group errors=tag.errors hasValidated=tag.hasValidated property="slug"}}
         <label for="tag-slug">Slug</label>
         {{gh-text-input
-                    value=(readonly scratchSlug)
+                    value=this.scratchTag.slug
                     id="tag-slug"
                     name="slug"
                     tabindex="2"
-                    focus-out=(action 'setProperty' 'slug' scratchSlug)
-                    input=(action (mut scratchSlug) value="target.value")}}
-        {{gh-url-preview prefix="tag" slug=scratchSlug tagName="p" classNames="description"}}
+                    focus-out=(action 'setProperty' 'slug' this.scratchTag.slug)}}
+        {{gh-url-preview prefix="tag" slug=this.scratchTag.slug tagName="p" classNames="description"}}
         {{gh-error-message errors=activeTag.errors property="slug"}}
         {{/gh-form-group}}
 
@@ -36,12 +34,11 @@
                     name="description"
                     class="gh-tag-details-textarea"
                     tabindex="3"
-                    value=(readonly scratchDescription)
-                    input=(action (mut scratchDescription) value="target.value")
-                    focus-out=(action 'setProperty' 'description' scratchDescription)
+                    value=this.scratchTag.description
+                    focus-out=(action 'setProperty' 'description' this.scratchTag.description)
                 }}
         {{gh-error-message errors=tag.errors property="description"}}
-        <p>Maximum: <b>500</b> characters. You’ve used {{gh-count-down-characters scratchDescription 500}}</p>
+        <p>Maximum: <b>500</b> characters. You’ve used {{gh-count-down-characters this.scratchTag.description 500}}</p>
         {{/gh-form-group}}
     </div>
     <div class="order-0 mb6 mb0-ns order-2-ns w-100 w-50-m w-third-l">
@@ -64,13 +61,12 @@
         {{gh-text-input
                     id="meta-title"
                     name="metaTitle"
-                    placeholder=scratchName
+                    placeholder=this.scratchTag.name
                     tabindex="4"
-                    value=(readonly scratchMetaTitle)
-                    input=(action (mut scratchMetaTitle) value="target.value")
-                    focus-out=(action "setProperty" "metaTitle" scratchMetaTitle)}}
+                    value=this.scratchTag.metaTitle
+                    focus-out=(action "setProperty" "metaTitle" this.scratchTag.metaTitle)}}
         {{gh-error-message errors=tag.errors property="metaTitle"}}
-        <p>Recommended: <b>70</b> characters. You’ve used {{gh-count-down-characters scratchMetaTitle 70}}</p>
+        <p>Recommended: <b>70</b> characters. You’ve used {{gh-count-down-characters this.scratchTag.metaTitle 70}}</p>
         {{/gh-form-group}}
 
         {{#gh-form-group errors=tag.errors hasValidated=tag.hasValidated property="metaDescription"}}
@@ -79,14 +75,13 @@
                     id="meta-description"
                     name="metaDescription"
                     class="gh-tag-details-textarea"
-                    placeholder=scratchDescription
+                    placeholder=this.scratchTag.description
                     tabindex="5"
-                    value=(readonly scratchMetaDescription)
-                    input=(action (mut scratchMetaDescription) value="target.value")
-                    focus-out=(action "setProperty" "metaDescription" scratchMetaDescription)
+                    value=this.scratchTag.metaDescription
+                    focus-out=(action "setProperty" "metaDescription" this.scratchTag.metaDescription)
                 }}
         {{gh-error-message errors=tag.errors property="metaDescription"}}
-        <p>Recommended: <b>156</b> characters. You’ve used {{gh-count-down-characters scratchMetaDescription 156}}</p>
+        <p>Recommended: <b>156</b> characters. You’ve used {{gh-count-down-characters this.scratchTag.metaDescription 156}}</p>
         {{/gh-form-group}}
     </div>
     <div class="w-100 w-50-m w-third-l">

ghost/admin/app/templates/member.hbs

@@ -45,9 +45,9 @@
 
         <GhMemberSettingsForm
             @member={{this.member}}
+            @scratchMember={{this.scratchMember}}
             @setProperty={{action "setProperty"}}
-            @isLoading={{this.isLoading}}
-            @showDeleteTagModal={{action "toggleDeleteMemberModal"}} />
+            @isLoading={{this.isLoading}} />
     </form>
 
     {{#unless this.member.isNew}}

ghost/admin/app/templates/tag.hbs

@@ -13,8 +13,8 @@
 
         <GhTagSettingsForm
             @tag={{this.tag}}
-            @setProperty={{action "setProperty"}}
-            @showDeleteTagModal={{action "toggleDeleteTagModal"}} />
+            @scratchTag={{this.scratchTag}}
+            @setProperty={{action "setProperty"}} />
     </form>
 
     {{#unless this.tag.isNew}}
@@ -38,5 +38,5 @@
         @model={{this.tag}}
         @confirm={{action "deleteTag"}}
         @close={{action "toggleDeleteTagModal"}}
-        @modifier="action wide"}} />
+        @modifier="action wide" />
 {{/if}}