Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[toasts] Improved Error Toast Experience #67270

Closed
Dosant opened this issue May 25, 2020 · 34 comments
Closed

[toasts] Improved Error Toast Experience #67270

Dosant opened this issue May 25, 2020 · 34 comments
Labels
enhancement New value added to drive a business result impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort papercut Small "burr" in the product that we should fix. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) usability

Comments

@Dosant
Copy link
Contributor

Dosant commented May 25, 2020

Updated scope

We have seen a few different instances of folks complaining about repeated error messages being shown in toasts. While we don't think there's a viable way to automatically deduplicate errors from the core toasts service, we do think there's an opportunity to introduce a new edit method to that toasts API.

This would allow plugins to store a reference to a toast which could later be edited (while possibly also extending its lifetime). Plugins could collect repeated errors that are being thrown with the same message, and instead of calling add for each new error, they could call add the first time, and then on subsequent matching errors, could edit the same toast to make a more informative message (something like Received 5 errors when retrieving an index pattern: some-error-message, where the number 5 could be incremented with each new error).


Original request

We use notification's service toasts to inform users about errors and warnings.
Sometimes those errors come from deep inside of codepaths without any orchestration before getting into notification service.
This could lead to following experience:
Screen with error toasts not fitting inside one screen
)
Screen with same warning shown 3 times

Initially with @elastic/kibana-app-arch we wanted to fix this particular issue #62649 on consumer side by adding errors debounce logic before calling notification service. But then during the discussion we agreed that this looks like a common issue that could be addressed inside notification service.

Some quick ideas we had:

  • Have useful "Dismiss all" button to quickly close all of those notifications
  • Have logic inside notifications service to group similar notifications (define similar)
  • Implement some kind of show last N notifications and have "show more" button
@Dosant Dosant added discuss Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc enhancement New value added to drive a business result needs design labels May 25, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-design (Team:Design)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@pgayvallet
Copy link
Contributor

IMHO:

Have useful "Dismiss all" button to quickly close all of those notifications

Makes sense, most of the toast libraries provide such an option. Design support would be necessary though.

Have logic inside notifications service to group similar notifications (define similar)

Seems like out of the scope / responsibilities of the toast core service to me. We could eventually regroup toasts defined as 'similar' by the API consumers (with something like 'category' field maybe, or something else), but I don't see it being a default / automatic behavior of the service.

Design support would also be necessary here.

Implement some kind of show last N notifications and have "show more" button

Having more toasts displayed than the screen height allows is imho an anti pattern of what toast notifications should be. As a user, I will personally NEVER want to scroll / expand a toast notification list, as that are supposed to be non-mandatory, volatile information messages.

Some quick ideas we had:

From the issue example, wouldn't basic toast orchestration from the consumer using ToastsApi.remove before adding a similar toast answer your need?

@joshdover joshdover added REASSIGN from Team:Core UI Deprecated label for old Core UI team and removed Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels May 26, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core-ui (Team:Core UI)

@Dosant
Copy link
Contributor Author

Dosant commented Jun 25, 2020

I closed #69697 as duplicate,
Carrying over from there nice suggestions from @philippkahr:


I can think of two improvements:

  1. there should be some close all toasts button somewhere.
  2. Stack toasts notifications and add close all something like this:

Untitled
When stacking we could go even further a stack similar errors into a stack. E.g. essagg error vs timeline error toasts.

@myasonik
Copy link
Contributor

myasonik commented Jun 25, 2020

So many toasts really feels like abuse of what toasts are meant to be... It really seems like we're trying to screw in a nail or hammer a screw or otherwise misuse our metaphors tools.

To actually solve our Toast problems, we need a notification center of some sort and not continue to overload Toasts with more actions and complexity.

@gimmic
Copy link

gimmic commented Sep 16, 2020

Just came to look if there was an issue related to improving toasts after for the Nth time I spammed myself with toast errors on a dashboard.

It's really annoying to have to acknowledge each toast on a dashboard that can generate 10+ of them on a single syntax error query.

dismiss all where are you?

@cchaos
Copy link
Contributor

cchaos commented Sep 16, 2020

I will carryover my comment from #69697

Kibana should consider not duplicating the same error messages and/or grouping all similar errors into one toast.

The previously mentioned potential solution is problematic because it could possibly obscure more "important" notifications. The user would have dismiss each one to find out which one is important. It's better to display them all with a simple scroll mechanism to easily scan for any that need attention.

What I'm seeing from the original screenshots are that 5 of the 7 (and all 3 of the second screenshot) are the same message (no extra information provided, no action to be taken). There should only be a single toast of it's kind no matter where the errors are coming from or how many times it occured.

@marius-dr
Copy link
Member

marius-dr commented Sep 29, 2020

We could probably change from "similar" to identical. That's the most common case encountered: opening a saved dashboard and each visualization on it errors out due to something like a missing index pattern. This will shove however many toasts as visualizations we have with the exact same message. In this case having them grouped up makes sense.

Just like the comment above, I found this issue when I wanted to open my own due to how bad it is for the user when this happens.

Having a good user experience when something goes bad is just as important (regardless if they are the ones that broke something) because it help debugging so much more. Right now I feel it only hinders the user.

@lukeelmers
Copy link
Member

Reviving this thread since it has been awhile, and we have been hearing more complaints recently about duplicated toast messages cluttering peoples' screens.

Overall I agree with pretty much all of @pgayvallet's comments above #67270 (comment)

re: deduplication, I want to go down a bit of a rabbit hole here:

There should only be a single toast of it's kind no matter where the errors are coming from or how many times it occured.

Totally agree with @cchaos on this, however we'd need to think through the criteria for how to determine this. Because core.notifications.toasts can accept most of the EuiToast props, and EuiToastProps extends HTMLAttributes, there are a lot of potential configurations somebody could provide when adding a toast. So if we were to add some logic to dedupe based on title/text alone, we'd need to make sure that any props a consumer of the service supplies have actually been added if a duplicate already exists (for example imagine providing an onClose prop which is never called because a duplicate toast exists).

So following Pierre's suggestion of using remove, I think we'd either need to:

  1. have consumers explicitly check if a duplicate exists & then remove it before adding a new one
  2. create a utility of some kind to make (1) easier, e.g. if (!similarToastExists(myToastInput)) { ...add my toast })
  3. internally in the toasts api we remove any duplicates (using title & text to test for matches) when adding a new one
  4. probably other options I'm not thinking of, like checking equality of all provided props (which could still result in dupes in the case where a toast has the same title/text but slightly different props)

TLDR, I think we should scope the issue to (1) "dismiss all" functionality, and (2) some sort of deduplication logic.

IMHO notification groupings & "show more" are things we should avoid for now if possible, especially because there ideally shouldn't be so many toasts that these types of features are necessary in the first place.

Would be interested to hear if @elastic/kibana-design has any other thoughts on this

@lukeelmers lukeelmers added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Sep 23, 2021
@ryankeairns
Copy link
Contributor

ryankeairns commented Sep 27, 2021

+1 to focusing on the 'Dismiss all' with deduplication.

If they're grouped, then a button within the toast would likely do the job, but you may then also need an option to show them all or, at minimum, state how many times it occurred. Otherwise, the dismiss all would be rather unclear (i.e. it looks like a regular, single toast with this extra 'dismiss all' button).

Screen Shot 2021-09-27 at 11 52 04 AM

I don't see how this works without the grouping capability unless we add something 'outside' the toasts to 'Dismiss all'. Sorta like this, replacing 'Show less' with 'Dismiss all'. That said, we'd then be dismissing all toasts regardless of whether they are similar or related. This may be desirable in its own right, but is a nuance worth noting.

Screen Shot 2021-09-27 at 11 55 13 AM

... but note that this gets outside of existing patterns and EUI support, potentially.

PS - I wish these were my yoga and new couch reminders. This was borrowed from the internetz.

@lukeelmers
Copy link
Member

I don't see how this works without the grouping capability unless we add something 'outside' the toasts to 'Dismiss all'. Sorta like this, replacing 'Show less' with 'Dismiss all'. That said, we'd then be dismissing all toasts regardless of whether they are similar or related. This may be desirable in its own right, but is a nuance worth noting.

I don't think this would be too bad functionally, but the more I think about it, maybe we should just focus on the deduplication piece alone, as in theory that could also mitigate the need for "dismiss all". I can't think of many instances I've seen where the screen is filled with more than maybe 2 or 3 toasts that are actually different.

@getkub
Copy link

getkub commented Sep 28, 2021

Another idea is to have a Drop-Down (Or Drop-Up) kind of toast whereby the notificaiton light can be red or amber etc. and click on it will drop down the errors/warnings. Then 'dismiss all' && 'x'(close sign) for each.

@lukeelmers
Copy link
Member

Discussed this with @elastic/kibana-core today. Overall we are worried about the complexity of automatic deduplication, for the reasons I enumerated above. It's not quite as simple as it seems on the surface if we want to handle this globally.

We think that a better possible solution could be introducing some sort of edit API for toasts, where you could store a reference to a toast which could later be edited (while possibly also extending its lifetime). This would allow plugins to collect repeated errors that are being thrown with the same message, and instead of calling add for each new error, they could call add the first time, and then on subsequent matching errors, could edit the same toast to make a more informative message (something like Received 5 errors when retrieving an index pattern: some-error-message, where the number 5 could be incremented with each new error).

In general our hope would be that features like "dismiss all" or toast grouping would become less necessary over time if we are adding logic like this to reduce the clutter.

This proposal would require a bit more thought though -- in the meantime, the fastest way to resolve this issue is going to be to throttle the calls to add new toasts directly from the data plugin, which seems to be the primary source of the issues that have been reported anyway.

@lockewritesdocs
Copy link

This issue surfaced in #78500, where security warnings display continuously and users cannot disable them. The link to documentation is not a hotlink (this really only applies to the toast messages), so users are forced to copy + paste the link.

@cjcenizal
Copy link
Contributor

cjcenizal commented Nov 3, 2021

Ok, what if we allow plugins to specify an identifier to deduplicate the toasts?

This was my thought as well. As a consumer, I'd like to specify whether a toast is groupable and provide a groupId, to identify which toasts can be collapsed into a group, knowing that the toasts service will do the rest for me. This provides a simple interface and encapsulates complexity (ID comparison, editing vs. removing toasts, surfacing the count to the user).

By encapsulating the complexity of how this manifests as behavior within the toasts service, implementers gain freedom to iterate towards the best UX.

In terms of implementation, I'd prefer to see a toast with a button that says "See 5 more". If this is too awkward to fit into the current toast design, an alternative would be a small counter badge, "5", that displays a tooltip that explains, "See 5 more". When clicked, this would open a modal that enables the user to analyze the toasts by doing things like cycling through them one at a time, searching them, and dismissing them individually or in bulk.

@cjcenizal
Copy link
Contributor

Cross-referencing https://discuss.elastic.co/t/spammed-by-deprication-warnings-on-scripted-fields/286032. User complained of being spammed by toasts. The toasts themselves were helpful, but the quantity and frequency was not.

@lukeelmers
Copy link
Member

Just had a sync with @ryankeairns and @cchaos about this issue, and here is a quick summary of the design team recommendations for the UX. (Some of these points have been brought up already, I'm just synthesizing them here):

  1. In a perfect world, we shouldn't need a concept of "grouping" or "see x more" in our toasts system.
    • If we start needing that, it's a sign that we are probably abusing the toasts system in the first place.
  2. Many of the current uses of toasts would make more sense rendered as inline errors in the application.
    • For example, if a dashboard fails because of a missing index pattern, instead of seeing dozens of toasts, each panel should render an inline error.
  3. Rather than showing individual toasts for duplicate errors, the preferred ux would be to show the number of times a message has appeared inline in the toast.
    • This number could then be incremented whenever a new/duplicate error occurs.

We propose achieving (3) by doing the following:

  1. Updating docs with UX best practices for toasts (autogenerated ToastApi docs here).
  2. @elastic/kibana-design can provide a recommendation for how a toast "counter" would look, since this is a convention we would want to be implemented consistently
    • Initially we could implement support for this new counter prop in Kibana directly, and then eventually it could be added to EUI
  3. @elastic/kibana-core team could then expose the counter on the toast config, and implement the proposed edit API
    • This would require sorting out answers to the questions Pierre outlines above (e.g. how to deal with TTL, ordering, etc). IMHO these are solvable, but would require discussion.
    • We could optionally introduce a deduplication identifier as Mikhail suggests, or consumers can decide on their own dedupe logic
      • Introducing a deduplication identifier might be nice, because then we could easily create a helper for plugins to use to make this easier, e.g. createToastIfNoneExists(core, someToastConfigWithIdentifier)

Thoughts? Next steps in this case would be for Core team to discuss implementation details, while Design team ideates on how this should look in the toast UI.

@getkub
Copy link

getkub commented Nov 16, 2021

Just had a sync with @ryankeairns and @cchaos about this issue, and here is a quick summary of the design team
....
Thoughts? Next steps in this case would be for Core team to discuss implementation details, while Design team ideates on how this should look in the toast UI.

Still not sure if the original problem (raised via support team) addressed here. The problem was whichever method you group, should NOT come in the 'Usable Space' of the UI. The trouble with current version is in large environment like ours, if there is a glitch, 100's of various errors happen and fills up the entire right hand-side of the screen (within usable space) from bottom to top and has to be closed one by one. So any UX solution should ensure that errors are contained towards a right top corner with just a count (eg as in Splunk or GitLab notifications) where the user can then click on it and see the errors. As a customer we don't need the entire screen to be filled-up with errors/pop-ups

@mshustov
Copy link
Contributor

@lukeelmers I'm curious why option (2) was ruled out? It seems to be more aligned with (1) than (3).

@pgayvallet
Copy link
Contributor

pgayvallet commented Nov 18, 2021

Initially we could implement support for this new counter prop in Kibana directly, and then eventually it could be added to EUI

I doubt we can do that in Kibana alone, even temporarily, given we're using the EUI components directly.

public render() {
return (
<EuiGlobalToastList
aria-label={i18n.translate('core.notifications.globalToast.ariaLabel', {
defaultMessage: 'Notification message list',
})}
data-test-subj="globalToastList"
toasts={this.state.toasts.map(convertToEui)}
dismissToast={({ id }) => this.props.dismissToast(id)}

I think we need this feature to be implemented upstream before being able to use it. (but hopefully @ryankeairns or @cchaos can prove me wrong here?)

Also I agree with @mshustov about:

I'm curious why option (2) was ruled out? It seems to be more aligned with (1) than (3).

Given #67270 (comment) and #67270 (comment), it seems that we all agree that toasts aren't really the correct solution to surface those specific errors and warnings, so it feels like we're trying to find the quickest workaround to this issue.

To be even more direct, I have to ask:

We have seen a few different instances of folks complaining about repeated error messages being shown in toasts

Did we had reports of such 'toast flood' outside of the data/dashboard/vis tuple, or is this whole issue opened only to address this specific problem?

Because if that's the case, having a button in the action menu opening a flyout with this list of messages (instead of using toast notifications) seems like it would answer the request from support

The problem was whichever method you group, should NOT come in the 'Usable Space' of the UI

while allowing way more control over it from the owning application (dashboard or vis), like being able to update them dynamically or remove them after the next data fetch.

@lukeelmers
Copy link
Member

I'm curious why option (2) was ruled out? It seems to be more aligned with (1) than (3).

Sorry, I should have been more clear -- that list was not a list of options that are mutually exclusive, but rather a list of guidance we want to give to teams around toast usage. So the TLDR would be:

  • Don't create so many toasts that they fill up a screen -- we shouldn't need a "dismiss all" or "grouping" feature if we are using toasts responsibility.
  • When possible, try to inline your error messages in a relevant part of the application.
  • If you must use a toast, and there's a risk of creating duplicated toasts, you should deduplicate those toasts, with the preferred UX being including a counter that increments when a duplicate is received.
    • This last point is the only part where Core team would need to be involved, to provide support for a counter feature via the Toasts API

I doubt we can do that in Kibana alone, even temporarily, given we're using the EUI components directly.

This was @cchaos suggestion I think, so if there isn't a way to add it in Kibana then yeah, we'd need to wait for upstream. I suppose it might depend on what the recommended design ends up being.

Did we had reports of such 'toast flood' outside of the data/dashboard/vis tuple, or is this whole issue opened only to address this specific problem?

Every complaint that I recall hearing on this topic ultimately had to do with data/dashboard/vis. I believe "toast flood" was the whole point of opening this issue in the first place. TBH I was not aware of the "should not take up Useable Space" concern and whether this is why @Dosant opened the issue -- while that would be partially mitigated with reducing the # of toasts, that also sounds like a larger question around the use of toasts in general.

I'll also add that because this mainly has to do with data/dashboard/vis, which should already be getting resolved via #109948 and #116686, I see this particular issue as a longer-term enhancement, which is why I re-scoped it. The toast flood should be fixed independent of this issue, so for folks following along here hoping for a reduction in the # of toasts, I'd encourage you to follow those linked issues instead, as they will be the short term solution to the problem.

Because if that's the case, having a button in the action menu opening a flyout with this list of messages (instead of using toast notifications) seems like it would answer the request from support

The problem was whichever method you group, should NOT come in the 'Usable Space' of the UI

while allowing way more control over it from the owning application (dashboard or vis), like being able to update them dynamically or remove them after the next data fetch.

This sounds like a viable option to me too, but I'd defer to @cchaos and @ryankeairns design recommendations here. When we chatted they had favored the incrementing counter approach, but we didn't talk about flyouts as a possibility.

@ryankeairns
Copy link
Contributor

ryankeairns commented Nov 19, 2021

I'm looking at this as a thing to be improved iteratively. Toasts are a globally used pattern that will take more time and thought to potentially rework and develop new guidance around. What we're proposing here (Luke's comments) is to do something in the interim to reduce the noise and to provide better guidance on how to live within what currently exists.

That does not preclude us from continuing on with other more significant changes, but the scope of that sort of effort would necessitate a larger discussion, research, design exploration, project definition, etc. The works :)

Along those lines, we have the notification center lurking and that would be an opportune time to think of how we handle error messages, globally, and whether or not things being suggested in this thread fit within that UX.

@cchaos
Copy link
Contributor

cchaos commented Nov 22, 2021

I doubt we can do that in Kibana alone, even temporarily, given we're using the EUI components directly.

The EuiToast component is pretty flexible with it's prop types. For instance title accepts ReactNode which can be a string or a whole component. Here's a codesandbox example showing how to extend the title prop. https://codesandbox.io/s/epic-snowflake-16yyg?file=/index.js

There are 2 main reasons we mention doing this in Kibana first:

  1. Quicker turn around. The Kibana teams can work on implementation of this now, without waiting for the full EUI development, publish, upgrade cycle.
  2. Quickly test viability. Adding this ability in Kibana first will also inform us that this is definitely the right direction, or at least a piece of it before we go through the process of modifying the EUI component itself.

If no. 2 is in fact validated early, we could start the EUI process in tandem, but don't want to be a blocker to getting this through to customers.

@alexfrancoeur alexfrancoeur added usability papercut Small "burr" in the product that we should fix. labels Jun 22, 2022
@alexfrancoeur
Copy link

@VijayDoshi @ghudgins this may be a good first papercut to prioritize

@VijayDoshi VijayDoshi changed the title [toasts] Add edit API to allow for modifying an existing toast message. [toasts] Improved Error Toast Experience Sep 8, 2022
@mbarretta
Copy link

+1 on this - common complaint

@jasonborchardt
Copy link

+1 on this behavior. The flood of errors (which in many cases aren't even necessary but thats another topic) cause the entire right side of the screen to be unusable. This is especially troublesome when trying to manipulate data with a lens. The lens editor is unusable entirely.

@pgayvallet pgayvallet added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label May 10, 2023
@elasticmachine
Copy link
Contributor

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

@petrklapka
Copy link
Member

This could be handled by the "notification center" feature that has been discussed but not green lit yet. A preferred simpler approach would be a new component that handles buffering/debounce/throttling notification in a queue and give it the same programatic interface as the current toast notification API. This would allow us to just swap the component import path, or use a different service to minimize code impact. To the user it would present a "group" of stacked toasts, collapsed in some way, and allow for cycling through them/dismissing them in bulk.

@sixstringcode , this is between 2 and 6 weeks worth of work by current guess. Do we want to prioritize this?

@Dosant
Copy link
Contributor Author

Dosant commented Jul 27, 2023

We've implemented basic automatic deduplication in #161482 and have some follow-up work planned in #162646

@Dosant Dosant closed this as completed Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort papercut Small "burr" in the product that we should fix. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) usability
Projects
None yet
Development

No branches or pull requests