Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 weeks ago

#19424 closed defect (bug) (fixed)

Admin Bar has RTL issues with the new icons and search

Reported by: helenyhou Owned by: ryan
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.3
Component: Toolbar Keywords: needs-patch
Focuses: Cc:


Admin bar appears to have some issues in RTL, namely with icons and search. Seems that the icons should be on the other side (and perhaps the comment bubble tail flipped) and search placement needs a little tweaking.


Attachments (4)

19424.diff (1.3 KB) - added by koopersmith 4 years ago.
opera-rtl.patch (386 bytes) - added by azaozz 4 years ago.
19424-2.patch (956 bytes) - added by azaozz 4 years ago.
19424-3.patch (1.5 KB) - added by azaozz 4 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 @nacin4 years ago

  • Summary changed from Admin bar issues in 3.3 to Admin Bar has RTL issues with the new icons and search

@koopersmith4 years ago

comment:2 @koopersmith4 years ago

  • Owner set to koopersmith
  • Status changed from new to accepted

attachment:19424.diff makes the RTL positioning happier. Thus, I am happier.

comment:3 @koopersmith4 years ago

  • Keywords has-patch added

comment:4 @nacin4 years ago

Leading with a sidenote: In my old version of Firefox, the text looks like it is one pixel lower on the baseline than it should be. koopersmith confirmed on a current version.

I don't have an IE7 test environment, but in Opera, the icons brush up against text: http://cl.ly/3t2e1f1U2L2X0M1r0b2i. Same for frontend (no jQ) and backend. Search appears unaffected. Looks good in Opera for LTR.

In Safari and Chrome, same deal as Opera, but not as bad: http://cl.ly/1t1O1w3a3r0Z2l0T0I1C. Perhaps a pixel or two. This was not tested in Firefox but is probably present.

comment:5 @nacin4 years ago

Spoke too soon for Opera LTR. The baseline is one pixel too high. http://cl.ly/0r3C0W03050I0X2V2G2v

comment:6 @koopersmith4 years ago

Looking into these now. The baseline bugs seem to vary by browser — fixing one breaks the others. There's no special rtl line-height code, these are just browser inconsistencies.

comment:7 @koopersmith4 years ago

I do not know what the deal is with that Opera screenshot, but it makes me sad.

comment:8 @koopersmith4 years ago

  • Owner koopersmith deleted
  • Status changed from accepted to assigned

comment:9 @nacin4 years ago

  • Keywords commit added

Suggest commit on 19424.diff but the ticket should be left open for investigation for A) generic baseline issues and B) Opera issues.

comment:10 follow-up: @helenyhou4 years ago

I think Opera tries too hard to be clever and actually flip-flops left and right margins all by itself once you declare the direction of something to be rtl in CSS. If you take out direction: rtl it at least solves the icon issues in Opera, but it affects the user menu and maybe more. Not sure there's really an elegant solution.

Also, admin bar in RTL (actually, all of RTL) has Tahoma specified first. Baseline is still all over the place between browsers anyway, but that could explain the extra discrepancy. Does RTL need to be in Tahoma? That seems kind of strange - isn't Arial great for Unicode?

comment:11 in reply to: ↑ 10 ; follow-up: @SergeyBiryukov4 years ago

Replying to helenyhou:

Does RTL need to be in Tahoma? That seems kind of strange - isn't Arial great for Unicode?

Related: ticket:3136:15, #6616, #11239, #17854

comment:12 in reply to: ↑ 11 @helenyhou4 years ago

Replying to SergeyBiryukov:

Ah, thanks. Scratch that part, then.

comment:13 @nacin4 years ago

  • Keywords dev-reviewed added

Dev reviewed in IRC.

comment:14 @ryan4 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from assigned to closed

In [19551]:

Admin bar RTL fixes for icon and search styling. Props koopersmith. fixes #19424

comment:15 @ryan4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen to investigate remaining issues.

comment:16 @nacin4 years ago

  • Keywords needs-patch added; has-patch commit dev-reviewed removed

@azaozz4 years ago

comment:17 @azaozz4 years ago

In [19561]:

Fix position of icons in Opera RTL, see #19424

@azaozz4 years ago

comment:18 @ryan4 years ago

Looks good.

@azaozz4 years ago

comment:19 @azaozz4 years ago

19424-3.patch includes the changes from 19424-2.patch and also fixes the background color on the "folded" search box in IE8 and 9.

comment:20 @azaozz4 years ago

In [19566]:

Toolbar fix bg color in IE8 & 9 folded search box and IE7 search box size, see #19424

comment:21 @azaozz4 years ago

As far as I can see the only thing remaining is the Opera RTL image padding/margin swap bug. Should be fixed in the next Opera version, http://my.opera.com/community/forums/topic.dml?id=190393

comment:22 @ryan4 years ago

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

comment:23 @slackbot4 weeks ago

This ticket was mentioned in Slack in #polyglots by sergey. View the logs.

Note: See TracTickets for help on using tickets.