#17603 closed enhancement (fixed)
Twenty Eleven theme options page (RTL)
Reported by: | rasheed | Owned by: | ryan |
---|---|---|---|
Milestone: | 3.2 | Priority: | normal |
Severity: | normal | Version: | 3.2 |
Component: | RTL | Keywords: | has-patch |
Focuses: | Cc: |
Description
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;
}
Attachments (8)
Change History (48)
#1
@
13 years ago
- Cc lancewillett iandstewart added
- Keywords needs-patch added; rtl-feedback removed
- Milestone changed from Awaiting Review to 3.2
#3
@
13 years ago
- Owner set to ryan
- Resolution set to fixed
- Status changed from new to closed
In [18125]:
#4
in reply to:
↑ 2
;
follow-up:
↓ 5
@
13 years ago
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.
#5
in reply to:
↑ 4
;
follow-up:
↓ 6
@
13 years ago
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.
#6
in reply to:
↑ 5
@
13 years ago
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.
#8
follow-up:
↓ 9
@
13 years ago
- Resolution set to fixed
- Status changed from reopened to closed
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.
#9
in reply to:
↑ 8
@
13 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
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.
#10
@
13 years ago
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.
#11
@
13 years ago
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'; }
#12
@
13 years ago
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.
#13
@
13 years ago
- Keywords needs-patch removed
- Resolution set to fixed
- Status changed from reopened to closed
guys, i re-upgraded the theme using ftp and seems like it's working ok.
#15
follow-up:
↓ 17
@
13 years ago
I imagine on install it defaults to content-sidebar, while in RTL it should default to sidebar-content. Let's fix that.
#16
follow-up:
↓ 18
@
13 years ago
- Keywords has-patch added
Note that patch will not move cleanly to WP.com, where a user can be in RTL but a blog may not.
#17
in reply to:
↑ 15
@
13 years ago
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.
#18
in reply to:
↑ 16
@
13 years ago
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 :)
#19
@
13 years ago
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.
#21
@
13 years ago
- Summary changed from Theme options page (Arabic RTL) to TwentyEleven theme options page (RTL)
#22
@
13 years ago
- Summary changed from TwentyEleven theme options page (RTL) to Twenty Eleven theme options page (RTL)
It was fine as is, but note the space. :-)
#23
@
13 years ago
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).
#24
@
13 years ago
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.
#25
follow-up:
↓ 26
@
13 years ago
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?
#26
in reply to:
↑ 25
;
follow-up:
↓ 28
@
13 years ago
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
#27
@
13 years ago
(the last version of twentyeleven-rtl.diff doesn't change that, just a small css update over the previous version)
#28
in reply to:
↑ 26
@
13 years ago
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.
#30
@
13 years ago
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
:)
#31
@
13 years ago
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. :)
#32
@
13 years ago
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.
#33
@
13 years ago
The best sollution is to change the text "content on left" to "content - sidebar", but it's not sounds good.
#34
@
13 years ago
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.
#35
@
13 years ago
- Resolution set to fixed
- Status changed from reopened to closed
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.
#36
follow-up:
↓ 39
@
13 years ago
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.
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.