Adds [dir="ltr"] scoped styles in addition to [dir="rtl"] scoped styles

[majapw]

After a lot of conversation, this change actually restructures a lot of things in the RWS-IA package to better support both a vanilla app and an app that requires automatic flipping of LTR styles for an RTL context (ie airbnb.com). As a result, this is probably semver-major, but hilarious semver-major to bring us back to a version that is compatible with users of v1.0.0. idk.

Essentially, react-with-styles-interface-aphrodite exports 3 different versions of the interface now:

• react-with-styles-interface-aphrodite (src/aphroditeInterface.js) This is the v1 compatible version of the interface. No funny business. Just resolves styles as expected using Aphrodite and inline styles.
• react-with-styles-interface-aphrodite/no-import This is the v1 compatible version of the interface that doesn't use the !important tag. No funny business. Just resolves styles as expected using Aphrodite and inline styles.
• react-with-styles-interface-aphrodite/with-rtl This is the prototypal, new technology version of the interface that supports writing styles for just an LTR context and automatically flipping them in an RTL context. It relies on the presence of a top level dir attribute in your application, and recommends using react-with-direction simultaneously. It has some funny business with inline styles, but in general solves 99% of RTL flipping.

In react-with-styles-interface-aphrodite/with-rtl, we've introduced a number of changes based on some comments from @lencioni. Specifically, he brought up the point that it might be impossible to choose the correct reset value for all scenarios.

react-with-styles-interface-aphrodite/with-rtl scopes all directional styles to either a [dir="ltr"] scope or a [dir="rtl"] scope. Everything else is applied as a style normally.

@yzimet @lencioni @ljharb

[ljharb]

it might be impossible to choose the correct reset value for all scenarios.

I don't see any comment about that there; can you elaborate?

[ljharb]

(fwiw this seems like a semver-major)

[majapw]

@ljharb it was when I was prototyping things internally:

From @lencioni on a variety of different values that we tried:

I'm not sure inherit is what we want here because inherit will take the style from the parent element. For some styles which default to being inherited, like color, this is okay, but for other styles, like margin, this is not okay. See this example: https://jsfiddle.net/exsqsLsw/1/

auto isn't necessarily what you want here either. e.g. margin: 0 auto; will center the element. A lot of properties have initial, which might be what you want. Also, I think it is not possible to get this value always correct, so this should be documented. e.g. if I have a (super insane) style on my page like div { margin: 10px; } this will never get it right.

The trouble is though, things like normalize.css (we use) and CSS reset are pretty common. I think that setting this to anything like initial will end up causing visual bugs in some browsers. e.g. https://github.com/necolas/normalize.css/blob/73b6b0c7e8690ab5005bca9d7e13d3fb319c98ac/normalize.css#L311-L313. I'm not sure if there's any reasonable solution to this other than a disclaimer in documentation.

We have actually recently run into an issue where a component that was using borderRadius in combination with borderBottomRightRadius would have some weird behaviors due to the attempt to overwrite things. Splitting it up into things that are scoped to the LTR context and things scoped to the RTL context would solve this issue. @yzimet can give more context there.

[ljharb]

With this approach, as long as there's a top-level dir attribute, what caveats are there wrt nesting direction contexts?

[yzimet]

Another advantage of this approach is that resetting styles with initial doesn't work on IE, so it's great to remove that.

As Maja mentioned, I was running into an issue where a rule reset was overriding the rtl behavior:

[dir="rtl"] .content_hg7zgt {
  border-radius: 8px 0 0 8px !important;
  border-top-right-radius: 8px !important;
  border-bottom-right-radius: 8px !important;
  border-top-left-radius: initial !important;
  border-bottom-left-radius: initial !important;
}

.content_hg7zgt {
  border-radius: 0 8px 8px 0 !important;
  border-top-left-radius: 8px !important;
  border-bottom-left-radius: 8px !important;
}

In the example above, the corner border radius for RTL should override for top-right and bottom-right, but it was also producing a side-effect on top-left and top-right by setting them to initial.

Your PR would fix this :)

@ljharb , I think we'll still need to address nesting direction contexts; that remains an issue.

[majapw]

My biggest concern was that this change requires that you have a top-level node that explicitly sets the direction context (otherwise flippable styles will just be lost completely).

I've thought about this and realize that... in addition to the resolveNoRTL method (which is used for edgecases in a directional app), we should also export a no-rtl version of the interface, much in the same way that we export a no-import version. I think that would make me a lot less nervous.

[ljharb]

That seems reasonable.

In that case, would it be better to have the default be "no RTL" (since it has no caveats), and then separately export the other pieces?

[majapw]

@ljharb @gabergg @lencioni @yzimet I have restructured everything! D: Please take a look and let me know what you think.

[majapw]

I've addressed all of Joe's comments I think. PTAL @lencioni @ljharb

The only way to address https://github.com/airbnb/react-with-styles-interface-aphrodite/pull/18#discussion_r138144705 require an eslint disable of no-param-reassign... does that make sense? I'd appreciate eyes on that in particular.

[majapw]

@lencioni I think everything has been addressed now

[lencioni]

@majapw at least one of the CI failures should be fixed by rebasing. See https://github.com/airbnb/react-with-styles-interface-aphrodite/pull/20 for more info.