Add aspect ratio support #123
Conversation
| }) | ||
| }) | ||
|
|
||
| it('handles CSS-style aspect ratios', () => { |
There was a problem hiding this comment.
Do we actually want to support the CSS-style aspect ratios? aspect-ratio isn't a CSS prop, so we're actually extending a React Native prop with a "CSS-style" syntax, so this is something that only works when using this parser.
There was a problem hiding this comment.
So there is a ratio type in CSS that's used in media queries (mdn), and this is shipped in browsers. This is the semantics this PR implements
There's also a proposal for a React Native style aspect-ratio property here. But that is just a proposal, it might never be implemented
I think we can be sure that if CSS did implement the proposal, it will use the ratio semantics. But this also isn't a feature anyone requested, so I don't mind if you think it's too premature to merge it in! I don't mind either way! 😃
There was a problem hiding this comment.
The property and the syntax is probably not going to change as it has a proposal, and the same syntax also being used in other CSS features, but there is a small chance that it will never implemented in browsers.
We can go forward with this, but it might mean that we have to do a breaking change in the future if things don't go as planned.
There was a problem hiding this comment.
I give it 50/50 of being implemented in browsers if that helps
Currently we allow aspect-ratio: 1.5 (via the generic value transformer), which is not web compatible.
We also have custom transforms for shadowOffset and textShadowOffset, and we indirectly support shadowColor and shadowRadius etc. via the generic value transformer. But these properties will almost certainly never be supported on the web
But I don't know if we should stop people using styles that RN added on-top of CSS
The values for some of these additional styles are objects, so to support them, we need a custom transformer. There's also no proposed CSS spec for this, so we're really hoping the web doesn't add these in a breaking way
What do you think on these?
Aspect-ratio is kind of interesting because the spec pretty much copies what RN does (except for the syntax). But the generic value transformer (i.e. 1.5) is not compatible with the proposed spec. The proposed spec only allows ratios like 3 / 2
So say browsers do support this, we would need to do a breaking change to remove the 1.5 version - and we'd need to do this regardless of if we decide to merge this PR
If we do decide to merge this in, I'll actually need to make this stricter - since I think you can put stuff like -3.2 / 8.4 - to avoid a breaking change later on
But if we decide not to merge this in (and while we're doing breaking changes with borders anyway), it might be an opportunity to reconsider the properties I put above
There was a problem hiding this comment.
But I don't know if we should stop people using styles that RN added on-top of CSS
We should definitely not do that, but what I see as a bit of a risk is to add syntax that is not supported in RN, and is only a proposal in Web.
If people know that their app is only going to be used with RN, then they should be able to use the RN specific styling props.
There was a problem hiding this comment.
So say browsers do support this, we would need to do a breaking change to remove the
1.5version - and we'd need to do this regardless of if we decide to merge this PR
But currently aspect-ratio is a React Native-only property, so we could just think about it as something that the parser does not care about until it's implemented in CSS. When the CSS property is final, then we could allow both RN and Web way of giving the value, and people who want to render in both native/web could use 3/2 syntax.
What we could do is to add docs about which props are treated as CSS properties and which are considered as React Native specific.
There was a problem hiding this comment.
Sure. Let's do that
I wanna find some time to work on documenting what we have, as a lot of people ask for this!
|
@kristerkari This is now in CSS - and the syntax is compatible with this PR. Can I get a tick? |
|
@jacobp100 now that this is merged, should we also release a new minor version with |
|
Yes, I will try to remember to do this |
No description provided.