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

Add addError function to toastNotifications #32187

Merged
merged 30 commits into from
May 29, 2019

Conversation

timroes
Copy link
Contributor

@timroes timroes commented Feb 28, 2019

Summary

This PR adds a addError method to the toastNotification service, to show a toast for an error, which allows expanding that error in a modal to see the full stacktrace. This PR also uses that new service in Discover, so you can test it (e.g. by creating an invalid filter in Discover).

Also it will add back using the appropriate configuration settings from the ui settings to configure the actual toast timeouts (this was lost at some point when moving over to the toast notifications I assume).

cc @cchaos

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@timroes timroes removed the blocked label Apr 17, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover
Copy link
Contributor

Depend on what gets merged first, we should move the modal functionality to use the new openModal method being added to the OverlayService in #36057

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover
Copy link
Contributor

@timroes After merging master on this, it doesn't look like the exception handlers in the Discover contoroller are getting called when there is a syntax error. I don't know enough about how this code works to know where to begin to look.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@LeeDr LeeDr added v7.3.0 and removed v7.2.0 labels May 23, 2019
@LeeDr
Copy link

LeeDr commented May 23, 2019

Bumped to 7.3. This doesn't appear to be a bug fix and we're passed Feature Freeze for 7.2

@timroes
Copy link
Contributor Author

timroes commented May 23, 2019

@joshdover Could you describe what you did in discover to trigger them. Entering e.g. :foo:bar into the query bar as Caroline did in her screencast should have revealed them?

@joshdover
Copy link
Contributor

@joshdover Could you describe what you did in discover to trigger them. Entering e.g. :foo:bar into the query bar as Caroline did in her screencast should have revealed them?

I was able to get that working, guess the string I was using wasn't triggering an error anymore. 🤷‍♂

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover merged commit 58ef3a3 into elastic:master May 29, 2019
@joshdover
Copy link
Contributor

Backport pending the backport of #36611 to avoid multiple conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.3.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants