-
Notifications
You must be signed in to change notification settings - Fork 293
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
Fix loading states when report errors #5000
Conversation
f6941c1
to
52d510c
Compare
09d19e8
to
955f12b
Compare
Size Change: +6.08 kB (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
f611a93
to
955f12b
Compare
@kuasha420 looks like Storybook is still broken here according to the check |
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.
Thanks @kuasha420 – this is looking good, just a few comments left to resolve; almost there I think 👍 Let me know if you have any other questions.
b5167d1
to
11d8845
Compare
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.
Thanks @kuasha420 – this looks good, just a few final tweaks to get this over the line such as applying the changes to the tests from the last review consistently and one comment about an intercepted request that I think we want to keep.
I just pushed a few changes that should address these last few things.
Confirmed that these changes fix the problem testing with permissions errors with Analytics and SC. |
@kuasha420 I can't approve this PR because it's my own 😄 Can you take a quick look over my changes and approve if they look good to you? |
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.
Thank you @aaemnnosttv for all the help with this one! Looks much cleaner now. Nice. 👍
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.
Looks good to me as well; moving to QA 👍🏻
Summary
Addresses issue:
Relevant technical choices
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist