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

Show toast on API errors #3561

Merged
merged 14 commits into from
Apr 2, 2021
Merged

Show toast on API errors #3561

merged 14 commits into from
Apr 2, 2021

Conversation

mariusandra
Copy link
Collaborator

@mariusandra mariusandra commented Mar 2, 2021

(PR hijacked by @paolodamico , ignore @mariusandra 's face next to the text)

Changes

Closes #3525. This PR brings back toast errors when something goes wrong, with a few key differences.

  • We will now show only one error at a time. Part of the problem we had before is that you would open a page and 4 toasts would pop up, which felt quite overwhelming.
  • We don't show errors for some specific endpoints that we know don't affect the user experience (e.g. preflight) or have special error handling (e.g. insights).
  • Show a more detailed error.
  • We will show a more detailed error message when a request fails due to a network error or blocked request.
  • Opens Papercups if available.
    image

Examples:


All feedback welcome!

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@timgl timgl temporarily deployed to posthog-add-happy-error-xq2fbr March 2, 2021 23:08 Inactive
@paolodamico
Copy link
Contributor

@mariusandra if you don't mind I'll hijack your PR, make some tweaks and get this out, want to get this in before the code freeze.

@mariusandra
Copy link
Collaborator Author

Please!

@paolodamico paolodamico requested a review from timgl March 24, 2021 21:44
@paolodamico
Copy link
Contributor

Ready for a review @mariusandra!

@paolodamico paolodamico marked this pull request as ready for review March 24, 2021 21:45
Copy link
Collaborator Author

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

Code looks good and considering I was already happy with how these toasts were before they were removed, I can only approve!

Because of this bias I'd say @timgl should also say if the approach works for him.

It's good that we can easily add actions to be ignored. This will help keep the interface noise free.

There's just one UX issue that I think should be solved, best illustrated by a screencast:

2021-03-25 17 29 34

If we show only one error, then I'd always like to see the last error shown to me, as that's probably the most urgent one.

@paolodamico
Copy link
Contributor

Agreed @mariusandra, I've updated it so that the last error is the one that's actually shown. Will wait for @timgl's input on the UX before merging.

@Twixes Twixes requested review from Twixes and removed request for Twixes April 1, 2021 18:18
@paolodamico paolodamico merged commit 3b72a3a into master Apr 2, 2021
@paolodamico paolodamico deleted the add-happy-errors branch April 2, 2021 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Many error notices are suppressed after limiting error toasts
3 participants