zendframework / zend-form

Form component from Zend Framework
BSD 3-Clause "New" or "Revised" License
69 stars 87 forks source link

bind() does not go into the fieldset #196

Closed modir closed 6 years ago

modir commented 6 years ago

I have a simple form with several text input fields. I would like to present them in the view in two different columns and hence I have two fieldsets.

Code to reproduce the issue

class CarForm extends Form
{
    public function __construct($name = null)
    {
        parent::__construct('car');

        $brandSection = new Fieldset('brand-section');

        $brandSection->add([
            'name' => 'brand',
            'type' => 'text',
            'attributes' => [
                'id'  => 'brand',
                'placeholder' => 'Brand',
            ],
            'options' => [
                'label' => 'Brand',
            ],
        ]);

        $this->add($brandSection);

        $ownerSection = new Fieldset('owner-section');

        $ownerSection->add([
            'name' => 'owner',
            'type' => 'text',
            'attributes' => [
                'id'  => 'owner',
                'placeholder' => 'Owner',
            ],
            'options' => [
                'label' => 'Owner',
            ],
        ]);

        $this->add($ownerSection);

    }
}

The controller has then this code:

$carForm = new CarForm();
$car = $this->carTable->getCar($id);
$carForm->bind($car);

And in the view I have this code:

    $carForm->prepare();
    echo $this->form($carForm);

The $car is of this class:

class Car implements InputFilterAwareInterface
{
    public $id;
    public $brand;
    public $owner;

    public function exchangeArray(array $data)
    {
        $this->id     = !empty($data['id']) ? $data['id'] : null;
        $this->brand = !empty($data['brand']) ? $data['brand'] : null;
        $this->owner  = !empty($data['owner']) ? $data['owner'] : null;
    }

    public function getArrayCopy()
    {
        return [
            'id'     => $this->id,
            'brand' => $this->brand,
            'owner'  => $this->owner,
        ];
    }
// Here are some more methods exactly like in the Quick Start Guide ZF3
}

In general I can say I created simple app based on the Getting Started tutorial and all I want to change is to have the form elements in two rows and this should be done with fieldsets.

Expected results

I expected that the form fields are prefilled with the content from the car model. In the quick start guide it is written that "If a form contains a fieldset that itself contains another fieldset, the form will recursively extract the values." Because of this I assume that the bind() would recursively get into the form and prefill the elements.

In this situation I expect that the content of $car->brand is filled into the form element "brand".

Actual results

None of the form fields are prefilled. If I change

$brand = new Fieldset('brand-section');

to 
$brand = new Fieldset('brand-section', ['use_as_base_fieldset' => true]);

Then one of the two fieldsets are prefilled. But never both of them.

From my point of view it is either a bug in the documentation or in the code.

froschdesign commented 6 years ago

@modir Sorry, but nobody can reproduce your example, because we do not know your class in $car and your example includes only one fieldset.

modir commented 6 years ago

@froschdesign I extended now my example. In general the whole setup is very simple. You could follow the getting started guide for ZF3 and only separate the form fields into two fieldsets. I didn't do anything more till now. (In my case I used different names than in the getting started guide but this doesn't interfere with my issue.)

froschdesign commented 6 years ago

I see two problems here:

  1. brand-section and owner-section are only defined in your form. Nothing similar can be found in your Car class. There is no magic in zend-form or other classes, so in your case the mapping must fail.
  2. You want two rows in your output and this should be done with fieldsets. This is the wrong way, because the fieldset element has a meaning in HTML for grouping elements. But in your form there is no separate group or elements which presents a set of form controls. To generate a layout with rows you can use a simple div element.

Example:

<div>
    <?= $this->formRow($form->get('brand')) ?>
</div>
<div>
    <?= $this->formRow($form->get('owner')) ?>
</div>

No fieldset needed, no misappropriation of HTML elements and no problems with mapping your class to form and vice versa.

modir commented 6 years ago

@froschdesign But doesn't the sentence "If a form contains a fieldset that itself contains another fieldset, the form will recursively extract the values." from the documentation mean that my code should work?

And regarding the two points you are mentioning:

  1. Can you point me to somewhere in the documentation where it is explained how I need to define it in the Car class?
  2. In my context the two fieldsets which do group elements which belong together from a user perspective. In the database they are in one table. Hence fieldsets are the right choice in my case. (I only gave here in my example a simplified code.)

It looks to me like all other element classes in zend_form represent an HTML element. But fieldset does more.

froschdesign commented 6 years ago

But doesn't the sentence "If a form contains a fieldset that itself contains another fieldset, the form will recursively extract the values." from the documentation mean that my code should work?

Extracting works, because you don't need an external object or class. But your problem is not extracting, it is the "mapping" between form and your class.

The handling is quite easy, because you can describe your form as an array:

[
    'brand' => [ // Fieldset
        'name' => '…', // Text element
        'logo' => '…', // File element
    ],
    'owner' => [ // Fieldset
        'first-name' => '…', // Text element
        'last-name'  => '…', // Text element
    ],
]

The same array can be used / is used to set data to the form and get or populate values.

The bind method does not do anything else: extract data from your object and populate values to the form. So you must provide the same array structure.

froschdesign commented 6 years ago

In the documentation you will find a complete example with fieldsets under the topic "Collections": https://docs.zendframework.com/zend-form/collections/ (A fieldset for a brand is also included.)

modir commented 6 years ago

Thanks. But here is exactly the problem. I read this part of the documentation and I see at the beginning of the page the entity class but there are no arrays. The example on this page has two entity classes for two fieldsets. In my case it is one entity class for two fieldsets. Hence the example doesn't work.

froschdesign commented 6 years ago

See my comment above with: "The handling is quite easy, because you can describe your form as an array"

modir commented 6 years ago

Yes, I have seen it and I am using it. My point is that it is not described in the documentation.

froschdesign commented 6 years ago

You must adapt the example for your class and follow my explanation:

$form->bind($car);

Do the same thing while extracting like this:

$data = $car->getArrayCopy();
$form->populateValues($data);

And if you have fieldsets like brand and owner, then you must provide these also in your array:

[
    'brand' => [
        'name' => '…',
        'logo' => '…',
    ],
    'owner' => [
        'first-name' => '…',
        'last-name'  => '…',
    ],
]

Then the name and logo is set for the brand(-fieldset) and the full name is set for the owner(-fieldset).

The same like in HTML:

<fieldset>
    <input name="brand[name]">
    <input name="brand[logo]">
</fieldset>
<fieldset>
    <input name="owner[first-name]">
    <input name="owner[last-name]">
</fieldset>

That's it what I mean with "you can describe your form as an array".