WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

#18000 closed defect (bug) (fixed)

Press This link (button) - RTL - Arabic

Reported by: rasheed Owned by: yoavf
Milestone: 3.3 Priority: normal
Severity: trivial Version: 3.2
Component: RTL Keywords: has-patch
Focuses: Cc:

Description

Hello,

Please check the attachment.

1- The current situation.
2- How it should look in RTL.

The button needs some padding + change background position.

Attachments (4)

press-this.gif (4.4 KB) - added by rasheed 3 years ago.
press-this-rtl.png (561 bytes) - added by jkudish 3 years ago.
18000.diff (31.4 KB) - added by jkudish 3 years ago.
here's the patch again (sorry first one failed for some reason)
rtl-press-this.diff (1.0 KB) - added by yoavf 3 years ago.

Download all attachments as: .zip

Change History (13)

rasheed3 years ago

jkudish3 years ago

comment:1 jkudish3 years ago

  • Cc joachim.kudish@… added
  • Keywords has-patch added
  • Owner set to jkudish
  • Severity changed from normal to trivial
  • Status changed from new to accepted
  • Type changed from defect (bug) to enhancement

There previously was a note that said that press this was intentionally not RTLized, not sure why though, so I decide to provide a patch anyways...

Also attached is press-this-rtl.png which is the same icon, but without the sprite that the non-RTL version has (otherwise the little hand shows up on the button). Not sure what the hand is used for, so I chose to ommit it for RTL only.

jkudish3 years ago

here's the patch again (sorry first one failed for some reason)

comment:2 andrewryno3 years ago

Related: #17184

I think that the intentional non-RTL was for the old button. Doesn't seem like they added that line during the redesign.

comment:3 ocean903 years ago

  • Milestone changed from Awaiting Review to 3.3

Do we really need a new image? I think the padding is enough.

Also, when submitting patches, only include the .dev versions of css and js files, the commitor will re-minify it afterwards.

comment:4 jkudish3 years ago

yes it was necessary because otherwise the rest of the sprite shows up.

thanks for the tip with .dev versions, didn't know about that one. Is there a way to tell WP to load those up instead of the minified versions when testing?

comment:5 ocean903 years ago

Just add define('SCRIPT_DEBUG', true); to your wp-config file.

Oh, didn't know that it's a sprite, is the hand cursor be used? I don't think so.

comment:6 jkudish3 years ago

Thanks for the little define, didn't know about that one :)

I don't think it's used anymore anywhere. It may be a good idea to just change that image and it be universal (not just RTL)

comment:7 yoavf3 years ago

  • Owner changed from jkudish to yoavf
  • Status changed from accepted to assigned
  • Type changed from enhancement to defect (bug)

This is actually a defect not an enhancement. I have no idea what I was thinking when didn't RTLize the press-this button in the previous release.
Anyway - attaching a refreshed and more complete patch - no need for a new image.

yoavf3 years ago

comment:8 downloadbook3 years ago

Also attached is press-this-rtl.png which is the same icon, but without the sprite that the non-RTL version has http://www.synet.net/(otherwise the little hand shows up on the button). Not sure what the hand is used for, so I chose to ommit it for RTL only.

Version 0, edited 3 years ago by downloadbook (next)

comment:9 azaozz2 years ago

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

In [19080]:

RTL fixes for 3.3, props yoavf SergeyBiryukov rosshanney, see #19042 fixes #18000, fixes #17988, fixes #19006

Note: See TracTickets for help on using tickets.