RFC: Media query support in StyleSheet - #1010
Conversation
d356b54 to
afac63d
Compare
afac63d to
df893c1
Compare
|
|
||
| - Each condition value flows through the property's normal processor (e.g. `processColor`), so once matched it behaves like any other value. | ||
|
|
||
| **2. Resolution (native commit hook).** The `styleConditions` prop is added to the node's props. A commit hook runs on every commit and, for each node carrying conditions, evaluates its queries against the current environment and patches the matched value onto the props (or restores the unpatched props when nothing matches). |
There was a problem hiding this comment.
I'm not an expert on styling in RN but I think that doing this type of logic in a commit hook is a red flag. Commit hooks should mostly be used by external systems not but RN internals. I feel like this should be done directly as part of layout when committing the shadow tree (within ShadowTree.cpp) not from outside.
We can probably do this the same way we propagate the configuration for point scale factor, etc. from the root shadow node down to individual nodes.
There was a problem hiding this comment.
Thanks for reviewing. My initial instinct was to put it in tryCommit function, right before the layout but then i updated it after seeing AnimationBackendCommitHook, thinking it to be cleaner 😅. Putting it in tryCommit should work, will test and update the proposal/draft PR.
There was a problem hiding this comment.
I think the shared animation backend was implemented that way because of different constraints (probably reconciling source props in React components vs. animation state for those props). We should be able to do it without them here.
There was a problem hiding this comment.
Tested, it works. Updated proposal - 9e0614b and PR - react/react-native@6ac78fa.
| "@media (orientation: portrait)": 24, | ||
| "@media (orientation: landscape)": 36, |
There was a problem hiding this comment.
I wonder if there's a more type-safe API. there's a lot of specialized syntax inside this string that's not type-checked or validated when writing code without external tools.
Maybe template literals could help here but could get messy when combining multiple.
There was a problem hiding this comment.
There was a problem hiding this comment.
Not necessarily advocating for this, but another option could be something like this:
const $landscape = StyleSheet.media({
orientation: 'landscape',
});
const $portrait = StyleSheet.media({
orientation: 'portrait',
});
const styles = StyleSheet.create({
padding: {
default: 12,
[$portrait]: 24,
[$landscape]: 36,
},
});And possibly define some defaults:
const styles = StyleSheet.create({
padding: {
default: 12,
[StyleSheet.media.PORTRAIT]: 24,
[StyleSheet.media.LANDSCAPE]: 36,
},
});There was a problem hiding this comment.
I guess we can keep the string syntax for CSS familiarity (with weak typings like StyleX) and also add StyleSheet.media helper. wdyt?
There was a problem hiding this comment.
I guess the CSS syntax + helper makes sense. prior art in unistyles: https://v2.unistyl.es/reference/media-queries/
but also this proposal is about specifying different property values based on conditions, unlike media query where where it provides a style block. so maybe it shouldn't be based on media query syntax, but CSS if conditions, which are closer to this concept? https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/if
i was actually thinking to use a similar API in react navigation:
const styles = StyleSheet.create({
padding: {
'media (orientation: portrait)': 24,
'media (orientation: landscape)': 36,
else: 12,
},
});There was a problem hiding this comment.
TIL if function. Nice, i guess long term that would be ideal. Wondering if it's still early as it lacks support in widely used browsers and the proposal seems to be in draft (so it will require some polyfill for web if we support its CSS if() syntax). We're not doing style blocks with media query but we can aim to provide CSS like @media syntax for familiarity (to humans and LLMs 😅).
This PR adds an RFC to support media queries with StyleSheet and resolving them natively.
Draft PR that implements the proposed RFC - react/react-native#57439