RTL CSS generation: Switch from CSSJanus to RTLCSS

[ocean90]

Prologue

Three days ago we were finally able to switch back from a temporarily fork of grunt-cssjanus and CSSJanus to the official ones, see [31425]. The temporary fork was created 6 months ago by me to fix an issue related to border-radius, see #29038.
This wasn't the first pin to a patched fork, the first one was 15 months ago, see [26610].

After [31425] (​build changeset r31406), I had noticed that there is something wrong with text shadows. And I was right, it switches the y-coordinate instead of the x-coordinate. I've reported the ​issue upstream. But I don't want to patch CSSJanus anymore.

RTLCSS

So I did some research and found ​RTLCSS by Mohammad Younes. Same as CSSJanus is RTLCSS a framework for converting LTR CSS to RTL. It supports the following CSS properties:

background, background-image, background-position, background-position-x, border-bottom-left-radius, border-bottom-right-radius, border-color, border-left, border-left-color, border-left-style, border-left-width, border-radius, border-right, border-right-color, border-right-style, border-right-width, border-style, border-top-left-radius, border-top-right-radius, border-width, box-shadow, clear, cursor, direction, float, left, margin, margin-left, margin-right, padding, padding-left, padding-right, right, text-align, text-shadow, transform, transform-origin, transition, transition-property

transform, transform-origin, transition and transition-property are not supported by CSSJanus. RTLCSS supports also multiple values for box-shadow and text-shadow.

I also did a look at the source code of RTLCSS and it looks much better than CSSJanus, it benefits of using ​PostCSS for parsing CSS. (PostCSS is also used by Autoprefixer. \o/)

And coincidentally, I've found an issue related to parsing text shadows, sigh. But hey, it got ​merged in under 24 hours!

Can i haz a grunt task?

Sure, there is ​grunt-rtlcss, also maintained by Mohammad. It supports everything what grunt-cssjanus does. (I only had to ask for a generateExactDuplicates:false option, see merged ​pull request.)

CSSJanus has a processContent option which I've replaced with a ​stringMap in RTLCSS:

{
	name: 'import-rtl-stylesheet',
	search: [ '.css' ],
	replace: [ '-rtl.css' ],
	options: {
		scope: 'url',
		ignoreCase: false
	}
}

Other options we should use:

swapLeftRightInUrl: false,
swapLtrRtlInUrl: false,
autoRename: false, // Don't rename left/right in selectors
preserveDirectives: true, // Don't remove the directives (optional, CSSJanus does this default)

Speaking of directives, CSSJanus has /* @noflip */ while RTLCSS uses /* rtl:ignore */ (​and others).

The patch

The attached patch adds "grunt-rtlcss": "~1.5.1", to our devDependencies, changes the RTL grunt task per above, and updates our stylesheets to use the new directive. Since RTLCSS supports transform too, we have to add some additional directives, like about.css. But we can also remove some, see media-view.css.

I set this as a bug, since RTL text shadows are broken currently.

[DrewAPicture]

+1 for making the switch. Seems like the RTLCSS dev has already been quite a bit more responsive. Would it possible to get some sample vizrecs for before (w/ CSSJanus) and after (w/ RTLCSS)?

This would mostly serve as a visual test. A comparison of the generated RTL styles might also suffice.

[jorbin]

In general, this sounds like a good switch. While CSSJanus has served us well, RTLCSS seems to have more features.

I agree with Drew, that a comparison is a good idea. We should make sure we aren't breaking anything by making this change.

The one thing I think I would do differently (and my above patch does it) is to keep cssjanus as a command, only have it turn into an alias for rtlcss with a warning.

I don't want to clutter our main Grunt file with this though, so I added a new deprectatedTasks file for this (and any future ones that we add).

[ocean90]

build.diff​ is a diff from the CSS files.
(Note: The change in wp-admin/css/ie-rtl.css is correct and fixes a bug in cssjanus.)

Would it possible to get some sample vizrecs for before (w/ CSSJanus) and after (w/ RTLCSS)?

There isn't much difference, but I just noticed that the shadow of the Press This button is now switched in RTL, see ​https://cloudup.com/c6tuCzztRqA.

The one thing I think I would do differently (and my above patch does it) is to keep cssjanus as a command,

I don't think this is really necessary. We already have a custom RTL task (grunt rtl) which should be used.

[jorbin]

Replying to ocean90:

build.diff​ is a diff from the CSS files.
(Note: The change in wp-admin/css/ie-rtl.css is correct and fixes a bug in cssjanus.)

Would it possible to get some sample vizrecs for before (w/ CSSJanus) and after (w/ RTLCSS)?

There isn't much difference, but I just noticed that the shadow of the Press This button is now switched in RTL, see ​https://cloudup.com/c6tuCzztRqA.

That change looks good to me and makes more sense with the pin. The pushpin being on the "up" side looks out of place.

The one thing I think I would do differently (and my above patch does it) is to keep cssjanus as a command,

I don't think this is really necessary. We already have a custom RTL task (grunt rtl) which should be used.

Yes, grunt rtl *should* be used, but we don't know if that is what people are using (I would guess that most commiters are more likely to use grunt precommit rather than an individual task), but I also don't see the benefit in breaking things when it is so easy for us not to.

[ocean90]

In 31573:

RTL CSS generation: Switch from CSSJanus to RTLCSS.

CSSJanus (introduced in [26107]), we had a great time with you, but sadly you don't like our fancy CSS.

RTLCSS is a framework for converting CSS from LTR to RTL, same as CSSJanus, with support for more CSS properties like transform, transition or multiple box and text shadows.

Changes:

• devDependencies: Remove grunt-cssjanus, add grunt-rtlcss.
• RTLCSS uses /* rtl:ignore */ to ignore a rule, switch existing /* @noflip */ to the new directive.
• RTLCSS supports the transform property, means we can remove some ignore rules.
• RTLCSS supports string maps for custom replace rules. This commit includes a rule import-rtl-stylesheet which replaces ".css" with "-rtl.css" in URLs.

Notes for core development:

• The file generation task is still grunt rtl.
• If you have used grunt cssjanus before, use grunt rtlcss now.
• Remember the new directive /* rtl:ignore */.

fixes #31332.

Build: ​https://build.trac.wordpress.org/changeset/31554

[ocean90]

In 31802:

Update grunt-rtlcss to 1.6.0.

Changes:

• ​https://github.com/MohammadYounes/grunt-rtlcss/compare/1.5.1...1.6.0
• ​https://github.com/MohammadYounes/rtlcss/compare/1.5.1...1.6.1

see #31332.