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

border radius doesn't work #2

Open
praneeth-alla opened this issue Feb 2, 2017 · 17 comments
Open

border radius doesn't work #2

praneeth-alla opened this issue Feb 2, 2017 · 17 comments
Assignees
Labels

Comments

@praneeth-alla
Copy link

I am trying to set border radius for WebImage but it is not working. It works for a regular Image element. Below is how I am setting the border radius. Can anyone please help me to set the border radius.

{ console.log('WebImage onError handler: ', e) }}/> cardThumbnailContainer: { overflow:'hidden', margin:0, borderTopLeftRadius:7, borderBottomLeftRadius:7, backgroundColor: '$light.background', height:"$imageHeight" }, cardThumbnail: { width:"$imageWidth", borderTopLeftRadius:7, borderBottomLeftRadius:7, padding:0, margin:0, height:"$imageHeight" },
@vovkasm
Copy link
Owner

vovkasm commented Feb 2, 2017

Hm...
Just try it on iOS... borderRadius seems to work, but borderBottomLeftRaius doesn't. Will investigate.

As a temporary workaround I can suggest to wrap WebImage in View:

<View style={{
  borderWidth: 2,
  borderColor: 'green',
  borderBottomLeftRadius: 7
}}>
  <Image source={{uri}} />
</View>

@vovkasm vovkasm self-assigned this Feb 2, 2017
@praneeth-alla
Copy link
Author

@vovkasm Thanks for replying me but I am using your library for Android. Tried your solution but still it doesn't work. Image always have the square edges. please let me know if there is any workaround for android WebImage borderTopLeftRadius and borderBottomLeftRadius

@vovkasm
Copy link
Owner

vovkasm commented Feb 2, 2017

Yea, on Android wrapped borders do not overlap content + works only if WebImage element has width & height :-(
Green border on wrapper View: android borders

Seems that default style handling provided by RN do not work in this case. So I will need to implement it in component itself. Bad, because there are many various properties.

I will try to fix this ASAP, but anyway it will take a few days.

@praneeth-alla
Copy link
Author

Thanks for your quick response. Will wait for your fix and meanwhile will try to find a workaround because this is very very important for my project.

@vovkasm
Copy link
Owner

vovkasm commented Feb 3, 2017

I have partial implementation to test. It is in fix-border-radius branch.
Current problems:

  1. Android only.
  2. Only global borderColor, but border{Top,Left,Right,Bottom}Width and border{Top,Left,Right,Bottom}Radius should be supported.
  3. Broke some resize modes (definitely stretch, but may be others too)

You can test it and use until full functional will be ready. To test:

git checkout fix-border-radius
npm start-sample
npm run-sample-android

If it works, you can setup you project to use this branch until stable version will be released.

@praneeth-alla
Copy link
Author

You're Awesome! I tried the sample and it works :) will integrate with my project. I appreciate your help.

@vovkasm
Copy link
Owner

vovkasm commented Feb 10, 2017

@praneethalla93 Can you please test the latest state of the fix-border-radius branch? If it does what you want, I will implement the same logic for iOS and after other small improvements will make release.

What was done:

  • Same speed drawing without borders as with stable version
  • Fast enough drawing with mono-colors borders
  • Somewhat slower drawing of borders with separate colors for each side
  • There is a bug in previous case - small artifacts at corners
  • Correct borders calculations for all supported resizeModes

@praneeth-alla
Copy link
Author

@vovkasm I took the latest fix-border-radius and tested it. It works perfect and solved my problem, Thanks

@praneeth-alla
Copy link
Author

@vovkasm Hey do you have any plans to merge this branch to master? Right now I am referring to this branch but just wanted to know if you will make all these changes to master so that I can point to master branch from my code.

@vovkasm
Copy link
Owner

vovkasm commented Mar 14, 2017

Yes I do have plans to have this feature in master but implementation will be different.
Currently I did some experiments with android & ios implementations, but I'm still not sure about the right way to do this. In short I try:

  1. make it as simple as possible => reuse react-native code
  2. reduce breaks with future react-native development
  3. make android & ios implementations look similar

But it seems to be impossible to satisfy all targets at once.

The current plan is to finish all remaining tasks (it may take a week or two) and make new release.

JS interface will be the same.

@praneeth-alla
Copy link
Author

Thanks for the info @vovkasm

@vovkasm
Copy link
Owner

vovkasm commented Apr 10, 2017

Sorry for long silence.

@praneethalla93 and anyone interested in borders support, can you please test branch exp/android-borders

It is ready to merge into master now.

Supported style props:

  • borderWidth (borderLeftWith, etc... for each side)
  • borderColor (borderLeftColor, etc... for each side)
  • borderRadius (borderTopLeftRadius, etc... for each corner)
  • padding (paddingLeft, etc... for each side)

Not tested/implemented in this iteration:

  • alpha values for colors, opacity
  • background* props
  • borderStyle props (currently always solid)

If this branch works, I will merge it to master and implement the same for iOS and release this.

@praneeth-alla
Copy link
Author

Yeah I am currently using this branch for borderRadius, borderWidth and it works. You can merge it master.

@vovkasm
Copy link
Owner

vovkasm commented Apr 18, 2017

Just merged Android part to master, but lets keep issue open until iOS will be implemented.

@praneeth-alla
Copy link
Author

Thank you!

@rogoja
Copy link

rogoja commented Nov 22, 2017

It's already been fixed?

@vovkasm
Copy link
Owner

vovkasm commented Nov 23, 2017

Sorry still not. On android it should work, but on iOS still not implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants