-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[Android] Add missing onError event to Image #9416
[Android] Add missing onError event to Image #9416
Conversation
Sync with upstream
Sync with upstream
By analyzing the blame information on this pull request, we identified @andreicoman11 and @Bhullnatik to be potential reviewers. |
@webzepter updated the pull request. |
Hi there! Thanks for the PR. This was actually intentionally left out because Fresco doesn't fully support it yet, see: #3791 (comment) |
@brentvatne Looks like we emit an react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.java Line 214 in bcf48e7
This PR would make it possible to handle it in JS, correct? |
@mkonicek yes |
It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @rigdern as a potential reviewer. Could you take a look please or cc someone with more context? |
@mkonicek - the problem is that the behaviour is inconsistent with iOS-- once Fresco has similar error semantics this is easy. I don't follow that project closely and I'm not sure what the current state is. if we merge this we have to keep in mind, and document, that the callbacks fire in different situations on each platform. |
Hey, so chatting with @brentvatne and based on the discussion in #3791 (comment) and this thread it seems like this current behavior is working as intended, at least as long as Fresco's behavior makes it impossible for iOS and Android to behave consistently. I'm going to close this pull request, but if it is indeed possible to have this with consistent behavior across iOS and Android, feel free to reopen. |
This PR adds
onError
prop toImage
on Android. Closes #7440Motivation:
We have image feed and show refresh button when image loading had failed.
Usage of
onError
event is necessary to track loading failures