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

Throttle toast storms related to saving/restoring URL state #154792

Merged
merged 5 commits into from
Apr 13, 2023

Conversation

lukasolson
Copy link
Member

Summary

Resolves #153073.

Throttles toast errors related to restoring/saving URL state.

Before:

image

After:

image

Checklist

@lukasolson lukasolson added Feature:Dashboard Dashboard related features Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.8.0 labels Apr 12, 2023
@lukasolson lukasolson requested a review from a team as a code owner April 12, 2023 00:21
@lukasolson lukasolson self-assigned this Apr 12, 2023
@kibanamachine kibanamachine added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Apr 12, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@lukasolson lukasolson added loe:small Small Level of Effort impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Apr 12, 2023
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id before after diff
kibanaUtils 69.7KB 69.8KB +94.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 433 436 +3

Total ESLint disabled count

id before after diff
securitySolution 512 515 +3

History

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

cc @lukasolson

Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good and work as expected - however, it would be nice to reduce this to a single toast.

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toast storms while reviewing this PR:
Bildschirmfoto 2023-04-13 um 09 32 12
@mattkime agreed, just one would be even better, could be a potential follow up (those toast a thrown at 2 different parts of our code, one way to deal with it could be throwing a custom error, and don't use this error to start a toast at one of code locations)

@lukasolson lukasolson merged commit dc1baa9 into elastic:main Apr 13, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Dashboard Dashboard related features Feature:Discover Discover Application impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover] Toast storm occurs when copying URL with state:storeInSessionStorage enabled
6 participants