Replace SubFormElement with FieldsetElement

[lippserd]

No description provided.

[lippserd]

@TAINCER Please rewrite the tests to verify that the array notation works both in terms of rendering and value processing. Also make sure that it is still possible to get elements with their original names.

[lippserd]

I don't expect any more changes than the ones just requested, so after that please squash ll your commits belonging to tests/FormElement/FieldsetTest.php into one "Test Fieldset" commit.

[yhabteab]

[lippserd]

I think it's because cloning just doesn't work. But that's not something that this PR introduces.

[lippserd]

I think it's because cloning just doesn't work. But that's not something that this PR introduces.

Or did you spot something wrong here in the code @yhabteab?

[yhabteab]

Or did you spot something wrong here in the code @yhabteab?

I would say yes :) Instead of doing so, why couldn't the concrete form set the default decorator of the element?

ipl-html/src/FormElement/FieldsetElement.php

Lines 60 to 71 in 91a5932

	public function prependWrapper(Wrappable $wrapper)
	{
	// TODO(lippserd): Revise decorator implementation to properly implement decorator propagation
	if (
	! $this->hasDefaultElementDecorator()
	&& $wrapper instanceof FormElementDecorator
	) {
	$this->setDefaultElementDecorator($wrapper);
	}
	return parent::prependWrapper($wrapper);
	}

So, I've just removed the prependWrapper() and added this to the job config form and it works.

$crontab>setDefaultElementDecorator(new IcingaFormDecorator());

[lippserd]

Or did you spot something wrong here in the code @yhabteab?

I would say yes :) Instead of doing so, why couldn't the concrete form set the default decorator of the element?

I wouldn't expect a default decorator to have to be set multiple times.

Can you please try this fix please.

[yhabteab]

I wouldn't expect a default decorator to have to be set multiple times.

Its not about adding the decorator multiple times or not, but with my version we wouldn't have to clone things as we are instantiating our own decorator instance.

Can you please try this fix please.

[lippserd]

Its not about adding the decorator multiple times or not, but with my version we wouldn't have to clone things as we are instantiating our own decorator instance.

The whole point is that I don't want library users to have to set decorators for fieldsets if the form already provides a decorator.

Can you please try the latest commit with the cloning patch applied?

[lippserd]

@TAINCER Please rebase.

[lippserd]

Wait before merging please. I think some commits can be squashed.

[lippserd]

Wait before merging please. I think some commits can be squashed.

Ready.

outdated