Problem/Motivation

The list of enabled styles seems to be pretty random and also changes, that results in unreadable diffs when risks of undoing/reverting changes as well as merge conflicts.

Proposed resolution

Sort the list of paragraph types alphabetically before saving.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 3034897-sorted-styles-7-5-interdiff.txt985 bytesmbovan
#7 3034897-sorted-styles-7.patch1.59 KBmbovan
#5 3034897-sorted-styles-5.patch646 bytesmbovan

Comments

Berdir created an issue. See original summary.

mbovan’s picture

mbovan commented
Issue summary: View changes
Issue tags: +drupalmountaincamp

Updated the issue summary and tagged it for Drupal Mountain Camp.

mbovan’s picture

mbovan commented
Issue summary: View changes

Updated the proposed resolution.

mbovan’s picture

mbovan commented
Assigned: Unassigned » mbovan

Assigned to myself for Drupal Mountain Camp demo.

mbovan’s picture

mbovan commented
Assigned: mbovan » Unassigned
Status: Active » Needs review
StatusFileSize
new 3034897-sorted-styles-5.patch646 bytes

Sorted the list of styles.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work
Issue tags: +Needs tests 
+++ b/src/Form/StylesOverviewForm.php
@@ -193,6 +193,7 @@ class StylesOverviewForm extends ConfigFormBase {
     parent::submitForm($form, $form_state);
     $enabled_styles = array_keys(array_filter((array) $form_state->getValue('styles'), function ($value) { return !empty($value['enabled']); }));
+    sort($enabled_styles);
     $this->config->set('enabled_styles', $enabled_styles);
     $this->config->save();

Looks good, now we just need a test.

mbovan’s picture

mbovan commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 3034897-sorted-styles-7.patch1.59 KB
new 3034897-sorted-styles-7-5-interdiff.txt985 bytes

Adding a test as well.

I couldn't easily provide a failing test-only patch...

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Not sure if the test is really testing this, I think we'd likely need to manually change the order and then checking that resaving the form updates it.

The fancy option would be a config save event subscriber that reorders there, then it would always happen, but seems a bit overkill. This is good enough for me.

  • Berdir committed 279ae6c on 8.x-1.x authored by mbovan 
    Issue #3034897 by mbovan: Sort list of enabled styles in...

Status: Fixed » Closed (fixed)

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