-
Notifications
You must be signed in to change notification settings - Fork 79
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
feat: add native RtL css support #193
base: master
Are you sure you want to change the base?
feat: add native RtL css support #193
Conversation
@@ -117,6 +130,8 @@ it('transforms margin shorthand with auto', () => { | |||
}) | |||
}) | |||
|
|||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you run prettier on this file?
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prettier again
export const getPropertyName = propName => { | ||
const isCustomProp = /^--\w+/.test(propName) | ||
if (isCustomProp) { | ||
return propName | ||
} | ||
return camelizeStyleName(propName) | ||
return cssRtLToNativeCamelized[propName] || camelizeStyleName(propName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return cssRtLToNativeCamelized[propName] || camelizeStyleName(propName) | |
return cssRtLToNativeCamelized[propName] ?? camelizeStyleName(propName) |
The spec for this isn't yet a candidate recommendation - https://www.w3.org/TR/css-logical-1/ I'm fine to merge this if we add a note to the readme saying it's not considered stable, and behaviour may change without a major version bump |
Why
It was decided the
I18nManager.forceRtL(true)
does nothing on Web, hence rendering RtL handling between native/non-native very difficult using properties like margin-left/margin-right (mobile flips whereas web doesn't).With this change, we can write RtL sensitive properties that will work across native (i.e. Mobile) and non-native (i.e. Web) platforms and allow for the components be re-used with the same strategy (more on that below) since RN does not understand
marginInlineStart
for example.Proposition
This PR would essentially allows us to write non-native css properties like so:
which would in turn be converted to accepted native css and applied like so:
It is worth nothing that this works without any code change but is less than ideal suggestion to mix native with non-native css:
Strategy
dir="rtl"
on a highest top level RN parent in the tree which will force all children to take on rtl.I18nManager.forceRtL(true)
Notes
Minimal code change/tests since the root issue is still be discussed