fix mistakes, suggest improvements in collections.md #133
Conversation
specifically, I have:
- fixed references to Zend\Stdlib\Hydrator\ClassMethods
- fixed a typo allws -> allows
- suggested a line in the controller to set the form's action attribute to submit to itself rather setting it to the 'home' route in the viewscript
- in the controller, wrapped the var_dump() in <pre> tags for legibility
- fixed a syntax error in the ProductFieldset getInputerFilterSpecification() (an array was opened with the old syntax but closed with the new)
- removed a line from the viewscript that gratuitously re-renders $this->formRow($product->get('name'));
As a side note, I was going to propose a revision to the part about not being able to submit the form with a Collection having fewer elements than it began with. I am not sure that's the case. The 'allow_remove' option seems to be just for that. But I haven't been able to prove it because at the moment I am struggling with my Doctrine and can't get it to remove entities from the N side of my 1:N.
Hope this helps :-)
doc/book/collections.md
Outdated
| $form->bind($product); | ||
|
|
||
| $request = $this->getRequest(); | ||
| $form->setAttribute('action',$request->getRequestUri()); |
There was a problem hiding this comment.
Please remove this. The url-helper was good in the view script. The helper is used in some more examples.
There was a problem hiding this comment.
consistency is not always a virtue. nothing wrong with url helper per se, of course, but a route called 'home' typically maps to something like a front page, while this controller action looks like it would typically be mapped to something like '/products/add'. my purpose was simply to make the code example behave like the illustration.
There was a problem hiding this comment.
Then change the name of the route to "products/add" in all examples.
Thanks!
There was a problem hiding this comment.
eh.... imho the status quo ante is preferable to that -- i.e., leave it as you had it.
doc/book/collections.md
Outdated
|
|
||
| if ($form->isValid()) { | ||
| var_dump($product); | ||
| echo '<pre>'; |
There was a problem hiding this comment.
Not needed. It's a simple example and everyone knows the output or has Xdebug installed in a development environment.
There was a problem hiding this comment.
fair enough. once again, the objective was to make the code act like the illustration without presupposing Xdebug or anything else.
There was a problem hiding this comment.
The documentation is not aimed at beginners and does not cover general PHP methods like var_dump. Therefore, the original code example is enough.
There was a problem hiding this comment.
indeed -- not aimed at beginners. I stand by my point but it's not a big deal, is it. I've applied all your suggestions, as noted below.
There was a problem hiding this comment.
To clarify: I mean PHP programming beginners! 😃
|
|
||
| ```php | ||
| <?php | ||
| $form->setAttribute('action', $this->url('home')); |
There was a problem hiding this comment.
There is no reason to delete this. (See my comment above)
There was a problem hiding this comment.
right. so, while respectfully disagreeing, I have updated my branch (fork, whatever) per your suggestions. I'm a github noob and not sure what if anything I should do next. so please let me know, if you could. thanks!
doc/book/collections.md
Outdated
| var_dump($product); | ||
| echo '<pre>'; | ||
| var_dump($product); | ||
| echo '</pre>'; |
fix mistakes, suggest improvements in collections.md
3 participants
specifically, I have:
<pre>tags for legibilityAs a side note, I was going to propose a revision to the part about not being able to submit the form with a Collection having fewer elements than it began with. I am not sure that's the case. The 'allow_remove' option seems to be just for that. But I haven't been able to prove it because at the moment I am struggling with my Doctrine and can't get it to remove entities from the N side of my 1:N.
Hope this helps :-)