WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 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:

Description

Location: wp-admin/tools.php

Please check the attachment.

Attachments (3)

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

Download all attachments as: .zip

Change History (16)

rasheed2 years ago

comment:1 ocean902 years ago

  • Keywords needs-patch added

Reproduced.

Related: #18000 / r19080

comment:2 nacin2 years ago

  • Milestone changed from Awaiting Review to 3.3

comment:3 dd322 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)

comment:5 dd322 years ago

IRC log

Ah cheers, Hadn't got to IRC yet.

dd322 years ago

dd322 years ago

comment:6 dd322 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

comment:7 azaozz2 years ago

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

comment:8 nacin2 years ago

  • Milestone changed from 3.3 to 3.3.1

comment:9 azaozz2 years ago

Looks good.

comment:10 nacin2 years ago

  • Milestone changed from 3.3.1 to 3.3.2

comment:11 nacin2 years ago

  • Milestone changed from 3.3.2 to 3.4

comment:12 nacin2 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.

comment:13 nacin2 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.