Twenty Thirteen: add RTL support

[lancewillett]

Probably OK to wait until further in the cycle, closer to freeze -- but adding it so we don't forget.

[DrewAPicture]

Noticed a few things testing RTL for something else, so I got the ball rolling with 23550.diff​.

I opted to keep the same index structure as stlye.css just to keep straight what I was overriding. As @lancewillett said in the ticket summary, this can probably wait til freeze, but here's a start.

[maor]

I know it might be a bit too early at this point, but I was going to use Twenty Thirteen on my blog anyway, so I worked a bit on the RTL stylesheet.

23550.2.diff​ extends Drew's initial patch. There's still room for improvement, but I have covered most of the RTL rules (incl. responsive design). As you may notice, I've also changed the functions.js file, since the widgets area in the footer is controlled from there (Twenty Thirteen is using jQuery Masonry, which does not enable RTL support by default but instead, offers a manual setting/switch for RTL pages - "isRTL").

Here's a quick snapshot of how it looks after 23550.2.diff​ is applied.
​http://i.imgur.com/eTcMOJb.png

[obenland]

maor: Man I was so hoping to see Twenty Thirteen on a site with content in Hebrew!

[maor]

Replying to obenland:

maor: Man I was so hoping to see Twenty Thirteen on a site with content in Hebrew!

Totally forgot to add a link. Here you go!
​http://wpm.co.il/staging/trunk

[lancewillett]

@maor Was the JavaScript change intentional? That should probably go in a different ticket / patch, please.

[maor]

Replying to lancewillett:

@maor Was the JavaScript change intentional? That should probably go in a different ticket / patch, please.

Yes, it was intentional. It's quite impossible to make the theme work under RTL environments without setting jQuery Masonry to RTL mode. Should I go ahead and create a new ticket for that, or is there an open one already?

[lancewillett]

Replying to lancewillett:

@maor Was the JavaScript change intentional? That should probably go in a different ticket / patch, please.

Oh, here is fine -- I didn't see the RTL line, I just saw the body selector improvements. :) Cool. That's fine.

[lancewillett]

@maor -- If you're around, jump into #wordpress-dev IRC Tue/Thu for Twenty Thirteen office hours, and thank you for your help on this!

​http://make.wordpress.org/core/tag/twentythirteen/ and in the sidebar is the office hours link and times.

[maor]

Perfect! It's all my pleasure!

It's a bummer I missed the office hours for today. I'll do my best to join next week.

[lancewillett]

In 23643:

Twenty Thirteen: first pass at RTL CSS and JS support. Props DrewAPicture and maor, see #23550.

[maor]

Oops! Seems like I missed the dot within the .is( ... ) conditional.

isRTL: body.is( 'rtl' ) ? true : false

Should be:

isRTL: body.is( '.rtl' ) ? true : false

Sorry about that!

[lancewillett]

In 23644:

Twenty Thirteen: RTL JS support, fix class selector, see #23550.

[lancewillett]

For the isRTL check, we might not need the tertiary notation.

Also, in other places in core WP this is the method:

isRTL: !! ( 'undefined' != typeof isRtl && isRtl )

From ​http://core.trac.wordpress.org/browser/trunk/wp-admin/js/custom-header.js?rev=23009#L12

[obenland]

Other places would be the admin, we would have to define the variable ourselves then. I'm fine with the is() check, though

[lancewillett]

In 23645:

Twenty Thirteen: simpler isRTL check for masonry, props obenland. See #23550.

[obenland]

23550.3.diff​ introduces the second pass on RTL and some minor style.css cleanup that I discovered while examining it in the RTL process. Needs testing!!

[lancewillett]

In 23674:

Twenty Thirteen: second pass at RTL support and minor style.css fixes, props obenland. See #23550.

[obenland]

Unfortunately I misinterpreted the purpose of three content attributes, which led to bugs in r23674. 23550.4.diff​ addresses that.

[lancewillett]

In 23687:

Twenty Thirteen: revert three changes to content attributes, props obenland. See #23550.

[lancewillett]

Discussing in core dev chat today, we're closing this main ticket. Lance is asking @maor and @yoav to do thorough testing for us, and they'll open new issues for new bugs.

[maor]

Hey y'all! It's been a while...

I went over the theme again this weekend and was only able to find 2 RTL issues. Both are minor.

Let me explain a bit about 23550.5.diff​.

Both fixes have to do with the pagination arrows that are showing up on attachment pages and on single posts. The thing is that the direction of the arrows should be flipped on RTL sites, otherwise it'll look quite weird (imagine this - ​http://note.io/10NbNym).

This is how the single post view looks with the patch applied. Notice that the links have been flipped to match the RTL "visual language" ​http://note.io/10CVgmj

There is the $text parmeter that is being passed on to all of these functions (previous_post_link, next_post_link, previous_image_link and next_image_link). There was one case where I couldn't use sprintf() to place the proper arrow direction because of some text that looks like a sprintf tag (Where it says %title -- '<span class="meta-nav">&larr;</span> %title'). So instead I used a if ( is_rtl() ) : ... else : ... endif; conditional.

There might be a better way to do it. I'd like to hear what you guys think about it. If I wasn't clear enough, I can try explaining it again.

Cheers!

[lancewillett]

Replying to maor:

There might be a better way to do it. I'd like to hear what you guys think about it. If I wasn't clear enough, I can try explaining it again.

Thanks Maor, for the look and your notes.

The better way to handle this is in the translation files for Hebrew (or other RTL language). There, you'd flip the arrow as needed—not in the theme source.