From efcecb886e115c5ac41db16fc34b1722d3b56d3f Mon Sep 17 00:00:00 2001
From: Kevin Ansfield <kevin@lookingsideways.co.uk>
Date: Mon, 25 Mar 2019 12:20:14 +0000
Subject: [PATCH] Improved theme activation warnings modal content

closes https://github.com/TryGhost/Ghost/issues/10627
- removed confusing/unnecessary message when activation was successful with warnings
- do not display "Warnings" sub-heading if there are no other errors to reduce duplication with the modal title
- changed "Close" button text to "Ok" so it feels more like a confirmation
- refactored modal template to have explicit `this` rather than implicit property lookup ([RFC](https://github.com/emberjs/rfcs/blob/e724c7087e21284f5a34dce9b5de0e5c65b11dbf/text/0308-deprecate-property-lookup-fallback.md))
---
 app/controllers/settings/design.js            | 13 ++--------
 .../components/modal-theme-warnings.hbs       | 25 ++++++++-----------
 app/templates/settings/design.hbs             |  1 -
 3 files changed, 12 insertions(+), 27 deletions(-)

diff --git a/app/controllers/settings/design.js b/app/controllers/settings/design.js
index e850aac6e3..77df70033e 100644
--- a/app/controllers/settings/design.js
+++ b/app/controllers/settings/design.js
@@ -17,12 +17,11 @@ export default Controller.extend({
     session: service(),
     settings: service(),
 
-    newNavItem: null,
-
     dirtyAttributes: false,
-
+    newNavItem: null,
     themes: null,
     themeToDelete: null,
+
     init() {
         this._super(...arguments);
         this.set('newNavItem', NavigationItem.create({isNew: true}));
@@ -127,8 +126,6 @@ export default Controller.extend({
 
         activateTheme(theme) {
             return theme.activate().then((theme) => {
-                let themeName = theme.get('name');
-
                 if (!isEmpty(theme.get('warnings'))) {
                     this.set('themeWarnings', theme.get('warnings'));
                     this.set('showThemeWarningsModal', true);
@@ -138,12 +135,6 @@ export default Controller.extend({
                     this.set('themeErrors', theme.get('errors'));
                     this.set('showThemeWarningsModal', true);
                 }
-
-                if (this.themeErrors || this.themeWarnings) {
-                    let message = `${themeName} activated successfully but some warnings/errors were detected.
-                                   You are still able to use and activate the theme. Here is your report...`;
-                    this.set('message', message);
-                }
             }).catch((error) => {
                 if (isThemeValidationError(error)) {
                     let errors = error.payload.errors[0].details;
diff --git a/app/templates/components/modal-theme-warnings.hbs b/app/templates/components/modal-theme-warnings.hbs
index f38e42a440..e4e3fa2c6d 100644
--- a/app/templates/components/modal-theme-warnings.hbs
+++ b/app/templates/components/modal-theme-warnings.hbs
@@ -1,9 +1,9 @@
 <header class="modal-header">
     <h1 data-test-theme-warnings-title>
-        {{#unless canActivate}}
-            {{title}}
+        {{#unless this.canActivate}}
+            {{this.title}}
         {{else}}
-            {{title}} with {{#if errors}}errors{{else}}warnings{{/if}}
+            {{this.title}} with {{#if this.errors}}errors{{else}}warnings{{/if}}
         {{/unless}}
     </h1>
 </header>
@@ -11,41 +11,36 @@
 
 <div class="modal-body">
     <ul class="theme-validation-errors" data-test-theme-warnings>
-        {{#if message}}
-            <li>
-                <p data-test-theme-warnings-message>{{message}}</p>
-            </li>
-        {{/if}}
-        {{#if fatalErrors}}
+        {{#if this.fatalErrors}}
             <div class="theme-validation-errordescription">
                 <h2 class="theme-validation-errortype fatal">Fatal Errors</h2>
                 <p><em>(Must-fix to activate theme)</em></p>
             </div>
         {{/if}}
-        {{#each fatalErrors as |error|}}
+        {{#each this.fatalErrors as |error|}}
             <li class="theme-validation-item">
                 {{gh-theme-error-li error=error}}
             </li>
         {{/each}}
 
-        {{#if errors}}
+        {{#if this.errors}}
             <div class="theme-validation-errordescription">
                 <h2 class="theme-validation-errortype">Errors</h2>
                 <p><em>(Very recommended to fix, functionality <span>could</span> be restricted)</em></p>
             </div>
         {{/if}}
-        {{#each errors as |error|}}
+        {{#each this.errors as |error|}}
             <li class="theme-validation-item">
                 {{gh-theme-error-li error=error}}
             </li>
         {{/each}}
 
-        {{#if warnings}}
+        {{#if (and this.warnings (or this.fatalErrors this.errors))}}
             <div class="theme-validation-errordescription">
                 <h2 class="theme-validation-errortype">Warnings</h2>
             </div>
         {{/if}}
-        {{#each warnings as |error|}}
+        {{#each this.warnings as |error|}}
             <li class="theme-validation-item">
                 {{gh-theme-error-li error=error}}
             </li>
@@ -56,6 +51,6 @@
 
 <div class="modal-footer">
     <button {{action "closeModal"}} class="gh-btn" data-test-modal-close-button>
-        <span>Close</span>
+        <span>Ok</span>
     </button>
 </div>
diff --git a/app/templates/settings/design.hbs b/app/templates/settings/design.hbs
index 3157c835e9..7523d970a1 100644
--- a/app/templates/settings/design.hbs
+++ b/app/templates/settings/design.hbs
@@ -153,7 +153,6 @@
                         title="Activation successful"
                         warnings=themeWarnings
                         errors=themeErrors
-                        message=message
                         canActivate=true
                     )
                     close=(action "hideThemeWarningsModal")