Twenty Eleven theme options page (RTL)

[rasheed]

Hello,

Please check the attached snapshot, some float right is needed at this page:

wp-admin/themes.php?page=theme_options

These changes make it look better:

.image-radio-option label {

display: block;
float: right;
margin: 0 2px 20px 30px;
position: relative;

}

17603.2.diff adds an rtl class to the admin body and makes use of that in theme-options.css. There's little point in a separate -rtl.css for as little extra css as this requires.

body.rtl

In [18125]:

Add rtl class to the admin body if is_rtl(). RTL fixes for twentyeleven theme-options.php. Props rasheed. fixes #17603

Replying to ryan:

There's little point in a separate -rtl.css for as little extra css as this requires.

What if we will have to make more RTL changes on this page? many more changes? separate rtl css is the best way to do this.

---

ooo, and i upgraded to wordpress ‎3.2-beta2-18085. the problem is not fixed. (using hebrew wordpress with ie8). i can upload a screenshot if you want.

Replying to ramiy:

What if we will have to make more RTL changes on this page?..

Actually was thinking we can combine all *-rtl.css files into one and load it on every screen. Don't see a point in having so many small separate files. This will make page loading a bit faster and simplify script-loader.

Replying to azaozz:

we can combine all *-rtl.css files into one and load it on every screen.

This will make page loading a bit faster and simplify script-loader.

It's a good idea but you have to make sure that xxx.css will have only LTR rules and xxx-rtl.css will have only RTL rules.

---

I updated to wp-‎3.2-beta2-18145, the [18125] changeset did not fixed the problem.

This was indeed fixed, you however most likely have an out of date TwentyEleven installation. Due to a bug in the upgrade code, it looks like Twenty Eleven didn't get updated when you updated Core.

If you try on a new installation, or delete the twentyeleven theme and then update again, you should find this fixed.

Replying to dd32:

This was indeed fixed, you however most likely have an out of date TwentyEleven installation. Due to a bug in the upgrade code, it looks like Twenty Eleven didn't get updated when you updated Core.

If you try on a new installation, or delete the twentyeleven theme and then update again, you should find this fixed.

dd32, i deleted the twentyeleven theme and then updated again, the problem not fixed.

We can't mirror-image the images in an RTL form. Is there a problem with what you've circled? They seem correct -- "Content on left" has the content on left, and vice versa.

Hi nacin,

• Color Scheme - Need to be aligned to right.
• Layout - Align to right. And about the pictures, in my language it's wrong. We can do something like this:

if ( !is_rtl() ) {
   echo 'sidebar-content.png';
} else {
   echo 'content-sidebar.png';
}

Rather than consider the photos wrong, let's consider the text wrong (you replaced left with right), and therefore also the theme's RTL styling wrong.

Get what I mean? What if we did no flipping at all? At most, we can then have it default to sidebar-content.

guys, i re-upgraded the theme using ftp and seems like it's working ok.

sorry for the inconvenience.

I imagine on install it defaults to content-sidebar, while in RTL it should default to sidebar-content. Let's fix that.

Note that patch will not move cleanly to WP.com, where a user can be in RTL but a blog may not.

Replying to nacin:

I imagine on install it defaults to content-sidebar, while in RTL it should default to sidebar-content. Let's fix that.

it's a good idea.

Replying to nacin:
This works. I'm just a little bit troubled by the semantics in the code, since it's not really sidebar-content on rtl, it's still content-sidebar, but RTL :)

Actually, Twenty Eleven RTL needs a bit of work (which I'm doing right now).
I think it's going to affect this - so better wait just a bit.

It was fine as is, but note the space. :-)

twentyeleven-rtl.diff updates the rtl.css file for Twenty Eleven.
It also adds proper RTL for the columns, making it easier to semantically reverse the images in the theme options.

The "Content on left/right" strings should be translated to "content on right/left" in RTL languages (we can add a comment for that string).

The rtl.css is better than before, but still needs testing.
Asked a few people to do that now, I'll update in a bit if I get any feedback.

Given that these images are the same, I don't think we need to add them multiple times.

That said, sidebar-content is just that -- sidebar on the left, content on the right. And vice versa. My patch wasn't very semantic, but it makes the code a lot less confusing. (We could also re-arrange them so sidebar-content comes first in the view in RTL.) Thoughts?

That said, sidebar-content is just that -- sidebar on the left, content on the right. And vice versa. My patch wasn't very semantic, but it makes the code a lot less confusing. (We could also re-arrange them so sidebar-content comes first in the view in RTL.) Thoughts?

Not quite right.
When you're in a rtl language mode, and you say sidebar content, you expect sidebar on the right, content on the left.

Maybe we should rename the images to say that explicitly, then we don't need the rtl versions, and can just switch the positions.
ie: sidebar-right-content-left.png / content-right-sidebar-left.png

(the last version of twentyeleven-rtl.diff doesn't change that, just a small css update over the previous version)

Replying to yoavf:

Not quite right.
When you're in a rtl language mode, and you say sidebar content, you expect sidebar on the right, content on the left.

But content-sidebar and sidebar-content is just how the data is stored and the images are named. The strings themselves can properly reflect what's going on. Two options: Change two strings and a default value, or completely hack up the theme-options page, create duplicate CSS reversing the meaning of the classes in rtl.css, etc.

I don't mind the latter, but I want to make sure we're doing it for practical reasons rather than theoretical ones.

yoavf: There is a typo in your patch. s/sans-serifl/sans-serif;/

Just to bring this thread up to date following an IRC discussion -

@nacin said:

I'm wondering why we need to switch all of it though
It seems like a waste of energy

@yoavf said:

It's not - because when an rtl developer sees a class that says sidebar-content - he expects the sidebar on the right

@nacin said:

y'all are crazy

:)

If all "y'all" RTL developers expect sidebar-content to mean a sidebar on the right I'm all for switching out images and flopping the layout. :)

in our language when we say 'sidebar-content', we start from right - first sidebar and then content. in english you start from left - first sidebat and them content.

The best sollution is to change the text "content on left" to "content - sidebar", but it's not sounds good.

The latest patch assumes:
wp-content/themes/twentyeleven/inc/images/content-sidebar.png renamed into
wp-content/themes/twentyeleven/inc/images/content-left.png
and
wp-content/themes/twentyeleven/inc/images/sidebar-content.png renamed into
wp-content/themes/twentyeleven/inc/images/content-right.png

This should make it all right - the same classes are used with both RTL and LTR, with the expected results, and no duplicate images.

One more try - with renamed images instead of duplicates

I'm kind of lost here. Many themes let you add left or right sidebar, or both. What's the difference for RTL languages?

This ticket seems to be about the theme options page needing couple small fixes that have been made in the meantime. Don't see why it should stay open.

Andrew, I piggybacked on this ticket ro resolve a larger issue.
I can open another ticket but in any case - twenty eleven RTL is currently way off ltr - and needs an update.

Go ahead yoavf and set it for 3.2.

Continued in #17882

Replying to yoavf:

Andrew, I piggybacked on this ticket to resolve a larger issue.

Much better in a new ticket IMO :)

Nothing.