RealOrangeOne / react-native-mock

A fully mocked and test-friendly version of react native (maintainers wanted)
MIT License
571 stars 153 forks source link

Add `repeat` (RN 0.30+) and `center` (RN 0.31+) to Image resizeMode p… #93

Closed jasonfma closed 8 years ago

jasonfma commented 8 years ago

…rops

jasonfma commented 8 years ago

These properties are iOS only. I'm not sure how to enforce that here. Also I'm not sure how to make these props available per RN version.

Any suggestions welcome.

RealOrangeOne commented 8 years ago

The only way I can think of doing this would be by appending them depending on the value of platform.os, but I don't think that's possible as the proptypes are defined at class definition. Correct me if i'm wrong?

jasonfma commented 8 years ago

I'll give it a shot.

As for the RN version thing... maybe we should tie the version to RN version. So we could split this PR into two. repeat goes into 0.30.0 with a peer dependency on RN 0.30.0 and center is 0.31.0 with a peer dep on RN 0.31.0.

RealOrangeOne commented 8 years ago

I'm not really concerned with previous versions of react-native, once the project slows down with its breaking changes, then i'll keep track. I do need to make a list of compatable versions at some point though

jasonfma commented 8 years ago

Ok, I am now pushing the iOS props based on Platform.OS. Please take a look.

RealOrangeOne commented 8 years ago

Correct me if i'm wrong, but You don't seem to be using your new imageresize props thing?

jasonfma commented 8 years ago

At the bottom of Image.js it uses it for one of the statics:

...
resizeMode: ImageResizeMode,
...

But I think it should be used for both. Let me make the change.

RealOrangeOne commented 8 years ago

Perfect. Does image also only allow those resize modes? I fear it may cause problems

jasonfma commented 8 years ago

Yeah looking at their docs for Image.style:

resizeMode ReactPropTypes.oneOf(Object.keys(ImageResizeMode))

RealOrangeOne commented 8 years ago

In that case, I think it's fine to merge this!