WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#22724 closed defect (bug) (fixed)

Context for Random and checkbox position

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

Download all attachments as: .zip

Change History (30)

@pavelevap3 years ago

comment:1 @SergeyBiryukov3 years ago

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

comment:2 @nacin3 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.

comment:3 @pavelevap3 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.

comment:4 @vanillalounge3 years ago

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

comment:5 @nacin3 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.

comment:6 @nacin3 years ago

  • Keywords 2nd-opinion removed

As requested. Thanks guys.

comment:8 @pavelevap3 years ago

Nice, thank you, nacin!

comment:9 @pavelevap3 years ago

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

Or should I reopen this ticket?

@SergeyBiryukov3 years ago

comment:10 @SergeyBiryukov3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:11 @nacin3 years ago

  • Component changed from I18N to Media

comment:12 @nacin3 years ago

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

@SergeyBiryukov3 years ago

comment:13 @nacin3 years ago

Please see attached screenshot.

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

comment:14 @nacin3 years ago

Okay, Firefox.

@koopersmith3 years ago

comment:15 @koopersmith3 years ago

  • Keywords commit added

Latest fixes vertical alignment a bit. Otherwise, solid.

comment:16 @ocean903 years ago

22724.3.diff​ looks good in LTR and RTL.

comment:17 @nacin3 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.

comment:18 @nacin3 years ago

[23034] also fixed #22737, accidentally.

comment:19 @pavelevap3 years ago

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

comment:20 @helenyhou3 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.

comment:21 @helenyhou3 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.

@helenyhou3 years ago

comment:22 @koopersmith3 years ago

  • Keywords commit added

My blessing, have it.

comment:23 @nacin3 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.