Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#22724 closed defect (bug) (fixed)

Context for Random and checkbox position

Reported by: pavelevap's profile pavelevap Owned by: nacin's profile nacin
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description

We should have context for "Random", because its meaning was changed a little:

1) Order by: "Random" (one of dropdown options in Links widget)

2) "Random" - New checkbox for galleries (before it was also dropdown option).

And I am not sure if checkbox should be centered? See attached screenshot.

Attachments (7)

random_context.patch (1.2 KB) - added by pavelevap 12 years ago.
Random_checkbox_position.jpg (5.3 KB) - added by pavelevap 12 years ago.
22724.patch (426 bytes) - added by SergeyBiryukov 12 years ago.
22724.2.patch (374 bytes) - added by SergeyBiryukov 12 years ago.
22724.3.diff (416 bytes) - added by koopersmith 12 years ago.
IE9_Gallery_checkbox.jpg (5.9 KB) - added by pavelevap 12 years ago.
22724.diff (380 bytes) - added by helenyhou 12 years ago.

Download all attachments as: .zip

Change History (30)

#1 @SergeyBiryukov
12 years ago

  • Keywords has-patch i18n-change added
  • Milestone changed from Awaiting Review to 3.5

#2 @nacin
12 years ago

  • Component changed from Media to I18N
  • Keywords 2nd-opinion added

The "Random" string was used in the same situation in 3.4 — links widget and galleries. Can we get away with releasing 3.5 then splitting this string in 3.6? Are you asking because it is imperative to split this string in your language, or are you asking because other translators might need it?

Go with the translation for "Random" that makes sense for galleries. The Links widget instance is fairly well hidden, and the widget is disabled for new installs anyway.

#3 @pavelevap
12 years ago

"Random" string was used in other situation for galleries in 3.4. It was also in dropdown widget, so suggested context change was not needed. But in 3.5 context was changed, so it is some kind of regression. Now there is not "Sorted by: Random" (so it can be asked How?), but only Random (a different meaning, noun). It is problem in my language, but may be also in other languages, I guess... But I am not sure, maybe we could ask some other users.

No problem waiting for 3.6, but there were too many changes after string freeze that it could also happen in 3.5.

#4 @vanillalounge
12 years ago

It's one string and it makes a difference. I vote for pushing it to 3.5 and warning the polyglots.

#5 @nacin
12 years ago

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

In 23021:

Add context to the 'Random' string. It is now used in two places: gallery order and the links widget. props pavelevap, fixes #22724.

#6 @nacin
12 years ago

  • Keywords 2nd-opinion removed

As requested. Thanks guys.

#8 @pavelevap
12 years ago

Nice, thank you, nacin!

#9 @pavelevap
12 years ago

And what about checkbox position? Shoud be centered? Please see attached screenshot.

Or should I reopen this ticket?

#10 @SergeyBiryukov
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

22724.patch is an attempt to fix the checkbox position.

#11 @nacin
12 years ago

  • Component changed from I18N to Media

#12 @nacin
12 years ago

There is already a .media-sidebar .setting input[type="checkbox"] { rule specifying margin-top: 10px. Can we combine them?

#13 @nacin
12 years ago

Please see attached screenshot.

There's no attached screenshot. When does this occur? It looks fine for me and koopersmith in Chrome.

#14 @nacin
12 years ago

Okay, Firefox.

@koopersmith
12 years ago

#15 @koopersmith
12 years ago

  • Keywords commit added

Latest fixes vertical alignment a bit. Otherwise, solid.

#16 @ocean90
12 years ago

22724.3.diff​ looks good in LTR and RTL.

#17 @nacin
12 years ago

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

In 23034:

Media: Fix vertical and horizontal alignment of the gallery "Random" checkbox, and checkbox settings in general. Includes RTL. props koopersmith. fixes #22724.

#18 @nacin
12 years ago

[23034] also fixed #22737, accidentally.

#19 @pavelevap
12 years ago

Checkbox is damaged in IE9. Could you please test it somebody? See attached screenshot.

#20 @helenyhou
12 years ago

  • Keywords has-patch i18n-change commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

I guess IE must apply height to checkboxes. A little speechless.

#21 @helenyhou
12 years ago

  • Keywords has-patch added

22724.diff is about as consistent as I can get it. It sits a little low in Opera and a little lower yet in FF on OSX, but Chrome, IE, and FF on the PC all look great, which is to say the majority will see it looking pretty.

@helenyhou
12 years ago

#22 @koopersmith
12 years ago

  • Keywords commit added

My blessing, have it.

#23 @nacin
12 years ago

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

In 23049:

Media views: Better checkbox positioning. props helenyhou. fixes #22724.

Note: See TracTickets for help on using tickets.