From feb133bc280e69fc9544be16c6952f7c4dbf86cd Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 7 Nov 2018 16:08:05 -0800 Subject: [PATCH 1/5] Add support for link in error containers --- ui/app/styles/components/error-container.scss | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ui/app/styles/components/error-container.scss b/ui/app/styles/components/error-container.scss index 298d7fe52fb..c1721b95b4d 100644 --- a/ui/app/styles/components/error-container.scss +++ b/ui/app/styles/components/error-container.scss @@ -3,7 +3,9 @@ height: 100%; padding-top: 25vh; display: flex; - justify-content: center; + flex-direction: column; + justify-content: start; + align-items: center; background: $grey-lighter; .error-message { @@ -19,4 +21,12 @@ border: 1px solid $grey-light; border-radius: $radius; } + + .error-links { + padding-top: 15px; + margin-top: 15px; + border-top: 1px solid $grey-light; + width: 600px; + text-align: center; + } } From d75aa8f99a4792b5c42c2e2b2d98519f7c4eb55b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 7 Nov 2018 16:08:26 -0800 Subject: [PATCH 2/5] Fix a bug where with-watchers wasn't bubbling the willTransition event The impact was the application error was no longer being nulled out, causing the application error to continue to be shown after transitioning. This never happened in apps since it's not possible to transition away from the error screen. --- ui/app/mixins/with-watchers.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/app/mixins/with-watchers.js b/ui/app/mixins/with-watchers.js index c9690dbdf02..ef3337866ec 100644 --- a/ui/app/mixins/with-watchers.js +++ b/ui/app/mixins/with-watchers.js @@ -33,6 +33,9 @@ export default Mixin.create(WithVisibilityDetection, { actions: { willTransition() { this.cancelAllWatchers(); + + // Bubble the action up to the application route + return true; }, }, }); From ac43c0ab7778b261fc09ddca6cfee2fe44fd5e8c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 7 Nov 2018 16:09:23 -0800 Subject: [PATCH 3/5] Add escape hatch links to the error page --- ui/app/templates/application.hbs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/app/templates/application.hbs b/ui/app/templates/application.hbs index a12a00c5f5d..fdade0d7062 100644 --- a/ui/app/templates/application.hbs +++ b/ui/app/templates/application.hbs @@ -30,5 +30,9 @@
{{errorStr}}
{{/if}} + {{/unless}} From d49631bb8aa27918aed5b5c8d02c641fdce953ec Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 7 Nov 2018 16:19:45 -0800 Subject: [PATCH 4/5] Test coverage for error page escape hatch links --- ui/app/templates/application.hbs | 4 +-- .../acceptance/application-errors-test.js | 25 +++++++++++++++++++ ui/tests/pages/jobs/list.js | 2 ++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/ui/app/templates/application.hbs b/ui/app/templates/application.hbs index fdade0d7062..cfa4247bff4 100644 --- a/ui/app/templates/application.hbs +++ b/ui/app/templates/application.hbs @@ -31,8 +31,8 @@ {{/if}} {{/unless}} diff --git a/ui/tests/acceptance/application-errors-test.js b/ui/tests/acceptance/application-errors-test.js index b07dbdd63ca..266375c522b 100644 --- a/ui/tests/acceptance/application-errors-test.js +++ b/ui/tests/acceptance/application-errors-test.js @@ -67,3 +67,28 @@ test('the no leader error state gets its own error message', function(assert) { ); }); }); + +test('error pages include links to the jobs and clients pages', function(assert) { + visit('/a/non-existent/page'); + + andThen(() => { + assert.ok(JobsList.error.isPresent, 'An error is shown'); + JobsList.error.gotoJobs(); + }); + + andThen(() => { + assert.equal(currentURL(), '/jobs', 'Now on the jobs page'); + assert.notOk(JobsList.error.isPresent, 'The error is gone now'); + visit('/a/non-existent/page'); + }); + + andThen(() => { + assert.ok(JobsList.error.isPresent, 'An error is shown'); + JobsList.error.gotoClients(); + }); + + andThen(() => { + assert.equal(currentURL(), '/clients', 'Now on the clients page'); + assert.notOk(JobsList.error.isPresent, 'The error is gone now'); + }); +}); diff --git a/ui/tests/pages/jobs/list.js b/ui/tests/pages/jobs/list.js index 37ebd67468a..24aff62f287 100644 --- a/ui/tests/pages/jobs/list.js +++ b/ui/tests/pages/jobs/list.js @@ -44,6 +44,8 @@ export default create({ title: text('[data-test-error-title]'), message: text('[data-test-error-message]'), seekHelp: clickable('[data-test-error-message] a'), + gotoJobs: clickable('[data-test-error-jobs-link]'), + gotoClients: clickable('[data-test-error-clients-link]'), }, namespaceSwitcher: { From b0028237d5e1388648bf64b53776abb6e73be2fa Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 9 Nov 2018 13:19:30 -0800 Subject: [PATCH 5/5] Get error messages closer to Structure designs --- ui/app/styles/components/error-container.scss | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ui/app/styles/components/error-container.scss b/ui/app/styles/components/error-container.scss index c1721b95b4d..82e5cf10914 100644 --- a/ui/app/styles/components/error-container.scss +++ b/ui/app/styles/components/error-container.scss @@ -6,27 +6,29 @@ flex-direction: column; justify-content: start; align-items: center; - background: $grey-lighter; + background: $white-ter; .error-message { + width: 95vw; max-width: 600px; - .title, - .subtitle { + .title { text-align: center; } } .error-stack-trace { - border: 1px solid $grey-light; + border: 1px solid $grey-lighter; border-radius: $radius; + background: $white; } .error-links { padding-top: 15px; margin-top: 15px; - border-top: 1px solid $grey-light; - width: 600px; + border-top: 1px solid $grey-lighter; + width: 95vw; + max-width: 600px; text-align: center; } }