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

more options as false by default #3

Closed
necolas opened this issue Sep 25, 2014 · 6 comments
Closed

more options as false by default #3

necolas opened this issue Sep 25, 2014 · 6 comments

Comments

@necolas
Copy link
Contributor

necolas commented Sep 25, 2014

This plugin is quite aggressive in what in changes in selectors and urls. I think almost all the options should be false by default, especially swapLeftRightInUrl, swapLtrRtlInUrl, swapWestEastInUrl, and autoRename because you have to intentionally agree to those conventions for it to work for you. I don't think we'd have use for autoRename at Twitter; that one seems particularly tied to one way of writing CSS.

It might be nice to merge the url options into a single one that takes a map of strings to swap (i18n friendly too, as people can define their own non-english words if necessary).

@MohammadYounes
Copy link
Owner

I've seen many converters and all of them fail in automatically providing an RTL variant that is usable with the same html markup (bi-lingual web sites/apps).

Up to now, the default options worked so great for me in many projects, and I believe they work for bootstrap too (see this branch), clone it and run the docs locally. and see for your self! Its auto flipped using rtlcss and without touching the html (thanks to autoRename) .. check Carousel for example.

tooltip.js was updated to flip the calculated position.

autoRename (as explained here) works under a simple rule: if a given rule has no directional difference and its opposite exists then the names should be flipped (for example: glyphicon-chevron-left and glyphicon glyphicon-chevron-right).

Its unlikely to write a CSS rule having no horizontal significance and name it .right without an opposite .left, and in case you do! you can simply mark it with /*rtl:ignore*/ directive.

Since CSS is written in English, I think i18n friendly doesn't apply here. Although rtlcss is highly customizable and you can inject your own logic but I find it useful in case you want to add prev/next combination.

@necolas
Copy link
Contributor Author

necolas commented Sep 25, 2014

Without knowing the HTML, flipping the class names based on relatively fragile information can be a problem. I don't think we have a use case for it yet at Twitter. What we rely on is using the equivalent of the rename directive to change property values in RTL. Either way, it's an opinionated feature that goes beyond CSS bidi flipping, and can easily break on existing codebases, so I was expecting them to be false in the API by default.

@MohammadYounes
Copy link
Owner

Without knowing the HTML, flipping the class names based on relatively fragile information can be a problem.

Can you provide a single example where my approach would fail ?

What we rely on is using the equivalent of the rename directive to change property values in RTL.

Can you clarify with an example ?

Can easily break on existing codebases

I'm using my flipped bootstrap and there is nothing broken! I would appreciate it if you can provide an example.

I feel one of us is missing something here!

@necolas
Copy link
Contributor Author

necolas commented Sep 25, 2014

Can you provide a single example where my approach would fail ?

Any existing code base with urls or classes that contain the strings you're swapping. If it wasn't authored with that expectation, there will be unintentional changes. e.g., maybe there's no image with the new path, or no intent to replace the image, or no intent to rename the class because it will be unchanged in the HTML.

There's a pretty good chance it would break things for our RTL stylesheets. Not saying no one will ever use the feature, but it should be off by default IMO.

Can you clarify with an example ?

.icon-chevronLeft:before { content: "\f00" /*rtl:\f10*/; }

@MohammadYounes
Copy link
Owner

I got your point, but still it goes both ways, and I think its a low probability. The replacement is not that aggressive, by default word boundaries are respected. In your example .icon-chevronLeft will not be auto-renamed unless you turn the greedy option on (default:off).

Actually, Icon rules in general are the reason behind autoRename, I don't have to manually maintain mapping between directional rules - take "Font-Awesome" as an example.

I believe a successful flipping requires good understanding of the tool options, The author must compare and validate the generated CSS, Then apply necessary directives to ensure the output is as excpected.

@MohammadYounes
Copy link
Owner

@necolas v2.0 is out with more options as false by default :)

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

No branches or pull requests

2 participants