callstack / linaria

Zero-runtime CSS in JS library
https://linaria.dev
MIT License
11.69k stars 417 forks source link

RTL support #491

Open jpnelson opened 5 years ago

jpnelson commented 5 years ago

Describe the feature

It would be nice to have some form of RTL support for the library. For example, you can see how react native web handles this – the requirement is that there are different static styles compiled that can be switched between at runtime, particularly the LTR and RTL styles. https://github.com/kentcdodds/rtl-css-js is a library that does the actual "style flipping" logic.

Motivation

We'd like to be able to support languages that have a RTL flow.

Related Issues

I couldn't find any but let me know if this has been talked about before!

As a side note, I'm interested in whether this is something that you'd be open to PR's for. Thanks!

umarmw commented 4 years ago

I'm also looking for the RTL support. :)

lencioni commented 4 years ago

It looks like Linaria can be configured to use PostCSS as a custom preprocessor, and there are PostCSS plugins like postcss-rtl to do RTL transformations. I haven't tried it, but it seems that this could be accomplished by configuring Linaria in that way.

rmarkins-godaddy commented 4 years ago

@jayu would like to see this supported as currently we use linaria for our library of components but the /* rtl */ comments are removed upon build. Do we have a base idea of what is needed to support this? I would be happy to contribute a change to add this support

jayu commented 4 years ago

This is really tricky thing. Since we compile styles into CSS files, rtl could be achieved in the following ways

  1. compile two different stylesheets and load one of them depending on user locale

    • additionally, we will have to care about flipping styles that are loaded dynamically based on props and attached to CSS variables
  2. compile one stylesheets, but produce different class names for RTL and LTR

    • there has to be an additional util function that selects the proper class name
      • probably it could be added on babel-level based on some flag in config
      • otherwise, users will have to modify codebase to use this util explicitly
      • additionally, we will have to care about flipping styles that are loaded dynamically based on props that are attached to CSS

First would not work well for statically hosted pages, the decision which stylesheet should be chosen will have to be made on client-side, it will require additional work on bundlers level to produce two style sheets.

Second, add additional complexity into linaria core, will require implementing some react context to define user locale. For non-react applications, some other solution has to be exposed to let the util know which class name should it choose which would effectively mean that some global variable will be involved if we decide to do it on the babel level.

Personally I would prefer second option

alikazemkhanloo commented 2 years ago

Any update on this?

alikazemkhanloo commented 2 years ago

Jayu, one way to do that would be using one class name but with an extra rtl selector in the css file for example

.asdf {
  padding-left: 10px;
}

[dir=rtl] .asdf {
   padding-right: 10px;
}

This way we don't need a class name selector utility function, so no extra work on that. As for dynamic styles, we don't need to worry a lot cus usually the value will be passed as props, and other styles like align-items: flex-end are usually never passed as props.

These extra entries can be generated based on a choice in a config file.

There might be some issues on pagespeed insights, regarding unused css but it is better than nothing.

alikazemkhanloo commented 2 years ago

Update: I used postcss-rtlcss plugin in postcss. it does exactly what I said. As NextJS uses postcss by default, all I needed to do was adding a file named postcss.config.js containing:

module.exports = {
  plugins: {
    "postcss-rtlcss": {},
  },
};
oren-l commented 2 years ago

Jayu, one way to do that would be using one class name but with an extra rtl selector in the css file for example

.asdf {
  padding-left: 10px;
}

[dir=rtl] .asdf {
   padding-right: 10px;
}

This way we don't need a class name selector utility function, so no extra work on that. As for dynamic styles, we don't need to worry a lot cus usually the value will be passed as props, and other styles like align-items: flex-end are usually never passed as props.

These extra entries can be generated based on a choice in a config file.

There might be some issues on pagespeed insights, regarding unused css but it is better than nothing.

why not just use css logical properties?

.asdf {
  padding-inline-start: 10px;
}
alikazemkhanloo commented 1 year ago

why not just use css logical properties?

.asdf {
  padding-inline-start: 10px;
}

Well, that's another way to do it which I didn't know at the time of writing.

Considering this approach and using the external postcss plugin, I don't think it is worth adding support for RTL internally. Maybe creating another postcss plugin to replace left/right with start/end alternatives would be a cleaner approach.

jimmy-guo commented 1 year ago

Update: I used postcss-rtlcss plugin in postcss. it does exactly what I said. As NextJS uses postcss by default, all I needed to do was adding a file named postcss.config.js containing:

module.exports = {
  plugins: {
    "postcss-rtlcss": {},
  },
};

@alikazemkhanloo I am also leveraging postcss-rtlcss. I'm hitting the same limitation as @rmarkins-godaddy (https://github.com/callstack/linaria/issues/491#issuecomment-659007125) in that Linaria's preprocessor stylis strips out comments, so leveraging the background-color: red /*rtl:blue*/; API does not work. Any chance you use this API and were able to work around it?

alikazemkhanloo commented 1 year ago

@jimmy-guo No, I decided not to use linaria for now.