Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#19502 closed defect (bug) (fixed)

"Press this" icon overlaps the text (RTL)

Reported by: rasheed Owned by: nacin
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.3
Component: RTL Keywords: has-patch
Focuses: Cc:


Location: wp-admin/tools.php

Please check the attachment.

Attachments (3)

Untitled-1.gif (10.2 KB) - added by rasheed 6 years ago.
19502.diff (944 bytes) - added by dd32 6 years ago.
press-this.png (567 bytes) - added by dd32 6 years ago.

Download all attachments as: .zip

Change History (16)

6 years ago

#1 @ocean90
6 years ago

  • Keywords needs-patch added


Related: #18000 / r19080

#2 @nacin
6 years ago

  • Milestone changed from Awaiting Review to 3.3

#3 @dd32
6 years ago

Works for me in Hebrew, as it has a short string, however has issues in Arabic (which has a long string).

The sprite doesn't seem wide enough for RTL, there needs to be extra room in the sprite between the hand icon and the PressThis icon. However I can't see whats actually using the hand icon part of the sprite.. Am I missing something?

(patch incoming)

#5 @dd32
6 years ago

IRC log

Ah cheers, Hadn't got to IRC yet.

6 years ago

6 years ago

#6 @dd32
6 years ago

  • Keywords has-patch added; needs-patch removed

RTL on the Press This icons wasn't perfect in 3.2 either, but it didn't have an icon obscuring the text.

New image based on the fact that the Hand isn't used at all, would've liked to use blocks of 5px for positioning, but it doesn't quite work out like that for RTL offsets - Image is a crop of the one in core saved without compressed/colour/pixel alteration

The .pressthis a span width is required for RTL languages with a smaller character count - the span shrinks, which results in the icon being mis-placed, defined it globally for consistency.

Patch tested with Opera/IE/FF/Chrome (latest in all cases) with English, Arabic, and Hebrew

#7 @azaozz
6 years ago

  • Summary changed from "Press this" icon everlaps in text (RTL) to "Press this" icon overlaps the text (RTL)

#8 @nacin
6 years ago

  • Milestone changed from 3.3 to 3.3.1

#9 @azaozz
6 years ago

Looks good.

#10 @nacin
6 years ago

  • Milestone changed from 3.3.1 to 3.3.2

#11 @nacin
6 years ago

  • Milestone changed from 3.3.2 to 3.4

#12 @nacin
6 years ago

Since we no longer enforce a width here ([20662]), the existing patch didn't quite work right.

By adding left and right margins of five pixels to the span, and decreasing the left and right paddings by five pixels, we get the same visual, but we no longer need a background x position of 5px. This enables us to then have a background x position of "right" in RTL.

Looks great regardless of text size.

#13 @nacin
6 years ago

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

In [20689]:

Fix the image overlap with the Press This bookmarklet in RTL. props dd32 for the initial patch. fixes #19502.

Note: See TracTickets for help on using tickets.