This implementation would make perfect sense if RTL is a property independent of the locale in a way that my Arabic may be RTL, but your Arabic may not. However, this is not the case. RTL (or LTR) is the property of the locale, independent of the user. If the `I18n.locale` is assigned a property called `direction` which may hold values `rtl` or `ltr`, with `ltr` being the default (let’s forget about the `ttb` direction for now because that is almost always represented as `ltr` on the web). Alternatively, a method `rtl?` is added to the `I18n.locale` that returns false by default, unless the locale is in the array of RTL locales.
This way, entire RTL class can go away and wherever a locale object is accessible, its direction is also known. We don’t need to know separately if a user’s locale has RTL enabled, we just find the effective locale and apply RTL if applicable.
I know that Discourse does not encourage code refactoring for the sake of refactoring, but being a newbie to the code base I would like to hear the rationale behind why it is the way it is?
---
### Author:  [@simon](https://meta.discourse.org/u/simon)
#### Post date: [11 Dicembre 2016, 3:37am UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/2 "2016-12-11T03:37:33Z")
I think the RTL class is used in `UserNotifications` for adding the `rtl` class to emails.
---
### Author:  [@ibnesayeed](https://meta.discourse.org/u/ibnesayeed)
#### Post date: [11 Dicembre 2016, 3:48am UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/3 "2016-12-11T03:48:20Z")
> [@simon](#):
>
> I think the RTL class is used in UserNotifications for adding the rtl class to emails.
Does this change anything? After all user’s locale or site’s default locale is determined in order to know if it belongs to RTL or not. If the direction was property of the locale, then we wont need this RTL class at all, be it in the email, notification, site’s layout, CSS classes and whatnot.
---
### Author:  [@simon](https://meta.discourse.org/u/simon)
#### Post date: [11 Dicembre 2016, 3:53am UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/4 "2016-12-11T03:53:34Z")
> [@ibnesayeed](#):
>
> Does this change anything?
It just means that the class can’t be eliminated without changing a few other things as well. But yes, what you’re saying seems correct to me.
---
### Author:  [@ibnesayeed](https://meta.discourse.org/u/ibnesayeed)
#### Post date: [11 Dicembre 2016, 4:17am UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/5 "2016-12-11T04:17:27Z")
> [@simon](#):
>
> It just means that the class can’t be eliminated without changing a few other things as well.
Such as, lines 389 and 401
>
become (if `direction` property is defined on locale)
```ruby
classes: user.effective_locale.direction
```
or (if `rtl?` method is defined on locale)
```ruby
classes: user.effective_locale.rtl? ? 'rtl' : ''
```
---
### Author:  [@simon](https://meta.discourse.org/u/simon)
#### Post date: [11 Dicembre 2016, 4:37am UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/6 "2016-12-11T04:37:12Z")
Yes. I don’t know if this is a change they will want to make, but it’s good you are interested in working on this. I’ve done some work on it in the past. I got involved with it to help set up a forum, but I don’t speak a RTL language. It’s a great way to learn the Discourse code. There are still a few issues in the javascript as well: [RTL interface has wrong position for some of popup boxes](https://meta.discourse.org/t/rtl-interface-has-wrong-position-for-some-of-popup-boxes/51503).
---
### Author:  [@ibnesayeed](https://meta.discourse.org/u/ibnesayeed)
#### Post date: [11 Dicembre 2016, 4:38am UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/7 "2016-12-11T04:38:16Z")
Also, I noted that in the existing TRL class there is some logic duplicated.
>
This chunk can be replaced by the following code.
```ruby
def enabled?
user.effective_locale.in?(rtl_locales)
end
```
Because `current_user_rtl?` and `site_locale_rtl?` are local methods and not used elsewhere. While the common portion of the two methods (finding the effective locale) is already defined in the `User` class.
>
---
### Author:  [@ibnesayeed](https://meta.discourse.org/u/ibnesayeed)
#### Post date: [11 Dicembre 2016, 5:00am UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/8 "2016-12-11T05:00:35Z")
> [@simon](#):
>
> I don’t know if this is a change they will want to make, but it’s good you are interested in working on this.
I feel that they might actually be willing to get rid of a class that does not deserve to be a class, because the class itself is not that deep rooted. If we keep the surface methods such as `rtl?` helper method behave the very same way, just change the base implementation without affecting the API then it should be OK. Obviously, we might need some more test cases related to locale direction beyond what is already there.
However, being new to the code base I won’t jump to a conclusion too quickly without hearing from some other people who have better understanding of the code inside out.
---
### Author:  [@Mittineague](https://meta.discourse.org/u/Mittineague)
#### Post date: [11 Dicembre 2016, 8:22am UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/9 "2016-12-11T08:22:34Z")
I won’t go so far to say I know the code inside out, but I’ll chip in my 
I looked to see if any tests might break if `current_user_rtl` or `site_locale_rtl` were removed. I found none and in fact the only place they exist in the Core code is in the models/rtl.rb file.
So removing them might break some plugins if they are using them, but it shouldn’t break Discourse.
I don’t think “i18n” would need to be used in the proposed `enabled` but I’ve seen it used in other files.
>
>
>
---
### Author:  [@ibnesayeed](https://meta.discourse.org/u/ibnesayeed)
#### Post date: [11 Dicembre 2016, 2:15pm UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/10 "2016-12-11T14:15:01Z")
It looks like `locale` is simply a string, not an object. Hence we can’t attach properties to it. However, there are other ways to specify the direction. One is to add a `direction` or `rtl?` attribute in the `config/locales/names.yml` file. Another possibility is to define `rtl?` method in a place from where it can be called from any part of the application.
>
---
### Author:  [@Mittineague](https://meta.discourse.org/u/Mittineague)
#### Post date: [12 Dicembre 2016, 12:01am UTC](https://meta.discourse.org/t/why-is-rtl-class-user-dependent-instead-of-locale/54077/11 "2016-12-12T00:01:01Z")
For some reason I like the idea of having it in the yml file.
I have no basis for the feeling other than it just seems like the logical best place for it.
I tried hacking the rtl.rb file to
```plaintext
def enabled?
# site_locale_rtl? || current_user_rtl?
user.effective_locale.in?(rtl_locales)
end
# def current_user_rtl?
# SiteSetting.allow_user_locale && user.try(:locale).in?(rtl_locales)
# end
# def site_locale_rtl?
# !SiteSetting.allow_user_locale && SiteSetting.default_locale.in?(rtl_locales)
# end
```
I don’t know Farsi so I could very well not be seeing a glaring issue.
But the hack did not crash Discourse and I saw no error messages anywhere.
I did notice that the title was not translated for some pages, but that was true for both hacked and not-hacked.
And I did not try the `set locale from accept language header` setting nor test emails or many pages at all.
I did test as a site setting desktop and mobile view, and allow user preference desktop and mobile view.
**site default setting desktop**
**site default setting mobile**
**user preference desktop**
**user preference mobile**
