💄 Theme upload modal style improvements (#784)

no issue With GScan sending error details now, the modal was a bit overloaded. This PR adds a toggle for each error rule which - when clicked - shows the details and the affected files.
TryGhost · Jul 20, 2017 · 3756f8a · 3756f8a
1 parent 6113b1a
commit 3756f8a
Show file tree

Hide file tree

Showing 6 changed files with 146 additions and 63 deletions.
diff --git a/app/components/gh-theme-error-li.js b/app/components/gh-theme-error-li.js
@@ -2,5 +2,12 @@ import Component from 'ember-component';
 
 export default Component.extend({
     tagName: '',
-    error: null
+    error: null,
+    showDetails: false,
+
+    actions: {
+        toggleDetails() {
+            this.toggleProperty('showDetails');
+        }
+    }
 });
diff --git a/app/styles/layouts/settings.css b/app/styles/layouts/settings.css
@@ -354,8 +354,8 @@
 .gh-themes-uploadbtn {
     margin-top: 25px;
 }
-/*Errors */
 
+/*Errors */
 .theme-validation-errors {
     padding-left: 0;
 }
@@ -364,17 +364,6 @@
     margin-bottom: 0;
 }
 
-.theme-validation-errors > li {
-    margin-bottom: 1.2em;
-    list-style: none;
-    font-size: 15px;
-}
-
-.theme-validation-errors > li > ul {
-    margin-top: 0.2em;
-    font-size: 13px;
-}
-
 .theme-validation-errordescription {
     margin-top: 1em;
     margin-bottom: 0.5em;
@@ -385,9 +374,59 @@
 }
 
 .theme-validation-errortype {
-    font-size: 18px;
+    font-size: 1.8rem;
 }
 
 .theme-validation-errortype.fatal {
     color: var(--red);
 }
+
+.theme-validation-item {
+    margin: 0 0 15px;
+    padding: 15px 20px;
+    border: 1px solid #e5eff5;
+    border-radius: 5px;
+    display: flex;
+    flex-direction: column;
+}
+
+.theme-validation-item h4 {
+    margin: 0;
+    font-size: 1.5rem;
+    font-weight: 500;
+}
+
+.theme-validation-item h6 {
+    font-size: 1.4rem;
+    font-weight: 500;
+}
+
+.theme-validation-toggle-details {
+    display: flex;
+    justify-content: space-between;
+    flex-grow: 1;
+    align-items: center;
+    padding: 0;
+    color: var(--darkgrey);
+    text-decoration: none!important;
+}
+
+.theme-validation-rule-icon {
+    flex-shrink: 0;
+    margin-left: 5px;
+    width: 13px;
+    color: var(--midgrey);
+    transition: all 0.1s ease-out;
+}
+
+.theme-validation-rule-icon svg path {
+    fill: var(--midgrey);
+}
+
+.theme-validation-details {
+    margin-top: 15px;
+}
+
+.theme-validation-list {
+    margin-top: 1em;
+}
diff --git a/app/templates/components/gh-theme-error-li.hbs b/app/templates/components/gh-theme-error-li.hbs
@@ -1,13 +1,28 @@
-<li>
-    {{#if error.details}}
-        {{{error.details}}}
-    {{else}}
+<a href="" class="theme-validation-toggle-details" {{action "toggleDetails"}} data-test-toggle-details>
+    <h4 class="theme-validation-rule-text">
         {{{error.rule}}}
-    {{/if}}
+    </h4>
+    <div class="theme-validation-rule-icon">
+        {{#if showDetails}}
+            {{inline-svg "arrow-down"}}
+        {{else}}
+            {{inline-svg "arrow-right"}}
+        {{/if}}
+    </div>
+</a>
 
-    <ul>
-        {{#each error.failures as |failure|}}
-            <li><code>{{failure.ref}}</code>{{#if failure.message}}: {{failure.message}}{{/if}}</li>
-        {{/each}}
-    </ul>
-</li>
+{{#if showDetails}}
+    <p class="theme-validation-details">
+        {{{error.details}}}
+    </p>
+    {{#if error.failures}}
+        <div class="theme-validation-list">
+            <h6>Affected files:</h6>
+            <ul>
+                {{#each error.failures as |failure|}}
+                    <li><code>{{failure.ref}}</code>{{#if failure.message}}: {{failure.message}}{{/if}}</li>
+                {{/each}}
+            </ul>
+        </div>
+    {{/if}}
+{{/if}}
diff --git a/app/templates/components/modals/theme-warnings.hbs b/app/templates/components/modals/theme-warnings.hbs
@@ -17,7 +17,9 @@
             </div>
         {{/if}}
         {{#each fatalErrors as |error|}}
-            {{gh-theme-error-li error=error}}
+            <li class="theme-validation-item">
+                {{gh-theme-error-li error=error}}
+            </li>
         {{/each}}
 
         {{#if errors}}
@@ -27,7 +29,9 @@
             </div>
         {{/if}}
         {{#each errors as |error|}}
-            {{gh-theme-error-li error=error}}
+            <li class="theme-validation-item">
+                {{gh-theme-error-li error=error}}
+            </li>
         {{/each}}
 
         {{#if warnings}}
@@ -36,7 +40,9 @@
             </div>
         {{/if}}
         {{#each warnings as |error|}}
-            {{gh-theme-error-li error=error}}
+            <li class="theme-validation-item">
+                {{gh-theme-error-li error=error}}
+            </li>
         {{/each}}
 
     </ul>

diff --git a/app/templates/components/modals/upload-theme.hbs b/app/templates/components/modals/upload-theme.hbs
@@ -33,7 +33,9 @@
                     </div>
                 {{/if}}
                 {{#each validationErrors as |error|}}
-                    {{gh-theme-error-li error=error}}
+                    <li class="theme-validation-item">
+                        {{gh-theme-error-li error=error}}
+                    </li>
                 {{/each}}
 
                 {{#if validationWarnings}}
@@ -42,7 +44,9 @@
                     </div>
                 {{/if}}
                 {{#each validationWarnings as |error|}}
-                    {{gh-theme-error-li error=error}}
+                    <li class="theme-validation-item">
+                        {{gh-theme-error-li error=error}}
+                    </li>
                 {{/each}}
             </ul>
         {{else}}
@@ -64,7 +68,9 @@
                 </div>
             {{/if}}
             {{#each fatalValidationErrors as |error|}}
-                {{gh-theme-error-li error=error}}
+                <li class="theme-validation-item">
+                    {{gh-theme-error-li error=error}}
+                </li>
             {{/each}}
 
             {{#if validationErrors}}
@@ -74,7 +80,9 @@
                 </div>
             {{/if}}
             {{#each validationErrors as |error|}}
-                {{gh-theme-error-li error=error}}
+                <li class="theme-validation-item">
+                    {{gh-theme-error-li error=error}}
+                </li>
             {{/each}}
         </ul>
     {{else}}

diff --git a/tests/acceptance/settings/design-test.js b/tests/acceptance/settings/design-test.js
@@ -282,25 +282,25 @@ describe('Acceptance: Settings - Design', function () {
                             errorDetails: [
                                 {
                                     level: 'error',
-                                    rule: 'Templates must contain valid Handlebars.',
+                                    rule: 'Assets such as CSS & JS must use the <code>{{asset}}</code> helper',
+                                    details: '<p>The listed files should be included using the <code>{{asset}}</code> helper.</p>',
                                     failures: [
                                         {
-                                            ref: 'index.hbs',
-                                            message: 'The partial index_meta could not be found'
-                                        },
-                                        {
-                                            ref: 'tag.hbs',
-                                            message: 'The partial index_meta could not be found'
+                                            ref: '/assets/javascripts/ui.js'
                                         }
                                     ]
                                 },
                                 {
                                     level: 'error',
-                                    rule: 'Assets such as CSS & JS must use the <code>{{asset}}</code> helper',
-                                    details: '<p>The listed files should be included using the <code>{{asset}}</code> helper.</p>',
+                                    rule: 'Templates must contain valid Handlebars.',
                                     failures: [
                                         {
-                                            ref: '/assets/javascripts/ui.js'
+                                            ref: 'index.hbs',
+                                            message: 'The partial index_meta could not be found'
+                                        },
+                                        {
+                                            ref: 'tag.hbs',
+                                            message: 'The partial index_meta could not be found'
                                         }
                                     ]
                                 }
@@ -319,19 +319,21 @@ describe('Acceptance: Settings - Design', function () {
             ).to.equal('Invalid theme');
 
             expect(
-                find('.theme-validation-errors').text(),
+                find('.theme-validation-rule-text').text(),
                 'top-level errors are displayed'
             ).to.match(/Templates must contain valid Handlebars/);
 
+            await click(testSelector('toggle-details'));
+
             expect(
-                find('.theme-validation-errors').text(),
+                find('.theme-validation-details').text(),
                 'top-level errors do not escape HTML'
             ).to.match(/The listed files should be included using the {{asset}} helper/);
 
             expect(
-                find('.theme-validation-errors').text(),
+                find('.theme-validation-list ul li').text(),
                 'individual failures are displayed'
-            ).to.match(/index\.hbs: The partial index_meta could not be found/);
+            ).to.match(/\/assets\/javascripts\/ui\.js/);
 
             // reset to default mirage handlers
             mockThemes(server);
@@ -397,13 +399,15 @@ describe('Acceptance: Settings - Design', function () {
                 'modal title after uploading theme with warnings'
             ).to.equal('Upload successful with warnings/errors!');
 
+            await click(testSelector('toggle-details'));
+
             expect(
-                find('.theme-validation-errors').text(),
+                find('.theme-validation-details').text(),
                 'top-level warnings are displayed'
             ).to.match(/The listed files should be included using the {{asset}} helper/);
 
             expect(
-                find('.theme-validation-errors').text(),
+                find('.theme-validation-list ul li').text(),
                 'individual warning failures are displayed'
             ).to.match(/\/assets\/dist\/img\/apple-touch-icon\.png/);
 
@@ -476,25 +480,25 @@ describe('Acceptance: Settings - Design', function () {
                             errorDetails: [
                                 {
                                     level: 'error',
-                                    rule: 'Templates must contain valid Handlebars.',
+                                    rule: 'Assets such as CSS & JS must use the <code>{{asset}}</code> helper',
+                                    details: '<p>The listed files should be included using the <code>{{asset}}</code> helper.</p>',
                                     failures: [
                                         {
-                                            ref: 'index.hbs',
-                                            message: 'The partial index_meta could not be found'
-                                        },
-                                        {
-                                            ref: 'tag.hbs',
-                                            message: 'The partial index_meta could not be found'
+                                            ref: '/assets/javascripts/ui.js'
                                         }
                                     ]
                                 },
                                 {
                                     level: 'error',
-                                    rule: 'Assets such as CSS & JS must use the <code>{{asset}}</code> helper',
-                                    details: '<p>The listed files should be included using the <code>{{asset}}</code> helper.</p>',
+                                    rule: 'Templates must contain valid Handlebars.',
                                     failures: [
                                         {
-                                            ref: '/assets/javascripts/ui.js'
+                                            ref: 'index.hbs',
+                                            message: 'The partial index_meta could not be found'
+                                        },
+                                        {
+                                            ref: 'tag.hbs',
+                                            message: 'The partial index_meta could not be found'
                                         }
                                     ]
                                 }
@@ -518,15 +522,17 @@ describe('Acceptance: Settings - Design', function () {
                 'top-level errors are displayed in activation errors'
             ).to.match(/Templates must contain valid Handlebars/);
 
+            await click(testSelector('toggle-details'));
+
             expect(
-                find(testSelector('theme-warnings')).text(),
+                find('.theme-validation-details').text(),
                 'top-level errors do not escape HTML in activation errors'
             ).to.match(/The listed files should be included using the {{asset}} helper/);
 
             expect(
-                find(testSelector('theme-warnings')).text(),
+                find('.theme-validation-list ul li').text(),
                 'individual failures are displayed in activation errors'
-            ).to.match(/index\.hbs: The partial index_meta could not be found/);
+            ).to.match(/\/assets\/javascripts\/ui\.js/);
 
             // restore default mirage handlers
             mockThemes(server);
@@ -572,13 +578,15 @@ describe('Acceptance: Settings - Design', function () {
                 'modal title after activating theme with warnings'
             ).to.equal('Activated successful with warnings/errors!');
 
+            await click(testSelector('toggle-details'));
+
             expect(
-                find(testSelector('theme-warnings')).text(),
+                find('.theme-validation-details').text(),
                 'top-level warnings are displayed in activation warnings'
             ).to.match(/The listed files should be included using the {{asset}} helper/);
 
             expect(
-                find(testSelector('theme-warnings')).text(),
+                find('.theme-validation-list ul li').text(),
                 'individual warning failures are displayed in activation warnings'
             ).to.match(/\/assets\/dist\/img\/apple-touch-icon\.png/);