Separate out LTR/RTL create and resolve methods #37
Conversation
src/aphroditeInterfaceFactory.js
Outdated
| return StyleSheet.create(styleHash); | ||
| }, | ||
|
|
||
| createRTL(styleHash) { | ||
| const styleHashRTL = {}; | ||
| Object.entries(styleHash).forEach(([styleKey, styleDef]) => { |
There was a problem hiding this comment.
to confirm, do we need/have a shim for Object.entries?
There was a problem hiding this comment.
@ljharb would know.
I didn't think so? We were using it before.
There was a problem hiding this comment.
We should probably use https://npmjs.com/object.entries; otherwise we have an implicit dependency on people using https://npmjs.com/airbnb-js-shims (which isn't terrible, but isn't ideal).
There was a problem hiding this comment.
Sounds good. I can add that.
src/utils/resolveRTL.js
Outdated
|
|
||
| // Styles is an array of properties returned by `create()`, a POJO, or an | ||
| // array thereof. POJOs are treated as inline styles. This version of the | ||
| // resolve function explicitly does no work to flip styles for an RTL context. |
There was a problem hiding this comment.
nit: this comment seems incorrect about RTL
| } | ||
|
|
||
| if (hasInlineStyles) { | ||
| result.style = rtlCSSJS(inlineStyles); |
There was a problem hiding this comment.
Could you remind me, why isn't this already captured by the interfaceFactory's call to rtlCSSJS in the createRTL function? (sorry i'm not super familiar with how this interface works)
There was a problem hiding this comment.
Right so the create method deals with stylesheets, e.g. styles that you define in the thunk that gets passed to withStyles. Inline styles can be defined on the fly and are passed in as an object to the css method itself... they are explicitly not part of the stylesheet definition and so don't get handled by the create. That's why we have to do this here as well. :)
(notice that the aphrodite styles are chill tho because the create took care of those)
There was a problem hiding this comment.
LGTM pending adding a peer dep on react-with-styles at ^3 and a release of airbnb/react-with-styles#124
majapw
force-pushed
the
maja-add-separate-resolve-methods-for-rtl-and-ltr
branch
3 times, most recently
from
dae26e5 to
80c8536
Compare
majapw
force-pushed
the
maja-add-separate-resolve-methods-for-rtl-and-ltr
branch
from
80c8536 to
d734546
Compare
This is a breaking change. It pairs with airbnb/react-with-styles#124.
This change removes the
with-rtlaphrodite interface and adds that functionality in the default (but only if you use the CSS/resolve method that will be passed down via props). You will still be able to import a non-directional css method fromreact-with-styles.to: @ljharb @garrettberg @yzimet @lencioni