Make WordPress Core

Opened 18 years ago

Closed 18 years ago

Last modified 18 years ago

#7498 closed enhancement (fixed)

Revised RTL CSS for 2.6

Reported by: mani_monaj's profile mani_monaj Owned by:
Milestone: 2.6.1 Priority: high
Severity: normal Version:
Component: I18N Keywords: rtl, i18n tested has-patch
Focuses: Cc:

Description

All *-rtl.css files for Wordpress (currently for 2.6 branch) has been re-coded and revised. This was part of wp-persian project. The new set of CSS files are compatible with FF 2 and 3, IE 6 and 7.

Attachments (4)

wp-26-rtl-rewrite-patch.diff (31.6 KB) - added by mani_monaj 18 years ago.
wp-26-rtl-rewrite-patch-IR-and-IL.diff (31.9 KB) - added by RanYanivHartstein 18 years ago.
wp-26-rtl-rewrite-patch-IR-and-IL-nocreadit.diff (31.3 KB) - added by mani_monaj 18 years ago.
No credits …
wp-26-rtl-rewrite-patch-final-1.patch (31.4 KB) - added by mani_monaj 18 years ago.
Final patch for 2.6.1

Download all attachments as: .zip

Change History (20)

#1 @RanYanivHartstein
18 years ago

  • Keywords tested has-patch added

This is a great patch, thanks a lot on behalf of any RTLer that hasn't already thanked you.

I tested it on FF3, IE7 and IE6 and noticed a few minor issues, I'm including a patch to address them:

  • Fonts re-set back to a more generic "sans-serif", so for every script and every platform the best/most common font is chosen. This was discussed in #6616.
  • Fixed peek-a-boo bug with sidemenu in IE6
  • Added white space before submenu, to align it with the main menu and other admin widgets
  • Centered media-upload window in IE
  • Fixed submenu in media-upload window in IE (it would get offset way out of view to the left)
  • Fixed install pages in IE6 (half the page would get hidden out of view to the right)

Apart from these changes, my patch is identical to yours.

There is one issue I didn't presume to address in my patch, but I do think it's worthy discussing - the credit issue. In your patch, all the RTL files are tagged by your team, with specific credit to one Navid Kashani.

While your work was of course noteworthy and deserving of credit, placing that credit in every file has the side effect of denying credit from any other developers and testers who worked on the RTL style sheets, my self included. Since there is no reasonable way to properly credit everyone who contributed to these styles, I think it would be best to avoid this practice all together, and commit these files without those lines - like all the other files in the WordPress source.

#2 @ryan
18 years ago

I strip attribution comments whenever I come across them. We handle attribution through the "Props" comments in the commit messages which are later extracted to create a Credits list. I'll make sure the person mentioned in the comments gets "Props".

#3 @mani_monaj
18 years ago

I removed all credit lines, but the font issue is still under discussion. The current patch is still based on Ran's proposed "Sans Serif" fonts, however Iranian users rather use of Tahoma font. We are working with Ran to reach a solution for 2.6.1;

#4 @RanYanivHartstein
18 years ago

Quoting from my correspondence with mani_monaj regarding the font issue:

The font issue is not such a big deal at all [...] we can definitely live with it, and even if we can't, I'd sooner make a local override just in our distribution than force our choices on everybody else.

Regarding a long term solution, how about a third level of style sheet, unique to each locale? For e.g., for the ie.css, when rtl is activated the ie-rtl.css style sheet would get loaded, and if for e.g. the he_IL locale is set, an extra ie-he_IL.css style sheet would also get loaded.

To simplify things, this could probably be one global locale.css style sheet that will get loaded in every admin page, instead of a locale-specific version for every different style sheet.

Technically, this could be probably already be achieved using the locale.php file to add overriding styles in every admin page, but a permanent solution would be easier to use.

#5 @ryan
18 years ago

I've thought about both approaches for 2.7. Using $locale.php might not be so bad if we add some API to streamline things. That would save adding more file_exists checks. Either approach is fine by me.

So the latest patch is okay with everyone for 2.6.1?

#6 @mani_monaj
18 years ago

Ran and I discussed the issues and I'll submit the final patch now. This patch is the revised version of the first patch including Ran's proposed changes except the font issue.

@mani_monaj
18 years ago

Final patch for 2.6.1

#7 @ryan
18 years ago

There were some conflicts when applying the patch to trunk. Patch is clean with 2.6. Committing.

#8 @ryan
18 years ago

(In [8631]) RTL CSS updates from Navid Kashani, RanYanivHartstein, mani_monaj, and the WP Persian team. see #7498

#9 follow-up: @ryan
18 years ago

That's for 2.6.1. Leaving the ticket open for trunk patch.

#10 in reply to: ↑ 9 @mani_monaj
18 years ago

Replying to ryan:

That's for 2.6.1. Leaving the ticket open for trunk patch.

Thank you Ryan. In our discussion with Ran, we talked about a work group for Wordpress RTL issues. I think we (the work group) can work together for further changes. Can I post the proposal here?

#11 @ryan
18 years ago

Sure. Or in Codex if you like.

#12 @ryan
18 years ago

(In [8632]) RTL CSS updates from Navid Kashani, RanYanivHartstein, mani_monaj, and the WP Persian team. see #7498. Partial patch for trunk

#13 follow-up: @ryan
18 years ago

ie-rtl.css and rtl.css patches didn't apply, but I committed the other files for trunk. Can anyone give those a look?

#14 in reply to: ↑ 13 @mani_monaj
18 years ago

Replying to ryan:

ie-rtl.css and rtl.css patches didn't apply, but I committed the other files for trunk. Can anyone give those a look?

We are now working on CSS files in trunk. I'll post the results here.

#15 @westi
18 years ago

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

As 2.6.1 has now gone I am going to close this as fixed.

Please raise a new ticket for the remaining trunk fixes.

#16 @ryan
18 years ago

(In [8663]) Sync trunk RTL styles with 2.6 branch. see #7498

Note: See TracTickets for help on using tickets.