Skip to content

Conversation

@mrousal
Copy link

@mrousal mrousal commented Mar 2, 2018

No description provided.

_height = height;
}

/// <summary>
Copy link
Contributor

@reseul reseul Mar 2, 2018

Choose a reason for hiding this comment

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

Small alignment #Resolved

return "topLoadStart";
case OnLoad:
return "topLoad";
case OnLoadEnd:
Copy link
Contributor

@reseul reseul Mar 2, 2018

Choose a reason for hiding this comment

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

What do we do with this one, then? Doesn't seem to be used.
Semantically "onLoad" marks the end, anyway.

[edit]: Based on the comments of the JS counterpart, onLoadEnd should be called in both cases (error & success).
So order for success:

  • onLoadstart, onLoad, onLoadEnd
    for failure:
  • onLoadStart, onError, onLoadEnd

(not sure about the order of onLoadEnd vs onXXX, perhaps peeking into Android code would inspire somehow.) #Resolved

Copy link
Author

@mrousal mrousal Mar 5, 2018

Choose a reason for hiding this comment

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

updated to fire both events as on android #Resolved

if (_eventType == OnError)
{
eventData = new JObject();
eventData.Add("error", _error);
Copy link
Contributor

@rozele rozele Mar 2, 2018

Choose a reason for hiding this comment

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

Use object initializer:

eventData = new JObject
{
    { "error", _error }
};
``` #Resolved

@rozele
Copy link
Contributor

rozele commented Mar 2, 2018

You should consider sending a similar PR to send the error message on Android.
https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/views/image/ImageLoadEvent.java #Resolved

@reseul
Copy link
Contributor

reseul commented Mar 2, 2018

@rozele Actually they dispatch from here: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.java

And yes, it's first onload/onerror, THEN onLoadEnd. #Resolved

@mrousal mrousal force-pushed the michalr/wireImageOnErrorEvent branch from f4ccb54 to 278adce Compare March 5, 2018 09:46
Copy link
Contributor

@reseul reseul left a comment

Choose a reason for hiding this comment

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

:shipit:

@reseul reseul merged commit 63322df into microsoft:master Mar 5, 2018
chukcha-wtf pushed a commit to axsy-dev/react-native-windows that referenced this pull request May 10, 2018
# Conflicts:
#	ReactWindows/ReactNative.Shared/Views/Image/ReactImageLoadEvent.cs
ladipro pushed a commit to ladipro/react-native-windows that referenced this pull request May 16, 2018
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.

4 participants