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

Improve identity error message. #2623

Merged
merged 4 commits into from
Oct 18, 2021
Merged

Conversation

amass01
Copy link
Member

@amass01 amass01 commented Sep 30, 2021

This adds a new react error boundary to handle global identity error.
The identity error can be thrown in various modals and error boundaries
is react's way to handle such errors.

Implementation:

  • Add a new IdentityErrorBoundary to catch global identity error.
  • Add a new useCarsh hook in order to crash inside the component tree
    when an identity error is thrown from an async action.
  • Add a new utils function isIdentityError and use it cross the app.

React error boundaries
Closes #2617.

@amass01 amass01 marked this pull request as draft September 30, 2021 20:12
@amass01 amass01 changed the title [wip] Improve identity error message. Improve identity error message. Oct 16, 2021
@amass01 amass01 marked this pull request as ready for review October 16, 2021 11:14
@amass01
Copy link
Member Author

amass01 commented Oct 16, 2021

Screen recording of the global error handling:

Screen.Recording.2021-10-16.at.14.03.34.mov

Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

tACK of firefox

I really don't like the way react handles these global errors. It trains the developer to just hit esc on errors in order to bybass the error overlay. There's not much we can do about it though. The react devs feel strongly about it and this is not something that is going to change.

https://stackoverflow.com/a/47400249

@lukebp
Copy link
Member

lukebp commented Oct 16, 2021

Needs code approval from @tiagoalvesdulce.

Copy link
Member

@tiagoalvesdulce tiagoalvesdulce left a comment

Choose a reason for hiding this comment

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

ACK

@tiagoalvesdulce tiagoalvesdulce merged commit 95556d5 into decred:master Oct 18, 2021
@amass01 amass01 deleted the identityerror branch October 18, 2021 23:10
@lukebp
Copy link
Member

lukebp commented Nov 11, 2021

Updated error message.

image

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.

Improve identity error message.
3 participants