WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 4 months ago

#24977 closed task (blessed) (fixed)

Workflow change: automate RTL css generation

Reported by: yoavf Owned by: nacin
Milestone: 3.8 Priority: normal
Severity: major Version:
Component: RTL Keywords: has-patch
Focuses: Cc:

Description

TL;DR:

WordPress has great support for languages written from right-to-left, but that support require a lot of manual work with each release and update. We can change that by automating the creation of RTL css file using CSSJanus in the build process.

Current Status / Issues:

  • WP core css files have manually created *-rtl.css files alongside them
  • RTL css files only override LTR css, and so must be loaded in addition to the LTR css.
  • With every CSS change across WP multiple css files, matching RTL css change must be committed as well.
  • RTL css files are often out of sync.
  • There's not enough awareness of RTL support for plugins and themes.

Proposed Solution:

  • No RTL css files in the core repository.
  • RTL css files will be created during the build process.
  • RTL css files will fully replace LTR css files and will be loaded instead of LTR files when is_rtl()

This process is already active on WordPress.com for over a year now:

  • RTL css files on WordPress.com (excluding core's and themes') are auto generated on commit.
  • If is_rtl() a filter to style_loader_src looks to see if an rtl file exists (based on a predefined naming convention), and will load it instead of the LTR file.

What else:

  • Some of core's css will need to be audited to make sure it can automatically be RTLized. There are a bunch of workarounds that can be used with cssjanus in cases where automatic rtl css is problematic( i.e. @no-flip rules, and .rtl class selector)
  • There's a CSSJanus nodejs module, but no grunt plugin yet afaik
  • While this process can be relatively easily applied to core, how can we extend it to themes and plugins? (I have a few ideas, but let's get the ball rolling first)

With WordPress moving over to a grunt based build, there's no better time to do this. Let's do it!

Attachments (16)

24977.gruntfile.diff (1.8 KB) - added by yoavf 8 months ago.
implementing the cssjanus plugin in the build process
24977.change_rtl_loading.diff (1.3 KB) - added by yoavf 8 months ago.
24977.change_rtl_loading.2.diff (1.9 KB) - added by yoavf 5 months ago.
Refreshed - this takes care of loading rtl styles instead of ltr styles
24977.gruntfile.2.diff (1.3 KB) - added by yoavf 5 months ago.
Refreshed: update to the gruntfile and package files
24977.remove_rtl_css.diff (76.1 KB) - added by yoavf 5 months ago.
refreshed
24977.sidebar_fold_icon.diff (743 bytes) - added by yoavf 5 months ago.
a fix for the sidebar fold icon
24977.remove_rtl_specific_css.diff (590 bytes) - added by yoavf 5 months ago.
remove rtl specific css from wp-admin.css, no longer needed
24977.theme-editor-dir.diff (934 bytes) - added by yoavf 5 months ago.
the theme editor needs to be LTR by default. HTML is better than css for that.
24977.ui-dialogue.diff (1.7 KB) - added by yoavf 5 months ago.
24977.wp-auth-check-use-rtl.diff (646 bytes) - added by yoavf 5 months ago.
24977.farbtastic.diff (1.1 KB) - added by yoavf 5 months ago.
24977.wp-pointer.diff (1.5 KB) - added by yoavf 5 months ago.
24977.bring_back_font_localisation.diff (4.4 KB) - added by yoavf 5 months ago.
24977.stop_using_dot_rtl_class.diff (12.3 KB) - added by yoavf 5 months ago.
refreshed
24977.admin-bar.rtl.diff (609 bytes) - added by yoavf 4 months ago.
24977.css_localization.diff (8.8 KB) - added by yoavf 4 months ago.

Download all attachments as: .zip

Change History (70)

comment:1 nacin9 months ago

  • Type changed from enhancement to task (blessed)

Yes please!!

comment:2 jeherve9 months ago

  • Cc jeremy+wp@… added

comment:3 maor9 months ago

  • Cc maorhaz@… added

Huge +1 for this!

So I believe the way to start with this would be to run all RTL CSS files through CSSJanus and then look at the results, and set exceptions, if needed. At that point, when things start to look good, we can work on filtering style_loader_src.

Did I get it right? Is that the plan, more or less?

comment:4 ramiy9 months ago

  • Cc r_a_m_i@… added

Great idea yoav!

comment:5 yoavf9 months ago

@maor that can be a first step to check feasibility - but I would skip it.

Here's what I think we would need to do, in no particular order:

  • Write a grunt plugin for CSSJanus and integrate it in the build process
  • Cleanup existing (ltr) css files for any RTL specific css
  • Look into wp_default_styles and related code, and refactor the loading of the RTL css so that it's done instead of ltr css and not in addition to it.
  • (once we have the build plugin) Test newly generated rtl css files for any problems and issues, and fix any problems in source ltr files
  • Document best practices for writing core css that "flips" well and doesn't cause any issues (i.e: % based positioning instead of pixel based, etc)

comment:6 hatul9 months ago

  • Cc amiadb@… added

comment:7 mark-k9 months ago

For the sake of people reading this and are not familiar with rtling I want to point out that rtling is many times more then just replacing margin-left:10px with margin-right:10px;

I vote against, this is a good way for developers to keep not testing their code in rtl with the excuse of "the tools will handle it". This might have been a valid strategy if in theory such a tool might have been made to work 100% of the time, but we know there are things that can not, or are hard to handle automatically.
The two biggest problems I can think of right now are background position which is designed to be used only in ltr without easy translation for anything a little complex to rtl, and fliping images where needed

In addition there are JS libraries that have no support for rtl directions in the HTML when calculation position offsets.

The most basic step of good QA is code coverage - make sure each line of your code is executed in your test. With automatic rtling the developer will not test the rtl code, so who will test it? when will the rtl bugs be discovered? I think it is a real problem because AFAICT none of the main contributors run beta versions in rtl, unless there is someone dedicated for that in wordpress.com I'm afraid that rtl bugs will be discovered only after the new version was released.

comment:8 follow-up: yoavf9 months ago

@mark-k - automated css is perfectly doable - from what I know this is entirely how Google operate across all of their sites and services, and how Wikipedia is built (the PHP port of CSSJanus we use on WP.com has been done by Wikimedia).

Sure, you may have to write your (ltr) css in certain ways to minimize problems. But that's completely in line with general css best practices. In any case that's why education and documentation are a big part of this challenge.

As for the specific examples you've mentioned - CSSJanus has built in support for flipped images - and we'll make this part of our build process. Additionally, core has been and will be removing a lot of images used to style the admin. Quite a few of them are replaced by pure css and/or genericons. Background positioning can be problematic, but totally manageable either with mirrored sprites and/or percent based positioning.

With regard to testing - it won't be much different that the situation today. We have a lot of core contributors who regularly test core on RTL mode during development, and there's no reason this will change.

comment:9 in reply to: ↑ 8 ; follow-up: mark-k9 months ago

Replying to yoavf:

@mark-k - automated css is perfectly doable - from what I know this is entirely how Google operate across all of their sites and services, and how Wikipedia is built (the PHP port of CSSJanus we use on WP.com has been done by Wikimedia).

well, this is not google with serious QA department and monopolistic grab on the market, and not wikipedia in which software development is the least important part of its operation and has similar monopolistic grab on the market.

Sure, you may have to write your (ltr) css in certain ways to minimize problems. But that's completely in line with general css best practices. In any case that's why education and documentation are a big part of this challenge.

And who will enforce the new css standard? Do we do a good enough code review to catch possible style violations?

As for the specific examples you've mentioned - CSSJanus has built in support for flipped images - and we'll make this part of our build process. Additionally, core has been and will be removing a lot of images used to style the admin. Quite a few of them are replaced by pure css and/or genericons. Background positioning can be problematic, but totally manageable either with mirrored sprites and/or percent based positioning.

background:url(image.png) top 30%; % rtl != background:url(image.png) top 70%;

the reason is that image are drawn ltr and there is no way to specify drawing rtl, therefor in the example above you will not get 30% margin on the right. You actually have to know the image width to handle this kind of transformation, and how do you get it when the image is part of a sprite?

As for genricons, flipping images there will be lots of fun. will the tool chain generate font files automatically?, cause I am not sure what tools should be used for that at all.

With regard to testing - it won't be much different that the situation today. We have a lot of core contributors who regularly test core on RTL mode during development, and there's no reason this will change.

testing rtl without actually blogging in rtl is problematic. How do they know what to test, what to look for? I never seen a QA plan in the WP development process....

Summery: I think there are theoretical hurdles and I suspect there will be practical ones as well. IMO The 3.7 time frame is too short for that. I suggest to break it in two, in 3.7 do the cleanup and education but introduce the tool as part of the build chain only in 3.8

comment:10 in reply to: ↑ 9 helen9 months ago

Replying to mark-k:

And who will enforce the new css standard? Do we do a good enough code review to catch possible style violations?

I will, and I do.

testing rtl without actually blogging in rtl is problematic. How do they know what to test, what to look for? I never seen a QA plan in the WP development process....

I know what you mean by "they", but there really isn't a "they" here. It's "we". It's true that it seems that most of the prolific CSS patchers are not using RTL in a native way, but I don't think the solution is to lament that - it's to get more eyes involved.


I think yoavf's outline of what would need to be done sounds pretty good. We should at least try it and not spend time predicting what could be less than perfect. Otherwise we'll never move forward on anything.

comment:11 follow-up: DrewAPicture9 months ago

Should this be on the 3.8 milestone?

comment:12 in reply to: ↑ 11 ; follow-up: nacin9 months ago

Replying to DrewAPicture:

Should this be on the 3.8 milestone?

No, I'd like this to happen in 3.7. I think we should/can move quickly on this.

Here is the node.js module for CSSJanus. Anyone up for writing a Grunt plugin? https://github.com/trevorparscal/cssjanus

comment:13 in reply to: ↑ 12 yoavf9 months ago

Here is the node.js module for CSSJanus. Anyone up for writing a Grunt plugin? https://github.com/trevorparscal/cssjanus

Taking a shot at this :)

comment:14 follow-up: ocean909 months ago

Seems like CSSJanus needs to be extended for box-shadows and gradients.

comment:15 alex-ye9 months ago

  • Cc nashwan.doaqan@… added

comment:16 in reply to: ↑ 14 ; follow-up: mark-k8 months ago

Replying to ocean90:

Seems like CSSJanus needs to be extended for box-shadows and gradients.

AFAICT cssjanus is semi abandonware, it probably doesn't support right now anything in CCS3 which is not a simple right and left replacement.
The list above can be extended to text shadows, transitions, border image, multiple background images etc. Probably not a big deal as long as the admin supports old IE versions

comment:17 follow-up: mark-k8 months ago

@nacin, I'm far from being a GPL zealot, but it seems to me that since the tool chain will include a unique ltr to rtl compiler it should be made available to download, otherwise people will not be able to hack core and generate a working rtl version of the hack.

comment:18 in reply to: ↑ 17 ; follow-up: yoavf8 months ago

Replying to mark-k:

@nacin, I'm far from being a GPL zealot, but it seems to me that since the tool chain will include a unique ltr to rtl compiler it should be made available to download, otherwise people will not be able to hack core and generate a working rtl version of the hack.

Not sure if I understand what you refer to here - everything related will be included in http://develop.svn.wordpress.org, including the grunt plugin and existing modules. The grunt plugin will be released as well, for others to use too.

comment:19 in reply to: ↑ 16 yoavf8 months ago

AFAICT cssjanus is semi abandonware, it probably doesn't support right now anything in CCS3 which is not a simple right and left replacement.
The list above can be extended to text shadows, transitions, border image, multiple background images etc. Probably not a big deal as long as the admin supports old IE versions

I wouldn't consider it abandonware - the php port is actively update in the mediawiki project, and there's no reason not to update the nodejs module as well. If you looking at the source, note that even if some css properties are not specifically declared, they still might get proper rtl conversion if they match one of the general patterns.

I don't doubt we'll encounter some bugs or missing features in cssjanus - but it's the best starting point, and we're equipped to fix those bugs, contribute upstream, and make everything better :)

comment:20 in reply to: ↑ 18 ; follow-up: mark-k8 months ago

Not sure if I understand what you refer to here - everything related will be included in http://develop.svn.wordpress.org, including the grunt plugin and existing modules. The grunt plugin will be released as well, for others to use too.

That is nice that everything is included there, but does it say somewhere in the codex that you need to get it in order to hack wordpress? I assume that even now with the minifying bot it violates the spirit of of the GPL as you can't produce exactly the same software from source code (php, js and css) if you don't know about the tools.
googling for "hacking wordpress" brings this outdated page as top result http://codex.wordpress.org/Hacking_WordPress

comment:21 in reply to: ↑ 20 ; follow-up: yoavf8 months ago

Replying to mark-k:

but does it say somewhere in the codex that you need to get it in order to hack wordpress?

We're lucky the codex is a wiki that can easily be edited :-). As soon as this drops in trunk, we'll update all the documentation accordingly, and write whatever guides need to be written.

I assume that even now with the minifying bot it violates the spirit of of the GPL as you can't produce exactly the same software from source code (php, js and css) if you don't know about the tools.

well, it's our role to make sure everyone knows about what tools are available and how to use them. Re minification, we still release the unminified css/js in the distributions. RTL css files will be included as well, if anyone want's to manually modify them for any reason. Anyway. I don't see the point here.

The only thing that's going to change with regard to RTL is that we significantly decrease the amount of manual work required to produce a top quality RTL experience.

Last edited 8 months ago by yoavf (previous) (diff)

yoavf8 months ago

implementing the cssjanus plugin in the build process

comment:22 yoavf8 months ago

First version of the css grunt-cssjanus plugin can be found at https://github.com/yoavf/grunt-cssjanus
It's my first node/grunt module, mostly based on other modules and examples - so any feedback is appreciated :)

It hasn't been published yet - if you'd like to test it, here's how to, in a checkout of the develop.svn repository:

  1. Apply 24977.gruntfile.diff
  2. run npm install https://github.com/yoavf/grunt-cssjanus/archive/master.tar.gz
  3. grunt build

Note that if you haven't deleted the existing rtl css files, they'll be flipped too :) (see 24977.remove_rtl_css.diff)

Again, any feedback appreciated.

Last edited 8 months ago by yoavf (previous) (diff)

comment:23 in reply to: ↑ 21 ; follow-up: mark-k8 months ago

Replying to yoavf:

Replying to mark-k:

but does it say somewhere in the codex that you need to get it in order to hack wordpress?

We're lucky the codex is a wiki that can easily be edited :-). As soon as this drops in trunk, we'll update all the documentation accordingly, and write whatever guides need to be written.

I assume that even now with the minifying bot it violates the spirit of of the GPL as you can't produce exactly the same software from source code (php, js and css) if you don't know about the tools.

well, it's our role to make sure everyone knows about what tools are available and how to use them. Re minification, we still release the unminified css/js in the distributions. RTL css files will be included as well, if anyone want's to manually modify them for any reason. Anyway. I don't see the point here.

Well, then why can't I find the minification tool neither in google nor in trac? It is kind of pointless to have a source code if I don't have the compiler used to create the executable.

Practically, people still need to test how their CSS changes will work in rtl, especially if they are trying to fix an rtl bug, therefor they need to be able to do a a complete build, therefor all the tools required should be available in a very accessible way.

The only thing that's going to change with regard to RTL is that we significantly decrease the amount of manual work required to produce a top quality RTL experience.

I totally disagree but I will be very happy if I will be proved wrong.

comment:24 in reply to: ↑ 23 ; follow-up: helen8 months ago

Replying to mark-k:

Well, then why can't I find the minification tool neither in google nor in trac? It is kind of pointless to have a source code if I don't have the compiler used to create the executable.

Right now it uses the YUI Compressor. I imagine it's not in the develop repo yet because that repo is just a couple days old and tools and such are still under development. This isn't some magical "here it's already done" process. Tools are developed just as core is developed.

Practically, people still need to test how their CSS changes will work in rtl, especially if they are trying to fix an rtl bug, therefor they need to be able to do a a complete build, therefor all the tools required should be available in a very accessible way.

That's exactly what the goal is. This is a very strange thing to be arguing about.

I think it would be much more helpful to see time being spent trying this out and gathering data rather than assuming it couldn't possibly work. Trying something out is not saying that it will definitely happen.

comment:25 in reply to: ↑ 24 ; follow-up: mark-k8 months ago

Replying to helen:

Replying to mark-k:

Well, then why can't I find the minification tool neither in google nor in trac? It is kind of pointless to have a source code if I don't have the compiler used to create the executable.

Right now it uses the YUI Compressor. I imagine it's not in the develop repo yet because that repo is just a couple days old and tools and such are still under development. This isn't some magical "here it's already done" process. Tools are developed just as core is developed.

I don't want to argue philosophy here as don't care about GPL and prefer BSD, but the GPL way implies that you need to specify all the build tools required and the minimizer was used for years without any documentation.

Practically, people still need to test how their CSS changes will work in rtl, especially if they are trying to fix an rtl bug, therefor they need to be able to do a a complete build, therefor all the tools required should be available in a very accessible way.

That's exactly what the goal is. This is a very strange thing to be arguing about.

I think it would be much more helpful to see time being spent trying this out and gathering data rather than assuming it couldn't possibly work. Trying something out is not saying that it will definitely happen.

Not everything is possible, and some goals are possible but have bad side affects. I think these should be pointed out as early as possible in the development cycle to at least try in advance to deal with it. So far I don't think I repeated a point that I made before, and I don't think that what I say is pointless rhetoric.

ok, end of this discussion for me until there will be an actual working rtl conversion tool.

comment:26 yoavf8 months ago

How to test this:

Prerequisites: nodejs and grunt

$ mkdir rtltest
$ cd rtltest
$ svn co http://develop.svn.wordpress.org/trunk .
$ wget http://core.trac.wordpress.org/raw-attachment/ticket/24977/24977.gruntfile.diff
$ patch -p0 -i 24977.gruntfile.diff
$ npm install grunt-cssjanus
$ npm install
$ cd src
$ rm wp-includes/css/*-rtl.css
$ rm wp-admin/css/*-rtl.css
$ wget http://core.trac.wordpress.org/raw-attachment/ticket/24977/24977.change_rtl_loading.diff
$ patch -p0 -i 24977.change_rtl_loading.diff
$ cd ..
$ grunt build

a build directory will be created. It contains a clean WP core build, with the following changes applied:

  1. Existing manually created rtl css files removed
  2. Auto generated RTL css files added
  3. Core RTL css files will be loaded instead of LTR css files (as defined by the wp_default_styles() function ), if the text direction is RTL

If you'd like to quickly test this build directory in RTL mode, you can use the RTL Tester plugin
As you'll quickly see, everything looks mostly OK - magic! However, there are a bunch of issues with how the admin looks in this case - in most cases related to sprites.

This is one of things to do next - each existing LTR css file needs to be audited to make sure sprites are used properly. In some cases, we might want to patch cssjanus itself if it isn't doing it's job properly.

Last edited 8 months ago by yoavf (previous) (diff)

comment:27 in reply to: ↑ 25 ; follow-up: nacin8 months ago

Replying to mark-k:

Replying to helen:

Replying to mark-k:

Well, then why can't I find the minification tool neither in google nor in trac? It is kind of pointless to have a source code if I don't have the compiler used to create the executable.

Right now it uses the YUI Compressor. I imagine it's not in the develop repo yet because that repo is just a couple days old and tools and such are still under development. This isn't some magical "here it's already done" process. Tools are developed just as core is developed.

I don't want to argue philosophy here as don't care about GPL and prefer BSD, but the GPL way implies that you need to specify all the build tools required and the minimizer was used for years without any documentation.

So, two points:

  1. As of 3.7, YUI Compressor is no longer used, and bumpbot is in retirement. uglify.js and cssmin are now leveraged via grunt. These are external dependencies as per Gruntfile.js, and are themselves open source software of course.
  1. http://wordpress.org/download/source/ is linked from the bottom of core's license.txt (as a written offer, as allowed by the GPL, for source items where only the built version is shipped). That page does mention YUI Compressor.

comment:28 nacin8 months ago

(I'll also mention that the actual sources available through http://wordpress.org/download/source/ are a release or two out of date, and that's my fault.)

comment:29 nacin8 months ago

yoavf: This is fantastic. Let's start looking at how we can fix up the sprites. At some point soon I suggest we commit this and get it out there so others can see what's broken and contribute.

I think grunt-cssjanus should be in devDependencies in package.json. But that might actually lead to a larger issue, that maybe it *isn't* just a development dependency, as RTL is not usable without building. When we did develop.svn, we wanted /src to be able to be executable as-is. This was really important as we didn't want to make it more difficult to contribute, and we also wanted to make it easy to run a "development" version of WordPress, as in no need to even turn on SCRIPT_DEBUG. koopersmith can talk a bit more about this, and I've let him know about this thread.

The issue here is that you can't run /src in RTL, unless we also committed the built RTL files to /src. We've previously discussed this for things like a CSS preprocessor. If we adopted, say, SCSS, we would still want to commit the compiled CSS to /src. It's possible that in this case we'd still want to commit the -rtl.css files to core. The problem is we'd then be conflating raw source with some kind of build output. So let's see what koopersmith thinks.

A note though, this by no means prevents this from occurring for 3.7, nor does it really affect the current patches or any future work on this project. It just affects our future workflows.

comment:30 in reply to: ↑ 27 mark-k8 months ago

Replying to nacin:

So, two points:

  1. As of 3.7, YUI Compressor is no longer used, and bumpbot is in retirement. uglify.js and cssmin are now leveraged via grunt. These are external dependencies as per Gruntfile.js, and are themselves open source software of course.
  1. http://wordpress.org/download/source/ is linked from the bottom of core's license.txt (as a written offer, as allowed by the GPL, for source items where only the built version is shipped). That page does mention YUI Compressor.

I don't want to hijack this ticket for an unrelated subject that I don't care too much about, but from those places I could not see which YUI version was used to generate 3.1 therefor I can't generate exact copy of 3.1 with that info without guessing or doing investigative work.
In this case it is actually worse then the GPL discussion because version control 101 is to have all the tools you use available in an exact way externally, or even better, tied to a specific version in your VC tool. If yahoo decides to stop maintaining the YUI, how will you generate 3.6 and earlier after it will be deprecated and probably removed from the build machine during the 3.7 cycle?
Of course, having the correct YUI version is just the beginning, there is no documentation that I could find of the bot being used to activate the minimization.

ok, if there is a will we can continue in #25117

comment:31 follow-up: y0natan8 months ago

Just FYI: automatic flipping of background-positions with unit lengths is possible in modern browsers (Chrome, FF, IE9+), with four-value position syntax.

I have a fork of the python cssjanus which does just that at https://github.com/yonatan/rtl-toolkit

It turns

background: url(image.jpg) 12px top;

into

background: url(image-rtlx.jpg) 12px top;background-position:right 12px top 0%;

The first background declaration swaps the image with a horizontally flipped version and leaves in the original position for IE8, which requires some javascript after page load to measure and reposition background images. The background-position that follows does the right thing in modern browsers.

Unfortunately it doesn't support a lot of CSS3 properties, and CSSJanus' regexp-centric approach makes it a little difficult to maintain and improve.

comment:32 in reply to: ↑ 31 mark-k8 months ago

Replying to y0natan:

It turns

background: url(image.jpg) 12px top;

into

background: url(image-rtlx.jpg) 12px top;background-position:right 12px top 0%;

There are two problematic things here

  1. If the automated process does the flipping, how does it know which images to flip? some things like the wordpress logo should not be flipped
  2. I don't understand how your CSS solves the problem, sure you can position the left edge of the image 12px from the right edge of the containing block, but what you need is to position the right edge of the image 12x from the right edge of the containing block. Am I missing anything?

comment:33 follow-up: y0natan8 months ago

  1. Ideally logos and other content would use an <img> tag. If CSS backgrounds have to be excluded a /* @noflip */ comment can be inserted before them for CSSJanus.
  1. Yes. background-position:right 12px top 0%; will position the right edge of the image 12px from the right edge of the containing block.

http://jsfiddle.net/yonatan/QuZ2p/
https://developer.mozilla.org/en-US/docs/Web/CSS/position_value
edit: this is better than the previous link:
http://dev.w3.org/csswg/css-backgrounds/#the-background-position

Last edited 8 months ago by y0natan (previous) (diff)

comment:34 in reply to: ↑ 33 mark-k8 months ago

Replying to y0natan:

/* sorry for the spam, deleted */

  1. Yes. background-position:right 12px top 0%; will position the right edge of the image 12px from the right edge of the containing block.

http://jsfiddle.net/yonatan/QuZ2p/
https://developer.mozilla.org/en-US/docs/Web/CSS/position_value

cool, tnx

Last edited 8 months ago by mark-k (previous) (diff)

comment:35 nacin7 months ago

  • Milestone changed from 3.7 to 3.8

Moving to 3.8 for better review and proper testing. CSS is likely to get a major overhaul due to MP6, so that would be a good time for this.

comment:36 yoavf5 months ago

Watching over #25858 - once mp6 is merged, I'll do my best to make this happen.

yoavf5 months ago

Refreshed - this takes care of loading rtl styles instead of ltr styles

yoavf5 months ago

Refreshed: update to the gruntfile and package files

yoavf5 months ago

refreshed

comment:37 follow-up: yoavf5 months ago

With MP6 now in core, I refreshed the above files and happy to report that almost everything just works out of the box!

Two small issues that I was able to find:

  1. The collapse button for the admin sidebar needs to be mirrored.
  2. I could spot some scrollbars in the add media modal window in RTL mode.

yoavf5 months ago

a fix for the sidebar fold icon

yoavf5 months ago

remove rtl specific css from wp-admin.css, no longer needed

yoavf5 months ago

the theme editor needs to be LTR by default. HTML is better than css for that.

comment:38 in reply to: ↑ 37 yoavf5 months ago

Replying to yoavf:

With MP6 now in core, I refreshed the above files and happy to report that almost everything just works out of the box!

Two small issues that I was able to find:

  1. The collapse button for the admin sidebar needs to be mirrored.

24977.sidebar_fold_icon.dif takes care of that

  1. I could spot some scrollbars in the add media modal window in RTL mode.

Can't reproduce, so maybe was something local.

comment:39 follow-up: nacin5 months ago

In 26107:

Use CSSJanus via a Grunt task to generate right-to-left CSS.

RTL files are now created on build for core CSS files. These files replace the LTR file completely, rather than being in addition to the existing LTR file.

Benefits:

  • For the user: less CSS is served in RTL, less HTTP requests on the frontend, and less work for the browser.
  • For the core developer: Let the tools do the work.

Notes for core development:

  • The file generation task is grunt rtl.
  • grunt watch now handles generating RTL files in /build when a CSS file in /src is saved.
  • /src is now locked to LTR. RTL testing must occur via /build. When attempting to run an RTL text direction with /src, an admin notice will display.

Expect RTL bugs. Please report them.

props yoavf.
see #24977.

comment:40 in reply to: ↑ 39 yoavf5 months ago

Nacin in irc:

so, next up would be untangling the .rtl stuff.
so, two groups of oddities.
one is files that janus wanted to add RTL bits to, but didn't have any RTL to begin with: buttons, jquery-ui-dialog, wp-auth-check
the other is ones that use .rtl - farbtastic, install, editor, wp-pointer
finally, colors-fresh uses some RTL, which I imagine should be pushed into wp-admin if at all possible (could specify border-color: instead of border-left-color:, assuming border-left-size: gets switched to border-right-size: in wp-admin.css, for example)

wp-auth-check.css the generated RTL should be used.

jquery-ui-dialog.css : most "flips" here should be skipped - but some are correct. All the .ui-resizable-xx should not be flipped, since they refer to the cursor changes at the various borders of the UI window. However, the float on .ui-dialog .ui-dialog-title should be flipped. A few of the rules are overwritten with .rtl calls in editor.css and need to be cleaned up there. I'll attach a patch for this file

buttons.css: There's one small minor rule that requires RTL margin-right: -1px; in .wp-core-ui .button-group > .button. There's another one in .wp-core-ui input[type="reset"],[...] where we set both padding-right and padding-left separately but those can be combined. Not sure about this one.

yoavf5 months ago

comment:41 yoavf5 months ago

farbtastic: let's dump the .rtl rules and use the auto generated file

yoavf5 months ago

comment:42 yoavf5 months ago

wp-pointer: same here: let's dump the .rtl rules and use the auto generated files.

yoavf5 months ago

comment:43 nacin5 months ago

In 26128:

Fix new RTL file loading when style concatenation is enabled.

see #24977.

comment:44 yoavf5 months ago

There's an bug with cssjanus (nodejs module) that's visible when you look at the new widgets screen. I've opened up a pull request with a fix:
https://github.com/trevorparscal/cssjanus/pull/3

comment:45 yoavf5 months ago

24977.stop_using_dot_rtl_class.diff Removes .rtl classes from all the wp-admin/css and wp-includes/css files, and sort out the loading of their RTL equivalents.

yoavf5 months ago

refreshed

comment:46 yoavf5 months ago

  • Keywords has-patch added

The following two patches should be ready to commit:

24977.bring_back_font_localisation.diff Brings back font localization in wp-admin. Tahoma for RTL, with the exclusion of he_IL
24977.stop_using_dot_rtl_class.diff Removes .rtl classes and use full rtl files instead

comment:47 nacin5 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 26243:

Stop using .rtl classes, instead relying on flipping LTR CSS to RTL.

props yoavf.
fixes #24977.

comment:48 nacin5 months ago

For 24977.bring_back_font_localisation.diff, I think we'll need to move some of these to their original stylesheets, like buttons.css for button-primary, button-secondary, etc. The media-views.css file can be loaded on the frontend, and that includes buttons.

comment:49 nacin4 months ago

  • Resolution fixed deleted
  • Severity changed from normal to critical
  • Status changed from closed to reopened

We never committed this last patch — RTL fonts need Tahoma (except Hebrew, which is better in Arial).

These additional files were backed with Tahoma rules in 3.7, to get an idea for where the rules may need to return:

  • src/wp-admin/css/install.css
  • src/wp-admin/css/media.css
  • src/wp-includes/css/admin-bar.css
  • src/wp-includes/css/editor.css

comment:50 nacin4 months ago

  • Component changed from General to RTL

comment:51 nacin4 months ago

In 26783:

RTL should use Tahoma, Hebrew should use Arial.

props yoavf.
see #24977.

comment:52 nacin4 months ago

  • Severity changed from critical to major

Still needs some work (yoavf is on it today), but I wanted to get this in for testing reasons.

yoavf4 months ago

comment:53 yoavf4 months ago

24977.css_localization.diff: do font resets for RTL in the respective files

Re wp-includes/js/tinymce/themes/advanced/skins/wp_theme/dialog.css:
tinymce popups don't include language indicators so we can set language specific css. We do have RTL direction as an HTML attribute, so that's ok.

comment:54 nacin4 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 26816:

RTL font-family overrides. (Tahoma generally, and Arial for Hebrew.)

props yoavf.
fixes #24977.

Note: See TracTickets for help on using tickets.