Ticket #7498 (closed enhancement: fixed)
Revised RTL CSS for 2.6
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Priority: | high | Milestone: | 2.6.1 |
| Component: | I18N | Version: | |
| Severity: | normal | Keywords: | rtl, i18n tested has-patch |
| 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
Change History
- 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.
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".
comment:3
mani_monaj — 3 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;
mani_monaj — 3 years ago
-
attachment
wp-26-rtl-rewrite-patch-IR-and-IL-nocreadit.diff
added
No credits ...
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.
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?
comment:6
mani_monaj — 3 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 — 3 years ago
-
attachment
wp-26-rtl-rewrite-patch-final-1.patch
added
Final patch for 2.6.1
There were some conflicts when applying the patch to trunk. Patch is clean with 2.6. Committing.
That's for 2.6.1. Leaving the ticket open for trunk patch.
comment:10
in reply to:
↑ 9
mani_monaj — 3 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?
comment:11
ryan — 3 years ago
Sure. Or in Codex if you like.
comment:12
ryan — 3 years ago
comment:13
follow-up:
↓ 14
ryan — 3 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?
comment:14
in reply to:
↑ 13
mani_monaj — 3 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.
comment:15
westi — 3 years ago
- Status changed from new to closed
- Resolution set to fixed
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.
