Allow to define presets for typical configurations (fences_presets submodule)

This feature would still be sooo so useful!

Definitely yes!! ;)

Perhaps Leon will be able to do this in some months ;) Perhaps you need to sponsor him :P

Ok, I entirely rewrote the issue summary for @LRWebks to have a better understanding of the whole thing. I also moved "reference" part in a separate follow-up issue, here: 📌 Add option to ovewrite fences presets in the formatter settings (preset by reference) Active .

Well, let's finally tackle this issue then! I'll start with this now.

LRWebks → closed merge request !31

LRWebks → reopened merge request !31

Quick question: In the issue description, you state:

Add a "default_preset" config entity through adding a config entity yml in config/install. Which holds the "Default Preset". This preset should be selected on default, when being on the formatter settings page and having this module installed.

Why would we want to do that, though? The default values apply either way.
If it is just in order to have a preset that appears when we open the config so the preset select isn't empty, wouldn't it be better if we had a "No preset" option that is selected by default and does not change anything?
This would also help with the follow-up issue so that when config entities are changed within the display settings form, we had the option "No preset" so that no preset would be changed if the values are changed within the display settings form. (If you make custom changes and don't want any preset to be changed due to those, you select "No preset" and do your changes then)

Yes indeed, that would make much more sense.

Thinking about this, I am unsure, what my intentions behind a default preset were.

Just had a quick look at the code, looks good right now!

I've been thinking a bit about this part of the plan:

Create a new select render element inside "fences_field_formatter_third_party_settings_form()", called "fences_presets", where the preset config entities can be selected from, note that this form element should be purely cosmetic and not save any real value in the config, it's just used to be picked up by the JavaScript.

The question is: Should it be purely cosmetic, though?
I believe that I would be more handy if the selected preset is saved in the config, so that it later appears as the selected option and the person editing the form knows that the values match this preset.
Example: You create about 50 presets (a lot) and then use one of the presets to pre-set the values of the display settings form. Later you re-open the settings page, but since the select value isn't stored anywhere, the select will again say "No preset" and you don't know which one you picked... If the preset select value was saved, you'd know which preset is currently active OR you get to know that a certain preset was used in the first place, and you cannot simply tweak any value because that might ruin the layout.
Also, if you DO tweak settings that were originally filled in by a preset, the selected preset name would still be visible in the select, and you would know which one to pick if you want to revert your small tweaks again. That'd be really handy, in my opinion.

All reviewed. I am unsure who should finish this. @Anybody should decide, based on the reviews.

Ok I just generated an example config entity in drush and there the "status" works as expected!
The form value is saved correctly and the status value is correctly being exported to the config:

+++ b/tmp/drush_tmp_1713947716_6628c444d2a0e/example_config_entity.example_config_entity.test.yml
@@ -1,6 +1,6 @@
 uuid: e36536f1-6b42-4014-8e37-8e6020d492a9
 langcode: de
-status: true
+status: false
 dependencies: {  }
 id: test
 label: test

I'll start a code comparison.

Oay this doesn't make any sense to me. The form structure and Config Entity structure are exactly the same. For the generated code it works with status, for the implementation here it doesn't.

The generated code doesn't even have "status" in config_export annotation, but the value is saved!

Maybe a local issue? @Grevil could you give it a try in your environment?

@Grevil & @LRWebks: Here's my guess:

toArray() is used internally in Drupal if present and this overrides the regular class properties getting. toArray() has no status and so it's missing ... Let's see!

Indeed, @Anybody "toArray()" is already declared by its parent.

@LRWebks, usually your IDE will let you know about this when defining a new method:

So if that is the case, check the outcome of the already existing method, and decide whether to:

• Overwrite the method.
• Extend the method.
• Create a separate method, as the original implementation focuses on something entirely different.

In this case, the already existing implementation does exactly what we need, so we do not need to define our own method.

BUT this still doesn't fix the problem for only loading active presets. I'll have a quick dig and otherwise we need to do it via a tiny workaound.

Should be all done! Unfortunately, we get a schema error, when saving the entity inside our test. No idea why... I defined it just as the main module would...

Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for core.entity_view_display.node.article.default with the following errors: core.entity_view_display.node.article.default:content.body.third_party_settings.fences.fences_presets missing schema in Drupal\Core\Config\Development\ConfigSchemaChecker->onConfigSave() (line 98 of core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php).

@Anybody, any idea?
Using "field.formatter.third_party.fences_presets" instead of "field.formatter.third_party.fences.fences_presets" won't fix it either.

Found the problem and fixed it!

Now I only need to add two more default presets.

Alright, all done! Please do a final review @Anybody!

Nice, Thomas (at my pc) and me left some comments. We're close to the finish line, great job!

As stated in the MR, concerning the need of a schema definition:

Yea, this doesn't seem to be easily implemented, unfortunately. There seems to be no hook_field_formatter_third_party_settings_form_save() hook, and I don't want to overwrite the form['actions']['save'] call, as we are already using a custom hook to add our render elements, and we would then also overwrite the save of the main modules form implementation. Also, it wouldn't be wise to reimplement an already existing core save method (Hence we are not in a class context, we can not simply extend it).

BUT, I think having a schema definition for the select element isn't that bad. It feels a bit dirty to always save an empty value as a third party setting, but there could be worse. Let's create a follow-up issue to fix this inconvenience and move on for now.

All done! We are now using an inline template as a workaround, which doesn't get saved and is purely cosmetic. Please review!

Really, really awesome work!
Left some final comments, afterwards we should merge it and create a new tagged release containing the submodule.

Could you also add some lines about the feature on the project page?

Added a new section, quickly going over the new submodule!

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