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 298d7fe52..c1721b95b 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 c9690dbdf..ef3337866 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 a12a00c5f..fdade0d70 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 fdade0d70..cfa4247bf 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 b07dbdd63..266375c52 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 37ebd6746..24aff62f2 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 c1721b95b..82e5cf109 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; } }