php warnings thrown by ClassyLayout when definition_default is NULL for default_section

@T.Barker the patch as per #2 seems to be missing tests.

Uploading a new version of the patch which contains the tests, as per feedback from @amjad1233 .

Setting to needs review so the tests run

+++ b/src/ClassyLayout.php
@@ -66,7 +66,7 @@ class ClassyLayout extends LayoutDefault implements PluginFormInterface {
-      $definition_default = $class_definition['default'] ?? NULL;
+      $definition_default = $class_definition['default'] ?? [];

I think we need to $class_definition['multiple'] here, as an array is appropriate if that's TRUE, but NULL is appropriate if not.

I also note you can probably put `default: {}` in your definition and it should resolve the error

Hi @larowlan,

Thanks for your feedback.

Would you mind elaborating?

I think we need to $class_definition['multiple'] here, as an array is appropriate if that's TRUE, but NULL is appropriate if not.

In regards to `default: {}` I thought it was mandatory but makes sense.

Regards,
Amjad

So the default value should be NULL if the class doesn't support multiple values.
If it does, then an empty array makes sense.

And in your definition of the class in the yaml, you can add `default: {}` and it should fix this error too (but I agree fixing it here is the best approach)

Ah, that makes sense. Cool no worries we will give it another look. Thanks for that.

I agree too fixing seems to be a good approach since Test Traits 2 are bit more sensetive.

Thanks @larowlan. @amjad1233 hasn't been able to work on this with me but I've attached a new patch file with changes from your feedback, as I've interpreted it. Is this along the lines of what you were meaning? I'm sure there is probably a better way to express the logic but I'm still learning - sorry!

T.Barker → closed merge request !2

+++ b/src/ClassyLayout.php
@@ -66,7 +66,11 @@ class ClassyLayout extends LayoutDefault implements PluginFormInterface {
+        $definition_default = NULL;

We still need to honour the default in non multiple scenarios, so we basically need the original code in the else clause

I found another issue and fixed out.

Thanks
Tagging 8.x-1.4

This has caused a regression/behaviour change where required classes now have an empty option and default to NULL instead of the first option in the list of classes.

1.3:

1.4:

Ah, we should be able to fix that and get a new release out

Added followup 🐛 Default value not set for required options Active

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