Extract fatal notification into fatalError service, add support for EuiToast notifications #15749

cjcenizal · 2017-12-22T00:17:43Z

Relates to #15721. Partially addresses #8194. This simplifies the notification system (making it easier to integrate non-fatal notifications with EuiToasts) and adds support for displaying EuiToasts.

To-do

Make it easier to use toasts within functional tests, with data-test-subj, in a non-brittle way
Add Jest test coverage for GlobalToastList

Functional test changes

Avoid calling `clickToastOK()`

This method has traditionally been used as a hacky proxy for the successful creation of a dashboard, visualization, etc. Successful actions will trigger toasts instead of banner notifications, moving forward. So this method will become less and less useful and will eventually be removed.

Saving dashboards and visualizations is self-confirming

By the same token, now when you call saveDashboard() or saveVisualization(), the method will wait until the confirmation toast appears and resolve successfully only once it appears. This means you no longer need to call these methods and then clickToastOK() afterwards.

New `getBreadcrumbPageTitle()` method

The last breadcrumb behaves as a page title for many apps. This method will return the text within this breadcrumb, essentially returning the page title value.

Make string-checking tests more robust

Visualization tests now leverage this method to confirm the name of a newly-created visualization. But they do so by asserting that the value contains the name of the visualization, not that it matches it. A strict equality match is brittle and will result in a test failure if we decorate the page title with information in addition to the visualization name. But the UX is still intact so the test should not fail. Checking that the page title contains the visualization name ensures that the test will still pass in this case. We should apply similar tactics where possible.

cjcenizal · 2017-12-22T23:02:33Z

@nreese Could you take a look at my changes to Dashboard and let me know what you think? @thomasneirynck Could you do the same with Visualize? @bmcconaghy Would you mind doing the same with Management?

thomasneirynck · 2018-01-03T17:06:04Z

jenkins, test this

thomasneirynck · 2018-01-03T22:16:34Z

src/ui/public/courier/fetch/fetch_now.js

@@ -30,7 +30,7 @@ export function FetchNowProvider(Private, Promise) {
      if (!req.started) return req;
      return req.retry();
    }))
-      .catch(courierNotifier.fatal);
+      .catch(error => fatalError(error, location));


seems like the optimizer is tripping over the uninitialized location variable

Oh shoot! Good catch, thanks. We should add a linting rule for that!

nreese

This looks good from the dashboard perspective. Just a comment about using single vs double quotes.

What are the plans for all of the skipped tests?

nreese · 2018-01-08T23:22:30Z

...anagement/sections/indices/edit_index_pattern/scripted_field_editor/scripted_field_editor.js

@@ -43,7 +43,11 @@ uiRoutes
        this.field = this.indexPattern.fields.byName[fieldName];

        if (!this.field) {
-          notify.error(this.indexPattern + ' does not have a "' + fieldName + '" field.');
+          toastNotifications.add({
+            title: `"${this.indexPattern.title}" index pattern doesn't have a scripted field called "${fieldName}"`,


Do you want to use single quotes to be consistent with the confirm modals?

Yes! Great catch. Will fix.

cjcenizal · 2018-01-08T23:40:59Z

@nreese I consider this level of skipped tests to be a blocker. I'm going to dig into them to identify the cause for their failure (I have a feeling it has something to do with the clickToastOK page object method). Once I've identified the cause, I'll fix several tests spread across several functional areas. Then I'll ask each functional team if they can submit PRs into this branch fixing the remaining tests, using my solution(s) as a guideline.

elasticmachine · 2018-01-26T21:37:41Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cjcenizal · 2018-01-26T21:44:17Z

jenkins, test this

elasticmachine · 2018-01-26T22:48:10Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-01-27T01:59:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-01-27T04:05:37Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…uiToast notifications (elastic#15749) * Reorganize notify/lib files. Extract fatal notification into a fatalError service. * Convert notify/lib tests to use Jest. * Add ToastNotifications, GlobalToastList, and documentation. * Remove notify.info method. * Add createFirstIndexPatternPrompt. * Update testSubjects.exists to accept a timeout argument. * Skip some flaky tests.

…uiToast notifications (#15749) (#16330) * Reorganize notify/lib files. Extract fatal notification into a fatalError service. * Convert notify/lib tests to use Jest. * Add ToastNotifications, GlobalToastList, and documentation. * Remove notify.info method. * Add createFirstIndexPatternPrompt. * Update testSubjects.exists to accept a timeout argument. * Skip some flaky tests.

…rt for EuiToast notifications (elastic#15749)" This reverts commit 6e9fc73.

gchaps

Edited toast text. Format is:

object type > object name > was/were >past tense of action

gchaps · 2018-02-07T17:11:21Z

src/core_plugins/kibana/public/dashboard/dashboard_app.js

@@ -183,14 +184,18 @@ app.directive('dashboardApp', function ($injector) {
      $scope.addVis = function (hit, showToast = true) {
        dashboardStateManager.addNewPanel(hit.id, 'visualization');
        if (showToast) {
-          notify.info(`Visualization successfully added to your dashboard`);
+          toastNotifications.addSuccess('Added visualization to your dashboard');


Visualization was added to your dashboard

gchaps · 2018-02-07T18:01:19Z

src/core_plugins/kibana/public/dashboard/dashboard_app.js

        }
      };

      $scope.addSearch = function (hit) {
        dashboardStateManager.addNewPanel(hit.id, 'search');
-        notify.info(`Search successfully added to your dashboard`);
+        toastNotifications.addSuccess({
+          title: 'Added saved search to your dashboard',


Saved search was added to your dashboard

gchaps · 2018-02-07T18:05:11Z

src/core_plugins/kibana/public/dashboard/dashboard_app.js

@@ -268,7 +273,11 @@ app.directive('dashboardApp', function ($injector) {
          .then(function (id) {
            $scope.kbnTopNav.close('save');
            if (id) {
-              notify.info(`Saved Dashboard as "${dash.title}"`);
+              toastNotifications.addSuccess({
+                title: `Saved '${dash.title}'`,


Dashboard '${dash.title}' was saved

gchaps · 2018-02-07T18:10:32Z

src/core_plugins/kibana/public/discover/controllers/discover.js

@@ -416,7 +416,11 @@ function discoverController(
            $scope.kbnTopNav.close('save');

            if (id) {
-              notify.info('Saved Data Source "' + savedSearch.title + '"');
+              toastNotifications.addSuccess({
+                title: `Saved '${savedSearch.title}'`,


Data source "searchSearch.title" was saved

gchaps · 2018-02-07T20:17:20Z

...anagement/sections/indices/edit_index_pattern/scripted_fields_table/scripted_fields_table.js

@@ -82,11 +81,17 @@ uiModules.get('apps/management')
          });

          if (fieldsAdded > 0) {
-            notify.info(fieldsAdded + ' script fields created');
+            toastNotifications.addSuccess({
+              title: 'Created script fields',


I think that maybe this should be "added" See the button guidelines for a description.
Also, the Kibana "Create Index" page uses "scripted field"

Recommendation:

Scripted field "fieldname" was added

or

Scripted field was added

Prefer one line over title and text

gchaps · 2018-02-07T21:02:01Z

src/ui/public/notify/toasts/TOAST_NOTIFICATIONS.md

+Toasts will automatically be dismissed after a brief delay, but if for some reason you want to dismiss a toast, you can use the returned toast from one of the `add` methods and then pass it to `remove`.
+
+```js
+const toast = toastNotifications.add('Saved document');


Your document was saved

gchaps · 2018-02-07T21:02:52Z

src/ui/public/notify/toasts/TOAST_NOTIFICATIONS.md

+
+```js
+toastNotifications.add({
+  title: 'Saved document',


Your document was saved

gchaps · 2018-02-07T21:06:13Z

src/ui/public/notify/toasts/TOAST_NOTIFICATIONS.md

+
+```js
+toastNotifications.add({
+  title: 'Saved document',


Your document was saved

Only you have editing privileges. Edit permisson (no ending period cause its a link)

gchaps · 2018-02-07T21:06:42Z

src/ui/public/notify/toasts/TOAST_NOTIFICATIONS.md

+
+```js
+toastNotifications.addSuccess({
+  title: 'Saved document',


Your document was saved

gchaps · 2018-02-07T21:08:44Z

src/ui/public/share/directives/share.js

        // Select the text to be copied. If the copy fails, the user can easily copy it manually.
        const copyTextarea = $document.find(selector)[0];
        copyTextarea.select();

        try {
          const isCopied = document.execCommand('copy');
          if (isCopied) {
-            notify.info('URL copied to clipboard.');
+            toastNotifications.add({
+              title: 'URL copied to clipboard',


URL was copied to the clipboard

cjcenizal changed the title ~~Extract fatal notification into fatalError service~~ Extract fatal notification into fatalError service, remove license-checker Dec 22, 2017

chrisronline mentioned this pull request Dec 22, 2017

[META] EUIfication #15721

Closed

47 tasks

cjcenizal changed the title ~~Extract fatal notification into fatalError service, remove license-checker~~ Extract fatal notification into fatalError service, remove pattern-checker Dec 22, 2017

cjcenizal mentioned this pull request Dec 22, 2017

Remove unused pattern-checker directive. #15756

Merged

cjcenizal force-pushed the euify/notification-toasts-extract-fatal branch from 15d1300 to 28a127c Compare December 22, 2017 20:18

cjcenizal changed the title ~~Extract fatal notification into fatalError service, remove pattern-checker~~ Extract fatal notification into fatalError service, add support for EuiToast notifications Dec 22, 2017

thomasneirynck self-requested a review December 26, 2017 15:50

cjcenizal force-pushed the euify/notification-toasts-extract-fatal branch from a14dde4 to 62ce88e Compare January 3, 2018 19:15

thomasneirynck reviewed Jan 3, 2018

View reviewed changes

cjcenizal force-pushed the euify/notification-toasts-extract-fatal branch 14 times, most recently from e08f9d5 to 0e0b7da Compare January 7, 2018 04:28

nreese reviewed Jan 8, 2018

View reviewed changes

cjcenizal force-pushed the euify/notification-toasts-extract-fatal branch 3 times, most recently from a3d42cc to c576066 Compare January 10, 2018 00:06

cjcenizal added 3 commits January 26, 2018 10:32

Change 'No index pattern' prompt to use warning color.

eaa1141

Fix discover functional tests.

c89ad15

Add note on functional test use cases to docs.

fb7d9e4

elastic deleted a comment from elasticmachine Jan 26, 2018

Improve structure of toast docs.

545589d

cjcenizal force-pushed the euify/notification-toasts-extract-fatal branch from b7252d9 to 545589d Compare January 26, 2018 20:31

cjcenizal added 2 commits January 26, 2018 14:53

Increase functional test timeout to 3 minutes.

e036a2d

Skip some flaky tests.

a990b0c

Fix copy to clipboard tests.

a898367

cjcenizal merged commit 6e9fc73 into elastic:master Jan 27, 2018

cjcenizal deleted the euify/notification-toasts-extract-fatal branch January 27, 2018 04:14

cjcenizal added Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. v7.0.0 v6.3.0 labels Jan 27, 2018

cjcenizal mentioned this pull request Jan 27, 2018

[6.x] Extract fatal notification into fatalError service, add support for EuiToast notifications (#15749) #16330

Merged

cjcenizal mentioned this pull request Jan 28, 2018

Fix assertions against toasts in Dashboard and Discover functional tests. #16336

Merged

pickypg mentioned this pull request Jan 28, 2018

Create Global Banner Area #16340

Merged

cjcenizal added a commit to cjcenizal/kibana that referenced this pull request Jan 28, 2018

Revert "Extract fatal notification into fatalError service, add suppo…

d23341c

…rt for EuiToast notifications (elastic#15749)" This reverts commit 6e9fc73.

gchaps reviewed Feb 7, 2018

View reviewed changes

cjcenizal mentioned this pull request Feb 15, 2018

Bring toast copy in line with writing style. #16752

Merged

cjcenizal mentioned this pull request Mar 12, 2018

Improve Notifications #8194

Closed

aphelionz mentioned this pull request May 7, 2018

Accessibility: Kibana 6.3 Meta Issue #18866

Closed

48 tasks

cjcenizal mentioned this pull request Jul 26, 2018

Functional testing guidelines #9162

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract fatal notification into fatalError service, add support for EuiToast notifications #15749

Extract fatal notification into fatalError service, add support for EuiToast notifications #15749

cjcenizal commented Dec 22, 2017 •

edited

Loading

cjcenizal commented Dec 22, 2017

thomasneirynck commented Jan 3, 2018

thomasneirynck Jan 3, 2018

cjcenizal Jan 4, 2018

nreese left a comment •

edited

Loading

nreese Jan 8, 2018

cjcenizal Jan 8, 2018

cjcenizal commented Jan 8, 2018

elasticmachine commented Jan 26, 2018

cjcenizal commented Jan 26, 2018

elasticmachine commented Jan 26, 2018

elasticmachine commented Jan 27, 2018

elasticmachine commented Jan 27, 2018

gchaps left a comment

gchaps Feb 7, 2018

gchaps Feb 7, 2018

gchaps Feb 7, 2018

gchaps Feb 7, 2018

gchaps Feb 7, 2018

gchaps Feb 7, 2018

gchaps Feb 7, 2018

gchaps Feb 7, 2018

gchaps Feb 7, 2018

gchaps Feb 7, 2018

Extract fatal notification into fatalError service, add support for EuiToast notifications #15749

Extract fatal notification into fatalError service, add support for EuiToast notifications #15749

Conversation

cjcenizal commented Dec 22, 2017 • edited Loading

Functional test changes

Avoid calling clickToastOK()

Saving dashboards and visualizations is self-confirming

New getBreadcrumbPageTitle() method

Make string-checking tests more robust

cjcenizal commented Dec 22, 2017

thomasneirynck commented Jan 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nreese left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal commented Jan 8, 2018

elasticmachine commented Jan 26, 2018

💔 Build Failed

cjcenizal commented Jan 26, 2018

elasticmachine commented Jan 26, 2018

💔 Build Failed

elasticmachine commented Jan 27, 2018

💔 Build Failed

elasticmachine commented Jan 27, 2018

💚 Build Succeeded

gchaps left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal commented Dec 22, 2017 •

edited

Loading

Avoid calling `clickToastOK()`

New `getBreadcrumbPageTitle()` method

nreese left a comment •

edited

Loading