[SharedUX] Merge similar toast messages in case of a toast-flood/storm

[delanni]

Summary

This PR addresses the occasional toast-floods/toast-storms with a simple catch mechanism: deduplicate/group toasts by their broad alikeness, their text and title.

This implementation plugs in to the global_toast_list.tsx in Kibana's GlobalToastList component, capturing updates on the toast update stream, and collapses toasts before passing them further to the downstream EUI Toast list react components.

The core idea is to not display notifications directly, but to keep the toast notifications apart from their visual representation. This way, we can represent more notifications with one toast on our end, if we group rightly. The only issue then, is to clean up everything nicely when it's time. For this we're also exporting a mapping that shows which toast ID represents which grouped toasts.

I also changed the type ToastInputFields to accept rendered react components as title/text - this will prevent attempts to unmount react components from elements that are not displayed, thus causing React warnings.

The close-all button is an EUI feature, which we've started discussing here, probably not part of this PR.

What toasts get merged?

The exact merging strategy was not settled, and it's kind of a valve, where we trade off potential detail lost in toasts for the prevented noise in the toast floods. The current strategy is as folows:

 * These toasts will be merged:
 *  - where title and text are strings, and the same (1)
 *  - where titles are the same, and texts are missing (2)
 *  - where titles are the same, and the text's mount function is the same string (3)
 *  - where titles are missing, but the texts are the same string (4)

The base merge case is (1) & (2), after some discussion with @Dosant we decided to include (3) as a compromise, where we're still merging somewhat similar toasts, and extend the merging to ToastsApi::addError where all error toasts have a MountPoint as their text. We understand this might hide some details (e.g.: same titles, but very slightly different MountPoints as their text) but realistically this shouldn't really happen.

The ultimate future improvement will be (as suggested in the comments by @jloleysens) a sort of identifier to the toast, based on which we can group without fear of losing information. But this will require more work on all the call-sites.

Relates to: #161482

Checklist

Delete any items that are not applicable to this PR.

• Unit or functional tests were updated or added to match the most common scenarios

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[delanni]

@elasticmachine merge upstream

[delanni]

@elasticmachine merge upstream

[elasticmachine]

Pinging @elastic/appex-sharedux (Team:SharedUX)

[Dosant]

I've tested, not sure what is wrong, but toasts doesn't seem to always auto-close. Maybe there is a race condition inside the GlobalToastComponent or something like that...

This is what I did:

1. In Kibana changed advanced settings to make toasts life shorter:

2. Then went to visualize library > create > tsvb
3. Opened dev tools and blocked the data request:

4. Hit reload, see fetch errors as toasts and see them grouped 👍

The problem is that I expect the toasts to go away automatically after 5 seconds, but this doesn't happen

[delanni]

@elasticmachine merge upstream

[andreadelrio]

@ryankeairns @andreadelrio Thanks, I also quite like the outside-of-title counter. However, that will probably require an update form EUI to support this on a toast level. I can do this now with absolute positioning but I think it's more prone to errors on the long run, and harder to set up accessibility for. Also, it would relieve us from having to combine a counter with a title in a collapsed notification.

This is what I got with some basic CSS:

position: absolute;
top: -8px;
right: -8px;
scale: 1.2;

If you all like this approach for now, I am happy to shape this up, and commit to this, but I'll need some help from the accessibility aspect.

I think this is fine for the time being. I can create the issue requesting the implementation of the counter in the EUI repo. What do you think @ryankeairns ?

@delanni are you using EuiNotificationBadge in size "m"? I think that size will work nicely.

[ryankeairns]

Yes, this seems like the right approach: quick fix, then replace with EUI solution once available

Thanks all!

[pgayvallet]

LGTM

[Dosant]

@delanni, It seems that MountPoint still has some edge cases, I've tried the following two toasts with different texts set inside the mount point and those are grouped together. I don't think this is expected

const toast1 = () => {
      core.notifications.toasts.addInfo({
        title: 'Hello World!',
        text: (element) => {
          element.innerText = 'Toast text 1';
          return () => {};
        },
      });
    };

    const toast2 = () => {
      core.notifications.toasts.addInfo({
        title: 'Hello World!',
        text: (element) => {
          element.innerText = 'Toast text 2';
          return () => {};
        },
      });
    };

[Dosant]

We discussed in slack:

Currently when grouping we check that title + text are the same if they both are texts. But if there is a MountPoint in one of those, then it skips it and checks just either title or text. This means we might group toasts with completely different contents inside the mount point.

I think that the chance is quite small to miss something important there, but I’d personally prefer a safer route and to always not group if there is a MountPoint

It also seems that if we simplify this MountPoint thing , it means that we won’t ever have a MountPoint in a grouped toast. Then TitleWithBadge could be simplified to not worry about the MountPoint edge case

This is just my opinion and I don't think this is critical, so 👍

[delanni]

After syncing with @Dosant , I'm taking a more conservative take on what to merge/collapse. This commit contains the changes (5b6079d) can be reverted alone if we decide we'd want to collapse over MountPoints.

[lukeelmers]

Maybe with this change we could also finally close #67270?

[Dosant]

I retested with the latest changes, looks great 👍

[Dosant]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 5e9adf7

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
core	449	463	+14

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
core	145.4KB	145.5KB	+125.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
core	361.7KB	369.1KB	+7.4KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
@kbn/core-notifications-browser-internal	0	2	+2

Total ESLint disabled count

id	before	after	diff
@kbn/core-notifications-browser-internal	0	2	+2

History

• 💔 Build #145173 failed 9f13cd6
• 💚 Build #144993 succeeded 5b6079d
• 💚 Build #144917 succeeded f227247
• 💔 Build #144857 failed d421da76eaa77f1d933dc6d205a2a971816c2ddc
• 💚 Build #144659 succeeded 02d1b96

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @delanni @Dosant