- Notifications
You must be signed in to change notification settings - Fork928
feat: add global notification component#996
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedApr 13, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #996 +/- ##==========================================+ Coverage 66.82% 66.99% +0.16%========================================== Files 241 250 +9 Lines 14549 14819 +270 Branches 115 131 +16 ==========================================+ Hits 9723 9928 +205- Misses 3854 3911 +57- Partials 972 980 +8
Continue to review full report at Codecov.
|
I'm nervous about adding this, as I've seen our global notifications lead to errors that aren't attached to any user action or component. An example is the connection failures we display in v1. Does anyone else feel similarly? |
BrunoQuaresma commentedApr 13, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@kylecarbs IMO this component itself is not the issue but how we were using it in V1. We will be cautious about the usage of this component for sure. It is also important to share more context:
|
Uh oh!
There was an error while loading.Please reload this page.
Something we could right now is only port |
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
A lot of this code can be cleaned up later --> I'll create a ticket to refactor it mentioning XService port.
Thanks for porting this@BrunoQuaresma
Closes#992