-
Notifications
You must be signed in to change notification settings - Fork 614
Fixes App crash when left for a long time #160
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
Conversation
ericallam
left a comment
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! Made a few comments and once those are addressed this can be merged in
|
Thanks for the review @afzalsayed96! @tobihans do you think you could address the issues they raised in the PR? |
|
Yes @ericallam, I just submitted the changes. To be honest, I'm new to React (long time Vue dev :-) ). But I'm learning a lot from @afzalsayed96 review. Do let me know if this resolves it. Thanks |
|
Rest looks good. Please make sure this runs well as I haven't tested this myself. |
|
Yes, it does, I have tested it. Below is an attached demo. Thanks sample.mp4 |
This closes #95.
The cause was a failing request for URL preview once the user is disconnected.
The current state of error handling with Remix doesn't allow to handle network errors manually when using
useFetcher.To workaround this, we detect if the user is online before loading the preview. In addition to that, errors are catched on the loader and an error response is returned.
/claim #95