lost icon indicating copy to clipboard operation
lost copied to clipboard

Add option to enable both RTL and LTR styles in one stylesheet

Open chaenu opened this issue 7 years ago • 8 comments

Hey, first of all thank you for you work, I used lost grid in several projects and it worked great!

Is this a feature request or a bug report? Feature request.

What is the current behavior? With the current lost grid version (8), it's only possible to use either ltr or rtl styles. To mix them in the same project, you need to define two stylesheets and prefix the related classes "manually" with e.g. [dir="rtl"] (see below).

What is the behavior that you expect? Be able to use rtl and ltr in the same stylesheet.

What's the motivation or use-case behind changing the behavior? I am currently working on a global website, including left-to-right and right-to-left pages which should both rely on the same codebase/stylesheet.

Anything else? I could think of a new value for the direction global option, e.g. @lost --beta-direction both; which would render both ltr and rtl styles, but adding the dir attribute as a selector, e.g. [dir="rtl"] .my-grid-col:nth-child(1n) { margin-left: 40px; margin-right: 0; } [dir="ltr"] .my-grid-col:nth-child(1n) { margin-left: 0; margin-right: 40px; }

Related: https://github.com/peterramsing/lost/issues/351 & https://github.com/peterramsing/lost/pull/352

What do you think?

chaenu avatar Mar 06 '17 11:03 chaenu

@chaenu I like it.

I'm still trying to sort some of the "how does pushing and pulling elements work if their order is reversed...what is 'right', what is 'left'?"

Are you up for putting together a PR for this?

peterramsing avatar Mar 07 '17 16:03 peterramsing

Yes, I will try! Any rough idea how to implement this best?

chaenu avatar Mar 07 '17 22:03 chaenu

@chaenu Super. If you want to create a PR early-on we can coordinate the changes.

Here's the initial PR: https://github.com/peterramsing/lost/pull/352/files That should get you some general understanding of what went into the change.

I'm not sure the implementation of making it dynamic based on how you described but I think that it's certainly something that could be put on the initial line such as

lost-column: 1/3 rtl;

...or something like that.

See in this area where/how you might do that: https://github.com/peterramsing/lost/blob/master/lib/lost-column.js#L56

I will say, there is a lot of potential for cleanup and optimization here. One thing that will be tough is that this RTL support can easily cause the rest of the code-base to become a bit hard to maintain...and while I really do want to get some awesome RTL support in LostGrid I don't want it to cause other features to be hindered from being added or existing features to be harder to maintain.

Thanks again and let me know how I can help!

peterramsing avatar Mar 08 '17 06:03 peterramsing

@chaenu If you want to pair on this let me know. Skype? Screenhero?

peterramsing avatar Mar 11 '17 17:03 peterramsing

Hey Peter,

you can see the current result here: https://github.com/peterramsing/lost/compare/master...chaenu:feature/rtl What do you think so far? I followed your suggestion with rtl as an argument for lost-column and lost-waffle, so the developer itself has still full control over the final selector.

I also think there's a bug in the line: lostColumnCycle = decl.value.split('/')[1]; E.g. if you add an argument, then lostColumnCycle contains not only the number of the cycle, but also all arguments. Tried to fix that as well.

Current tests are ok so far, but I will check tomorrow if I should add more tests (and documentation, of course).

chaenu avatar Mar 11 '17 18:03 chaenu

That's looking like a great direction! I'll create a PR into develop and let's see what tests we'll need to write and then look into 1. making sure the existing grids don't break and 2. making sure it's not muddying things up too much. The RTL support might warrant re-thinking the core lost-column/lost-waffle architecture – but thankfully RTL is in beta so we have the freedom to change as we need.

peterramsing avatar Mar 11 '17 23:03 peterramsing

Thank you for your feedback! I added tests (I'm not that experienced writing them, so I hope I covered all relevant cases) and documented the option.

I just struggled over the float-right option for lost-waffle. If I understand the use case correctly, then we would need to introduce a float-left option for rtl-layouts, right? Or maybe update this to a more general feature "last-line-different-direction", no matter if ltr or rtl is used (breaking change...).

Yes, re-thinking the core lost-column / lost-waffle architecture could be a good idea, as they share mostly the same features and adding rtl to them feels mostly like duplicating code. :)

chaenu avatar Mar 12 '17 16:03 chaenu

...not only like duplicating code but it's only designed to go one direction so the code starts to get really confusing if the grid direction is right-to-left. (not my FIXME comment)

peterramsing avatar Mar 12 '17 16:03 peterramsing