💄 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.
This commit is contained in:
Aileen Nowak 2017-07-20 17:30:45 +07:00 committed by Kevin Ansfield
parent a99ecf4818
commit 2cbb181866
6 changed files with 146 additions and 63 deletions

View File

@ -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');
}
}
});

View File

@ -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;
}

View File

@ -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}}

View File

@ -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>

View File

@ -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}}

View File

@ -280,6 +280,16 @@ describe('Acceptance: Settings - Design', function () {
message: 'Theme is not compatible or contains errors.',
errorType: 'ThemeValidationError',
errorDetails: [
{
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>',
failures: [
{
ref: '/assets/javascripts/ui.js'
}
]
},
{
level: 'error',
rule: 'Templates must contain valid Handlebars.',
@ -293,16 +303,6 @@ describe('Acceptance: Settings - Design', function () {
message: 'The partial index_meta could not be found'
}
]
},
{
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>',
failures: [
{
ref: '/assets/javascripts/ui.js'
}
]
}
]
}
@ -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/);
@ -474,6 +478,16 @@ describe('Acceptance: Settings - Design', function () {
message: 'Theme is not compatible or contains errors.',
errorType: 'ThemeValidationError',
errorDetails: [
{
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>',
failures: [
{
ref: '/assets/javascripts/ui.js'
}
]
},
{
level: 'error',
rule: 'Templates must contain valid Handlebars.',
@ -487,16 +501,6 @@ describe('Acceptance: Settings - Design', function () {
message: 'The partial index_meta could not be found'
}
]
},
{
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>',
failures: [
{
ref: '/assets/javascripts/ui.js'
}
]
}
]
}
@ -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/);