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

support border raduis for android #59

Closed
wants to merge 5 commits into from
Closed

support border raduis for android #59

wants to merge 5 commits into from

Conversation

yiky84119
Copy link

No description provided.

@omeraplak
Copy link

@DylanVann

Image of Yaktocat

@foggy1
Copy link

foggy1 commented Oct 16, 2017

Might be missing something but if I try to manually implement these changes I can't get a truly round image, the corners just get weirdly clipped.

@omeraplak
Copy link

omeraplak commented Oct 17, 2017

@foggy1 yes true but it's working different size.
Example, 60x60 (round: w-h half = 30) image can't round but 62x62 (32) it's working. i don't understand but i need this. I changed my picture sizes on my application and use the temp solution.

@yiky84119
Copy link
Author

i just test 60*60 radius 30, it works fine,could you show me how to reproduce the bug?

@omeraplak
Copy link

@yiky84119 Yes i can give more information few hours later. Maybe you just test on ios? Did you test on Android?

@omeraplak
Copy link

omeraplak commented Oct 17, 2017

@yiky84119
Usage
<FastImage style={profilePhoto} source={{ uri: imageUrl }} />

my image style,
profilePhoto: { borderRadius: 30, height: 60, width: 60 } as ImageStyle,

Android version: 5.1.1
React native: 0.48.4

@yiky84119
Copy link
Author

I have fix it, you can try again later.
npm install react-native-fast-image-nevo

@omeraplak
Copy link

@yiky84119 i checked and it's fixed. Thank you!

@dotansimha
Copy link

dotansimha commented Oct 28, 2017

@yiky84119
Trying to use your latest version of the fork.

Android seems a bit buggy, and the radius is incorrect.

using: style={{ width: 80, height: 80 }} and borderRadius={40}, in iOS the Image seems as circle, but in Android it seems like an ellipse and the top/bottom are cutted and not as perfect circle.
When adding to props resizeMode={'contain'} it seems a bit better and not does not cut, but still not a perfect circle. With resizeMode={'stretch'} I managed to get a perfect circle, but the image is to stretched.

Any idea why? Is there a way to make it work like the original react-native's Image? @yiky84119

@guoliang1206
Copy link

Does any new things about this issue?

@wkoutre
Copy link

wkoutre commented Nov 7, 2017

@guoliang1206 @dotansimha

My workaround for this is as follows:

const IMAGE = IOS ? (
      <FastImage
        resizeMode={FastImage.resizeMode.cover}
        source={picObj}
        style={Styles.profilePicStyle}
      />
    ) : (
      <View style={Styles.profilePicStyle}>
        <Image
          source={picObj}
          resizeMode={"cover"}
          style={[Styles.flex1, Styles.androidProfilePic]}
        />
      </View>
    );

The styles are just width, height, and borderRadius. IOS is just const IOS = Platform.OS === "ios"

@yiky84119
Copy link
Author

sorry guys, only support square picture now, i will fix later

@DylanVann
Copy link
Owner

DylanVann commented Jan 31, 2018

Thanks for all the help looking into this, sorry for the wait.

I think the wrapping view approach is probably the best because it:

  • Works on iOS and Android.
  • Doesn't cause any issues in common scenarios.
  • Will support other types of border radius when they are fixed in RN core on Android.
    • borderTopLeftRadius
    • borderTopRightRadius
    • borderBottomLeftRadius
    • borderBottomRightRadius
    • etc.
  • Currently these other types work on iOS but the android view masking code doesn't handle them. I'm thinking about taking a shot at fixing it.

screen shot 2018-01-30 at 20 43 02

A new version has been released that you can try out now.

@brunolemos
Copy link
Contributor

brunolemos commented Jan 31, 2018

I think the wrapping view approach is probably the best

@DylanVann I had border radius working on android before migrating to FastImage by using the combination View + borderRadius + overflow hidden. But it doesn't work with FastImage.

If you are talking about the comment above yours, it's basically using normal Image when it's android, which I don't consider a solution.

@DylanVann
Copy link
Owner

@brunolemos That's the solution being used in the image above. I think it does work, maybe you were trying before they implemented the masking on android.

@DylanVann DylanVann mentioned this pull request Feb 1, 2018
@brunolemos
Copy link
Contributor

brunolemos commented Feb 1, 2018

Upgrading react native made it work on Android :)

Before upgrading I was using <ImageBackground /> instead of <View><FastImage /></View> so maybe that is why borderRadius was already working on android.

@ezha86
Copy link

ezha86 commented Feb 18, 2018

can anyone confirm that border radius does work on android ?
for me seems it doesnt
"react-native": "^0.53.0",
"react-native-fast-image": "^2.2.5",

@CristiTr
Copy link

same here, not working on android device:

"react-native": "^0.53.3",
"react-native-fast-image": "^2.2.5",

@ezha86
Copy link

ezha86 commented Feb 21, 2018

i ended this by wrapping fast-image within view:

<View style={styles.avatarRadius}>
       <FastImage
                    style={styles.avatar}
                    source={{
                      uri: {uri},
                      priority: FastImage.priority.normal,
                    }}
       />
</View>
avatarRadius:{
    borderRadius: 25,
    width: 50,
    height: 50,
    overflow: 'hidden',
  },
  avatar: {
    flex: 1,
  },

@DylanVann
Copy link
Owner

DylanVann commented May 4, 2018

@ezha86 @CristiTr The current version of react-native-fast-image is v4.0.8. It will work if you upgrade.

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.

None yet

10 participants