-
Notifications
You must be signed in to change notification settings - Fork 514
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
Prevent infinite load when avatar upload fails #8881
Prevent infinite load when avatar upload fails #8881
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
When a user attempts to upload a non-permitted image, the system currently throws an error and closes the entire pop-up. This should not be the expected behavior. Instead, the user should remain on the same pop-up, where the error message is displayed, allowing them to re-select and upload an appropriate image. |
Made some changes and this is how it works now: Recording.2024-10-23.144048.mp4 |
try { | ||
await handleUpload(selectedFile, () => { | ||
setSelectedFile(undefined); | ||
setPreview(undefined); | ||
setPreviewImage(null); | ||
}); | ||
} catch (e) { | ||
console.error(e); | ||
} finally { | ||
setIsCaptureImgBeingUploaded(false); | ||
setIsProcessing(false); | ||
}); | ||
} |
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.
These changes are not required, just calling onError() in the handler should be enough
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.
Done, I removed them.
Is it ok now?
👋 Hi, @nithish1018, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
@nithish1018 can you try updating the profile pic instead of facility cover image? |
Thank you, I added the fix for it |
clear merge conflicts; rest lgtm |
LGTM |
@nithish1018 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist