Opened 6 years ago
Closed 5 years ago
#43466 closed enhancement (wontfix)
Add ltr admin body class
Reported by: | birgire | Owned by: | |
---|---|---|---|
Milestone: | Priority: | low | |
Severity: | normal | Version: | |
Component: | Administration | Keywords: | has-patch |
Focuses: | rtl | Cc: |
Description (last modified by )
In wp-admin/admin-header.php
we have:
if ( is_rtl() ) { $admin_body_class .= ' rtl'; }
added in [18125].
But I wonder if we should have both classes available:
$admin_body_class .= is_rtl() ? ' rtl' : ' ltr';
e.g. to help CSS or JS selectors target ltr
only.
Attachments (2)
Change History (16)
#3
in reply to:
↑ description
@
6 years ago
Replying to birgire:
e.g. to help CSS or JS selectors target
ltr
only.
Do you have a use case that would be simpler to achieve with the ltr
class?
#4
@
6 years ago
@SergeyBiryukov Thanks for checking out the other admin files.
I see that there's also no dir="ltr"
for the <html>
tag, just dir="rtl"
:
$dir_attr = ''; if ( is_rtl() ) { $dir_attr = ' dir="rtl"'; }
in wp-includes/load.php
:
I was just speculating about various possible options to avoid complicating things with PHP and is_rtl()
in inline admin styling.
Here's an example from the Health Check plugin, related to #39165 src:
<span style="display: block; width: 100%; text-align: <?php echo ( is_rtl() ? 'left' : 'right' ); ?>"> <a href="#system-information-table-of-contents" class="debug-toc"><?php esc_html_e( 'Return to table of contents' ); ?></a> </span>
Here's an example from Hello Dolly plugin (1.6):
// We need some CSS to position the paragraph function dolly_css() { // This makes sure that the positioning is also good for right-to-left languages $x = is_rtl() ? 'left' : 'right'; echo " <style type='text/css'> #dolly { float: $x; padding-$x: 15px; padding-top: 5px; margin: 0; font-size: 11px; } </style> "; }
This could be written in pure (enqueued) CSS, using .rtl
to help override float and padding directions.
There are also tools that can auto-generate the rtl stylesheet (like core is using).
There're also the textleft
, textright
, alignleft
and alignright
classes.
Another approach could be to not use an override, but use both .rtl
and .ltr
for the float and padding directions. But if that's against best practices then we shouldn't.
#5
@
6 years ago
@birgire: What would be the benefit of adding .ltr
to the body class? If I understand you correctly, the benefit would be to help CSS and JS to target specific elements directly, correct? Would the definition body::not('.rtl')
, in case of CSS, not be sufficient enough?
#6
follow-up:
↓ 9
@
6 years ago
@nielslange yes, thanks for reminding me of the :not()
CSS pseudo-class.
I had totally forgotten about it ;-)
Probably because I've used it so rarely
I wonder if that could be true for some other plugin authors as well? ;-)
ps: I noticed that Drupal 8.5 uses dir="ltr"
on the <html>
admin tag, so an alternative html[dir="ltr"]
would be possible there. But WP only uses dir="rtl"
.
#8
@
6 years ago
@birgire I created a patch for this issue. The worse thing that can happen is that the patch gets refused, right? 😉
#9
in reply to:
↑ 6
@
6 years ago
Replying to birgire:
@nielslange yes, thanks for reminding me of the
:not()
CSS pseudo-class.
I had totally forgotten about it ;-)
Probably because I've used it so rarely
In some cases, e.g. as long as ltr
hasn't been implemented (or merged), :not()
comes quite handy.
#10
follow-up:
↓ 11
@
6 years ago
@nielslange yes thanks for the patch and the :not()
reminder ;-)
ps: the ternary operator ?:
could be an alternative shorter version for if/else.
#11
in reply to:
↑ 10
@
6 years ago
Replying to birgire:
ps: the ternary operator
?:
could be an alternative shorter version for if/else.
As the conventional if else
statement had been used on several occasions I decided not to use the ternary operator
to avoid mixed coding styles.
#12
@
5 years ago
I don't think we need to add this for the dir
attributes, as ltr
is the default value for that attribute.
As for the class, it seems like :not( '.rtl' )
is a perfectly valid selector, if you ever need that. The more common pattern is to write the CSS to work in LTR, then add a smaller file of RTL rules that override the LTR rules where necessary. The examples in comment #4 should really be making use of that pattern.
#13
follow-up:
↓ 14
@
5 years ago
@pento: Thanks for your feedback!
@birgire: Guess we can close this issue then, can't we?
#14
in reply to:
↑ 13
@
5 years ago
- Keywords needs-testing removed
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from new to closed
Replying to nielslange:
@pento: Thanks for your feedback!
@birgire: Guess we can close this issue then, can't we?
@nielslange yes, as we have the :not( '.rtl' )
workaround,
and thanks for looking into this.
If we go with it, the same should be done in these files as well for consistency: