-
Notifications
You must be signed in to change notification settings - Fork 83
borderRadius property regression? #11
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
Comments
I'll have to test this. RN claims to support this. https://facebook.github.io/react-native/docs/view.html#style |
I don't quite understand what the issue here is, what should be happening that isn't happening? |
I've just tested this out again using In the simulator, could you inspect that with the inspector and see what style properties are given? What is That's because The way <RoundImage source={{uri: 'http://placehold.it/100x100'}}/>
// styles
import styled from 'styled-components/native'
const RoundImage = styled.Image`
height: 100;
border-radius: 50;
width: 100;
` This either means not being able to upgrade to the newer version or having to refactor everywhere in a project where an I've just given this another go without So I've gone from |
Got you. It’s fixed in #13. |
I don't think it can be fixed here can it? I think this is potentially a
It would be a nice to have for now if only supplying one value, it only output
Of course, if multiple props are supplied you can't really give a single value for |
Can you not wrap the image in a View, and use Although we might need to look at some special casing. |
Yeah I mentioned that as a solution above but then that means refactoring every instance of an
That's why I'm leaning towards this being an issue with |
I’m thinking, special casing is going to be hard: const StyledImage = styled(Image)`...`; // This is hard enough to work out that we need to special case
const SuperSpecialStyledImage = styled(StyledImage)`...` // This is almost impossible Maybe we need to turn off this shorthand for the time being. @mxstbr |
Let's turn it off then! |
But that's a major version release again, ugh |
That's true. Maybe we just document the behaviour? |
Although I suppose this was a breaking change in the first place? I believe semver says we should revert the breaking change. |
Oh if it was a breaking change in the first place let's push this as a patch. Would you mind doing that? |
Happy to help if required 😄 |
Over here! styled-components/styled-components#254 |
Looks like we could turn this off soon There's a few other cases that'll get fixed too |
Poking on this one @jacobp100 |
Has anyone confirmed this works now? And from what version it works |
Also, the change to re-enable these has to happen in styled-components. We take a blacklist of properties, and the border-radius is passed in there in SC Once SC removes this, we'll remove the code around blacklisting here too, since it will no longer be needed |
I'm closing this because it's been years since this was an issue |
I'm still having problem with it My current setup: Edit: ImageWrapperView: styled(View)`
overflow: hidden;
width: 280px;
height: 328px;
border-radius: 20px;
`,
Image: styled(Image)`
width: 100%;
height: 100%;
`, But it would be nice to rely directly on the styled Image |
I had opened up an issue over on
styled-components
here about theborderRadius
property on native.It seems going from version
1.0.11
to1.1.1
instyled-components
regressed howborderRadius
would work.borderTopLeftRadius
etc. seemingly have no effect when developing for native from what I can tell in the simulator and what works is simply usingborderRadius
.After having a dive into the source, I think this just requires a slight change to the grammar file to only output
borderRadius
.I'll submit a PR for this.
@jh3y
The text was updated successfully, but these errors were encountered: