Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#20918 closed defect (bug) (fixed)

Tweak available theme action links for i18n purposes

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: koopersmith's profile koopersmith
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.4
Component: I18N Keywords: has-patch commit
Focuses: Cc:

Description

Small ru_RU CSS fix to visually separate the Delete link from others.

Attachments (14)

20918.png (3.8 KB) - added by SergeyBiryukov 13 years ago.
20918.patch (515 bytes) - added by SergeyBiryukov 13 years ago.
20918.after.png (3.8 KB) - added by SergeyBiryukov 13 years ago.
20918.2.patch (515 bytes) - added by SergeyBiryukov 13 years ago.
Could even use 5px
20918.after.2.png (3.7 KB) - added by SergeyBiryukov 13 years ago.
20918.diff (863 bytes) - added by nacin 13 years ago.
20918.czech.png (3.5 KB) - added by SergeyBiryukov 13 years ago.
Theme_action_links_Czech.jpg (5.6 KB) - added by pavelevap 13 years ago.
20918.noun.png (3.8 KB) - added by SergeyBiryukov 13 years ago.
Shorter string
20918.3.patch (636 bytes) - added by SergeyBiryukov 13 years ago.
20918.4.patch (669 bytes) - added by SergeyBiryukov 13 years ago.
20918.5.patch (565 bytes) - added by SergeyBiryukov 13 years ago.
20918.after.3.png (7.8 KB) - added by SergeyBiryukov 13 years ago.
20918.2.diff (731 bytes) - added by SergeyBiryukov 13 years ago.

Download all attachments as: .zip

Change History (37)

@SergeyBiryukov
13 years ago

#1 follow-up: @pavelevap
13 years ago

Same problem with Czech version - translated strings are a little bit longer...

#2 @pavelevap
13 years ago

  • Cc pavelevap@… added

#3 @nacin
13 years ago

Adding a margin-left: 7px to .available-theme .action-links .delete-theme seems to provide for enough space between the left-floated links and the right-floated links. The CSS already allows the Delete link to wrap to the next line if it has run out of space.

Will also need RTL treatment of margin-right versus margin-left.

@SergeyBiryukov
13 years ago

Could even use 5px

@nacin
13 years ago

#4 @nacin
13 years ago

How does 20918.diff look?

#5 in reply to: ↑ 1 @SergeyBiryukov
13 years ago

Replying to pavelevap:

Same problem with Czech version - translated strings are a little bit longer...

How does it look in Czech version? I've just exported the translation from GlotPress. As far as I can see, the "Live Preview" link is not translated yet, but otherwise seems fine to me: 20918.czech.png.

Replying to nacin:

How does 20918.diff look?

Same as 20918.png. My concern is that "Details" and "Delete" links are too close with no separator between them. Adding margin-left: 7px to .delete-theme has no effect in this case.

Last edited 13 years ago by SergeyBiryukov (previous) (diff)

#6 @pavelevap
13 years ago

I am using Poedit to use on testing site easily, so these strings are not in GlotPress now :-) Screenshot attached (Chrome, without patch).

Change suggested by Sergey would work perfectly also for Czech version, I guess.

@SergeyBiryukov
13 years ago

Shorter string

#7 @SergeyBiryukov
13 years ago

Looks a bit better if I translate "Live Preview" as a noun instead of a verb: 20918.noun.png. But that makes less sense in theme upgrader, since other action links are verbs there.

20918.3.patch brings the same change to cs_CZ.

#8 @nacin
13 years ago

I feel like 20918.png is fine. The lack of separator is made up by the clear contrast in color. On the other hand, since there is no actual padding/margin here, it is possible for the characters to practically touch without 20918.diff.

Also, adding right margin/padding to the left action links means that the last action link could wrap to the next line solely due to the extra padding, even if it would fit on that line. To avoid that, we can simply add left margin/padding to the delete link, which if it doesn't wrap, generates space, and otherwise looks fine when it is forced to wrap.

So I'm still good with 20918.diff. 7px could probably become 10px, at worst.

#9 @nacin
13 years ago

This is not a ru_RU or cs_CZ specific problem. It is a general i18n problem for any LTR or RTL language with long strings, like German.

#10 @pavelevap
13 years ago

I will try to tweak translation once again, but I am also not sure if specific locale style changes are best solution...

#11 @nacin
13 years ago

Something that is not okay: http://cl.ly/2n0g1T1e221V1y1J2h17

Something that would be okay: http://cl.ly/1F1g0Y1N2a0J393l002S (with 20918.diff)

(I used i's to get the links as close as possible, to illustrate a point.)

#12 @nacin
13 years ago

  • Summary changed from Tweak available theme action links for ru_RU to Tweak available theme action links for i18n purposes

#13 @SergeyBiryukov
13 years ago

Replying to nacin:

Also, adding right margin/padding to the left action links means that the last action link could wrap to the next line solely due to the extra padding, even if it would fit on that line.

There shouldn't be extra padding, since that links already have 10px padding:
http://core.trac.wordpress.org/browser/trunk/wp-admin/css/wp-admin.dev.css?rev=21057#L4435
The patch just reduces it to 5px.

I see the point with the longer links in general, 20918.diff makes sense indeed. I still feel like condensed links would look a bit better for ru_RU. Perhaps a combined patch would be feasible?

#14 @pavelevap
13 years ago

General solution would be better, but for many languages would be some more pixels better then moving Delete link to another row.

#15 follow-up: @nacin
13 years ago

There shouldn't be extra padding, since that links already have 10px padding

Padding, margin, and border are assigned to the right of a link, and they are zeroed out for the last link:

#current-theme .theme-info li:last-child,
.theme-options li:last-child,
.available-theme .action-links li:last-child {
	padding-right: 0;
	margin-right: 0;
	border-right: 0;
}

I would rather just move the Delete link inside "Details" and avoid issues with longer strings, but I don't know if that is tenable from a UX perspective.

I could be okay with a patch to reduce the padding for specific locales, in addition to 20918.diff, but there has to be a consensus on which locales and how much reduction, soon.

#16 in reply to: ↑ 15 @SergeyBiryukov
13 years ago

Replying to nacin:

Padding, margin, and border are assigned to the right of a link, and they are zeroed out for the last link:

Ah, missed that part. 20918.4.patch resets padding and margin for li:last-child.

In practice, however, this doesn't matter, since .available-theme is either 300px or 240px in width, and both states look fine without resetting: 20918.after.3.png.
Hence 20918.5.patch.

#17 @nacin
13 years ago

It will be very common for the text to wrap even with a .locale-ru-ru patch, as 240px screenshots are still shown for screens up to 1200px -- if not most monitors these days, then most browsers (assuming they are not maximizing their windows).

To me, the bottom of 20918.after.3.png is a better user experience, rather than the top. Sure, the delete link is isolated in both, but the light borders are not enough contrast themselves to differentiate the links -- the full padding seems necessary from a usability standpoint. It feels better to wrap the link than to condense everything.

#18 @SergeyBiryukov
13 years ago

I have nothing against wrapping in the bottom part of 20918.after.3.png :-)

Doesn't the top part of it seem a better UX than 20918.png though?

#19 @SergeyBiryukov
13 years ago

On a second thought, I guess I'm okay with 20918.png for 3.4.

#20 @nacin
13 years ago

  • Keywords commit added

"I think the delete link looks fine when wrapped. If it doesn't fit, it doesn't fit, man." — a very tired koopersmith

Let's go with 20918.diff for 3.4 then.

#21 @SergeyBiryukov
13 years ago

There should probably be 10px as well.

#22 @koopersmith
13 years ago

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

In [21067]:

Prevent delete theme link from bumping against theme action links. If it doesn't fit, it doesn't fit, man. props SergeyBiryukov, fixes #20918.

#23 @ryan
13 years ago

Looks good.

Note: See TracTickets for help on using tickets.