Skip to content
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

Bootstrap 3 RTL Support #8958

Closed
wants to merge 1 commit into from
Closed

Bootstrap 3 RTL Support #8958

wants to merge 1 commit into from

Conversation

hero-m
Copy link
Contributor

@hero-m hero-m commented Aug 1, 2013

I have started working on adding RTL support to bootstrap 3.

I have done this by overriding the base styles for each component in a separate .less file, so it would be easier to integrate with the customizers.
I have also generated a separate .css file, rather than modifying "bootstrap.css" itself, because i think this is more in line with the way most themes and CMSs (drupal, wordpress, ...) handle multilingual sites.

The .html files have only been modified (through header-rtl.html) to include a link to bootstrap-rtl.css, and kept separate so that the original and rtl pages can be easily compared side-by-side.

@hero-m hero-m mentioned this pull request Aug 1, 2013
@cvrebert
Copy link
Collaborator

cvrebert commented Aug 1, 2013

Please search for and read the previous RTL pull request attempts. @mdo wants to add RTL support piecemeal, to one component at a time, not all of them at once, so I don't think this pull request will be merged as it currently is.

@hero-m
Copy link
Contributor Author

hero-m commented Aug 1, 2013

@mdo @cvrebert I have added another pull request (#8981), just containing RTL support for forms. Although, most of this pull request was RTL support for the core css stuff; so, now some parts (like the grid) are mixed up in there.

@matih
Copy link

matih commented Aug 1, 2013

I think that the best way to integrate RTL support bulit-in is with @less-bidi or with his concept:
https://github.com/danielkatz/less-bidi

@hero-m
Copy link
Contributor Author

hero-m commented Aug 2, 2013

@matih I don't like the @less-bidi approach, because it adds another layer of complexity throughout the code, and putting the bidi- rules alongside the rest of the rules doubles the total amount of css generated. And for users that want to override the generated css (rather than modifying the .less files), every non-bidi- rule has to be overriden twice.

@mdo
Copy link
Member

mdo commented Aug 2, 2013

We won't be duplicating any of the layouts or includes to support RTL—that just doubles our work and is super rough on us maintainers and anyone browsing the repo. Let's punt further discussion to #8981 to get some of this stuff in on a per-component basis.

@mdo mdo closed this Aug 2, 2013
@danielkatz
Copy link

@hero-m, actually, in less-bidi model you don't have to generate both the ltr and the rtl output to the same file. There is nothing stopping you from putting the .ltr { .style(ltr); } and the .rtl { .style(rtl); } into two separated files; bootstrap.ltr.css and bootstrap.rtl.css for example, that could be loaded interchangeably by a web page, depending on the locale. This way, the actual load stays about the same size.
@mdo, less-bidi was created to avoid code duplication like you mentioned. So from maintenance point of view, by using less-bidi model you have to maintain only single copy of every rule.

@hero-m
Copy link
Contributor Author

hero-m commented Aug 4, 2013

@danielkatz Generating separate files is what I assumed too. My arguments against less-bidi are (in simpler words):
For anyone downloading the final css, changing every non-bidi- rule requires modifying both .ltr.css and .rtl.css files.
And the bidi- rules will only make the .less code harder to understand (e.g. ".bidi-padding-end" vs "padding-right"), while I assume the majority of bootstrap users don't care about the RTL version.

Also, @mdo commented here on using overrides vs. mixins.

@danielkatz
Copy link

@hero-m, for users that don't care about RTL support there is no need to override anything in .rtl.css stylesheet since they wont include it in their web pages at all.
On the other hand, i can understand that if in the eyes of the bootstrap team the css files are the final product they care about, and not the less files as i thought. then indeed, the bidi users would have to override two copies of the same rule in ltr.css and rtl.css files. And this as far as i understand it is what @mdo meant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants