zendframework / zend-form

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

Form/Fieldset messy and not interchangeable #192

Open scroach opened 6 years ago

scroach commented 6 years ago

Since I did not get any response in the slack channel and couldn't find anything related:

Currently Zend Form and Fieldset seem like a huge mess to me.

In most of our projects we extendes the Form class to make our form generation easier and faster. And since Form extends Fieldset I thought they were kinda exchangeable in terms of nested forms/fieldsets - but as it seems they are not.

Now I tried to compose multiple Forms into a bigger one with collections trying to carve the code out of this doc: https://framework.zend.com/manual/2.4/en/modules/zend.form.collections.html

This didn't work (due to several reasons?). The collection has never been hydrated. After lots and lots of frustration I found out that using Forms inside other Forms (instead of Fieldset inside Form) seemed to be the problem. This is kinda weird for me, since Form extends Fieldset I automatically assumed I could nest Forms in Forms too - but instead of giving me an error it just does not work!

One of the main problems seems to be the difference in bindValues in both. 1) Why does Form NOT return $this->object; ? Fieldset does 2) Why does Form not foreach over all it's children like Fieldset does?

froschdesign commented 6 years ago

@scroach

…the code out of this doc: https://framework.zend.com/manual/2.4/en/modules/zend.form.collections.html

Please look at the top of the page, this documentation is outdated! The current documentation can be found here: https://docs.zendframework.com/zend-form/collections/

…using Forms inside other Forms (instead of Fieldset inside Form)…

Question to you: Why are you doing this?

scroach commented 6 years ago

Please look at the top of the page, this documentation is outdated! The current documentation can be found here: https://docs.zendframework.com/zend-form/collections/

I have already looked at the new docs too but there isn't so much that has changed.

Question to you: Why are you doing this?

Cause we extended the Form class for faster development and code reduction (i.e. calling a single line for adding a commmon element instead of passing a huge array or creating objects).

And I don't understand why Form vs Fieldset hydration/binding differ so much from each other. One of the main bind errors I just solved by adding 1 line to the end of bindValues:

return $this->object;

The docs aren't quite obvious about this topic either, I couldn't find any page that said: "do not nest forms inside forms!" or "here are the distinct differences between form and fieldset"

froschdesign commented 6 years ago

@scroach

Cause we extended the Form class for faster development… … And I don't understand why Form vs Fieldset hydration/binding differ so much from each other.

Please don't mix the problems here! (Your first statement has nothing to do with the second.)

Using forms inside other forms is not a documented way and also not recommended.


calling a single line for adding a commmon element instead of passing a huge array or creating objects

Setting a new factory to create the elements was not an option?

scroach commented 6 years ago

Please don't mix the problems here! (Your first statement has nothing to do with the second.)

The first is just the explanation of why we do it this way, the 2nd causes the problems with nesting (which we never needed until now)

Using forms inside other forms is not a documented way and also not recommended.

I found that out the hard way, but in my opinion this should be handled differently cause debugging why the values have not been bound took me a few hours.

For me there are currently 2 options: a) Don't allow nesting Forms in Forms by either changing the class hierarchy or explicitely throwing errors. b) "Fix" the probems and/or discrepancies between Form/Fieldset in order to make nesting work (I can add a Form to a Form programatically so I expect it to work)

Setting a new factory to create the elements was not an option?

I haven't looked into that, maybe our structure isn't the best either but 1) we've got lots and lots of code which works perfectly and 2) thats a whole other topic anyway.

froschdesign commented 6 years ago

I haven't looked into that…

Replace the factory with your own and you can use this: (pseudo code) $form->add(['input[type=date][name=foo][value=2018-01-22]']) (value must be an array)

scroach commented 6 years ago

Alright, thanks for your input so far.

If you say nesting forms isn't supported and never will be I'm gonna have to to fix this for our project or switch to using fieldsets instead.

froschdesign commented 6 years ago

If you say nesting forms isn't supported and never will be…

HTML doesn't allow nested forms, why should zend-form do something different?

The form element

Content model:

Flow content, but with no form element descendants.

https://www.w3.org/TR/html52/sec-forms.html#the-form-element

And zend-form already includes many options to build a form and allowing nested forms means further complications; for code and documentation.

froschdesign commented 6 years ago

What we can do here: throw an exception if a form is add to a form.

scroach commented 6 years ago

If you consider the HTML side - yeah you're right. But I just considered the Zend implementation as an abstract container holding all the elements, and allowing nesting and hierarchy (like the FieldsetInterface suggests and Form implements those methods).

froschdesign commented 6 years ago

I never said that I like this implementation. 😉

For me the main reason is the explanation for the end user. This component is quite heavy and has many many options. Therefore nested forms are the wrong direction. Also the comparison to HTML forms is for users quite simple.

weierophinney commented 4 years ago

This repository has been closed and moved to laminas/laminas-form; a new issue has been opened at https://github.com/laminas/laminas-form/issues/6.