Problem/Motivation

The dropdown select for new effects in ImageStyleEditForm sort effects by id. This results in weird sequence if you have contrib effects installed.

Proposed resolution

Sort the effects in the dropdown by label instead.

Before patch:

After patch:

Remaining tasks

review patch

User interface changes

More readable list of effects in the dropdown.

API changes

None

Data model changes

None

CommentFileSizeAuthor
#17 interdiff_13-17.txt744 bytesmondrake
#17 2654150-17.patch1.87 KBmondrake
#13 2654150-13.patch1.9 KBcilefen
#13 2654150-13-test.patch1005 bytescilefen
#13 interdiff-2654150-13.txt873 bytescilefen
#10 2654150-10.patch930 bytespritish.kumar
#3 after.png10.41 KBmondrake
#3 before.png9.94 KBmondrake
#2 2654150-2.patch935 bytesmondrake
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mondrake created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Status: Active » Needs review
FileSize
2654150-2.patch935 bytes
mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Issue summary: View changes
FileSize
before.png9.94 KB
after.png10.41 KB
mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Issue summary: View changes

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

bander2’s picture

bander2 CreditAttribution: bander2 as a volunteer commented
Status: Needs review » Needs work

Patch not applying to 8.3.x-dev.

bander2’s picture

bander2 CreditAttribution: bander2 as a volunteer commented
Issue tags: +Needs reroll
pritish.kumar’s picture

pritish.kumar CreditAttribution: pritish.kumar at OpenSense Labs commented
Issue tags: -Needs reroll
FileSize
2654150-10.patch930 bytes

Providing the Reroll

pritish.kumar’s picture

pritish.kumar CreditAttribution: pritish.kumar at OpenSense Labs commented
Status: Needs work » Needs review
bander2’s picture

bander2 CreditAttribution: bander2 as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Looks good.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Status: Reviewed & tested by the community » Needs review
FileSize
interdiff-2654150-13.txt873 bytes
2654150-13-test.patch1005 bytes
2654150-13.patch1.9 KB

We require tests for bugs in most cases. Here is one.

The last submitted patch, 13: 2654150-13-test.patch, failed testing.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Let's get this in then?

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue tags: +Usability

lgtm

+++ b/core/modules/image/src/Form/ImageStyleEditForm.php
@@ -143,7 +144,7 @@ public function form(array $form, FormStateInterface $form_state) {
+      return Unicode::strcasecmp((string) $a['label'], (string) $b['label']);

Although I wonder why the string casting is necessary here.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Status: Reviewed & tested by the community » Needs review
FileSize
2654150-17.patch1.87 KB
interdiff_13-17.txt744 bytes

@Wim Leers re

I wonder why the string casting is necessary here

I think I remember there was a problem passing TranslatableMarkup objects instead of strings to the strcasecmp method when I first developed the patch in #2 in Jan 2016, but I cannot reproduce it now. Removing the explicit casting.

katzilla’s picture

katzilla
she/her
Primary language German
Location Göttingen, Germany
CreditAttribution: katzilla at Reinblau commented
Version: 8.3.x-dev » 8.4.x-dev
Status: Needs review » Reviewed & tested by the community
Issue tags: +drupalsprintberlin

Tested at #drupalsprintberlin against 8.4.x and it works.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

Looks perfect now!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 17: 2654150-17.patch, failed testing. View results

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Status: Needs work » Reviewed & tested by the community

Cannot understand #20

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.4.x, thanks!

  • catch committed 3ecc833 on 8.4.x 
    Issue #2654150 by mondrake, cilefen, pritish.kumar: Image effects in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

rajeevku’s picture

rajeevku CreditAttribution: rajeevku commented

It isn't displaying image styles provided by contrib module , i am using AMP and not getting image style 'AMP Image'.??