Make WordPress Core

Opened 13 years ago

Closed 13 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 13 years ago.
Random_checkbox_position.jpg (5.3 KB) - added by pavelevap 13 years ago.
22724.patch (426 bytes) - added by SergeyBiryukov 13 years ago.
22724.2.patch (374 bytes) - added by SergeyBiryukov 13 years ago.
22724.3.diff (416 bytes) - added by koopersmith 13 years ago.
IE9_Gallery_checkbox.jpg (5.9 KB) - added by pavelevap 13 years ago.
22724.diff (380 bytes) - added by helenyhou 13 years ago.

Download all attachments as: .zip

Change History (30)

#1 @SergeyBiryukov
13 years ago

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

#2 @nacin
13 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
13 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
13 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
13 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
13 years ago

  • Keywords 2nd-opinion removed

As requested. Thanks guys.

#8 @pavelevap
13 years ago

Nice, thank you, nacin!

#9 @pavelevap
13 years ago

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

Or should I reopen this ticket?

#10 @SergeyBiryukov
13 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

#11 @nacin
13 years ago

  • Component changed from I18N to Media

#12 @nacin
13 years ago

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

#13 @nacin
13 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
13 years ago

Okay, Firefox.

@koopersmith
13 years ago

#15 @koopersmith
13 years ago

  • Keywords commit added

Latest fixes vertical alignment a bit. Otherwise, solid.

#16 @ocean90
13 years ago

22724.3.diff​ looks good in LTR and RTL.

#17 @nacin
13 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
13 years ago

[23034] also fixed #22737, accidentally.

#19 @pavelevap
13 years ago

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

#20 @helenyhou
13 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
13 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
13 years ago

#22 @koopersmith
13 years ago

  • Keywords commit added

My blessing, have it.

#23 @nacin
13 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.