Responsive image field formatter list should be sorted by label, not machine name

Thanks for the issue and patch.

The only thing in the patch that is for the issue summary is below. The rest of the changes are formatting and other unrelated fixes. Either the issue summary needs to be updated to reflect all the changes in the patch or the patch needs to be updated to only have the fix for the issue in the issue summary.

+++ b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php
@@ -136,8 +139,11 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
+    // Sort styles by their labels in 'natural order'.
+    asort($responsive_image_options, SORT_NATURAL);

Patch #3 was not applying on Drupal 9.1.x, so I have rerolled the patch #3 on Drupal 9.1.x and I have removed unrelated fixes.

Thanks for the update. The changes address #6.

Not sure this warrants tests but still needs manual testing.

Moving back to needs work as the sorting for the drop down doesn't match the sorting for Responsive Image Styles list.

Responsive image styles

Listing page

Dropdown without patch

Dropdown with patch

Thanks for the update. Marking RTBC based on:

1) Tested and now the order is the same as the Responsive Image Styles listing as expected. See screenshots.

2) Patch applies cleanly and tests pass.

3) Title & issue summary seem clear.

4) natcasesort is used elsewhere in core code.

5) Not sure tests need to be added for this as the only natcasesort checking I see is in one test (LanguageLocaleListTest).

+++ b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php
@@ -135,6 +135,9 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
+    // Sort styles by their labels in 'natural order'.
+    asort($responsive_image_options, SORT_NATURAL);

Config entities come with built-in sorting methods.

Above this we do ]

    $responsive_image_styles = $this->responsiveImageStyleStorage->loadMultiple();

after this we should do

uasort($responsive_image_styles, '\Drupal\responsive_image\Entity\ResponsiveImageStyle::sort');

and then if we add weights or some other custom sorting stuff to image styles this with get the same functionality without us having to remember about it.

As mentioned by #14 added a patch for the same.

After apply #15, Drupal show me the pre set before and only so show the style that I created sorted correctly, so if I doed this correctly, I think this need more work.

See:

Re-rolled the #15 for 9.2.x and also fixed the issue mentioned on #16

After #19 works to me:

+++ b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php
@@ -135,6 +136,8 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
+    // Sort styles by their labels.
+    natcasesort($responsive_image_options);

This can be removed. The initial sort works as expected. (I think).

Worked on comment # 22

I tested this on Drupal 9.3.x and when the patch is applied the sorting at /admin/config/media/responsive-image-style and for the field formatter at admin/structure/types/manage/article/display are the same.

For all the reasons in #14, back to RTBC

Discussed this with @catch w.r.t. whether we'd require a test - as per our typical policy on tests.

He agreed we don't in this specific instance, because 'it's barely a bug' 😅

Committed baa711c and pushed to 9.3.x. Thanks!

Backported to 9.2

Thanks for committing the "barely-bug". :) Should it be marked "minor" based on this?