search

AshishJoshi-asj / zfdatagrid

Automatically exported from code.google.com/p/zfdatagrid
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

addElement() does not work on Bvb_Grid_Form #196

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. $myBvbForm->addElement($element)

What is the expected output? What do you see instead?

Expected : that addElement of the form was called

Instead : Exception
object(Zend_Form_Exception)[234]
  protected 'message' => string 'Method addElement does not exist' (length=32)
  private 'string' => string '' (length=0)
  protected 'code' => int 0
  protected 'file' => string
'/home/m.thomas/www/stamp/trunk/vendors/Zend/Form.php' (length=52)
  protected 'line' => int 2857

Whereas addElement method() exists.

What version of the product are you using? On what operating system?
v0.6 alpha, under Fedora.

Please provide any additional information below.
Bvb_Grid_Form, line 59, performs a $this->form->__call($name, $args).
The 'addElement' method exists in Zend_Form, so __call does not be called
right away.
A call_user_func_array(array($this->form, $name), $args) could be solved
this (and if 'addElement' did not exist then Zend_Form would have call
itself its __call method)

Original issue reported on code.google.com by tribal...@gmail.com on 1 Mar 2010 at 4:50

GoogleCodeExporter commented 9 years ago 
Hi,

there was a change in forms recently. Try

$myBvbForm->form->addElement($element)

Original comment by vlatko.b...@gmail.com on 1 Mar 2010 at 5:07

GoogleCodeExporter commented 9 years ago 
In fact that will work, but I thought the implemented design pattern was 
'Proxy' one.
Could you explain the reason for disabling access to existing methods of the 
client
object ? (here Zend_Form by default) ?

Original comment by tribal...@gmail.com on 1 Mar 2010 at 5:16

GoogleCodeExporter commented 9 years ago 
The reason for that was to gain ability to create forms of Zend_From 
descendants.

Before, Bvb form was direct descendant of Zend_Form and it was not possible to 
make
it inherit from our custom form. Now, that is possible. :-)

However, there was a price to pay. I think the proxy design wasn't done as it 
would
require much more work, and the time to release 0.6 is approaching.

It might be strange usage at first, but maybe such approach could prove to be 
more
beneficial in the future. 

One thing that cross my mind at the moment is to do something like

$myBvbForm->form['Edit']->addElement($element)
$myBvbForm->form['Delete']->addElement($element)
$myBvbForm->form['VerySpecialCase']->addElement($element)

To implement something like this would now require little effort.

Original comment by vlatko.b...@gmail.com on 1 Mar 2010 at 5:38

GoogleCodeExporter commented 9 years ago 
Hi,

Try the latest revision 801

Should be fixed.

Thanks

Best Regards,
Bento Vilas Boas

Original comment by pao.fre...@gmail.com on 1 Mar 2010 at 6:35

GoogleCodeExporter commented 9 years ago 
Upsss.

A lot of talk while this window was open. 

My comment is about the report and not any comment.

Will look into comments and say something...

Best Regards,
Bento Vilas Boas

Original comment by pao.fre...@gmail.com on 1 Mar 2010 at 6:37

GoogleCodeExporter commented 9 years ago 
perfect, that works, thanks :)

Original comment by tribal...@gmail.com on 2 Mar 2010 at 8:54

GoogleCodeExporter commented 9 years ago

Original comment by pao.fre...@gmail.com on 2 Mar 2010 at 11:03

GoogleCodeExporter commented 9 years ago 
pao.fresco, vlatko.b,

I have read that inheritance was a problem at a moment (even if I do not 
understand
why at first sight).

However, I think it was a good idea to externalize the form object, allowing
developpers to use it independently of the component if they want.

But I do not understand why Bvb_Grid_Form manages decorators.
I have looked furtively the Zend_Db_Select class code and I have seen that each
Zend_Form_Element implement the elementDecorator of the Bvb_Grid_Form...

I think there is a easier (and most logical) solution : Bvb_Grid_Form could use 
by
defaut not a "Zend_Form" class, but a specific one implementing Zend_Form 
(offered by
Bvb component as an example of implementation, like My_Grid_Form)

For me the component does not imposed the use of decorators in one of this 
class, as
it does for templates (My_Template_*).
Ok, it is possible to extend Bvb_Grid_Form to redefined $*Decorators 
properties, but
the way I think would differenciate the action form management (for me 
Bvb_Grid_Form)
and the form properties (Zend_Form implementation, managing itself its 
decorators).

To resume : i think this component must not impose that user-defined decorators 
are
in a class different of the (real) form one.

An example to support theses arguments :

class Bvb_Grid_Form
{
    // all current properties and methods are kept,
    // <!> exept $*Decorators ones <!>

    // constructor use default implementation of Zend_Form
    function __construct ($formClass = 'My_Grid_Form', $formOptions = array())
}

class My_Grid_Form
{
    /* DECORATORS CURRENTLY IN BVB_GRID_FORM */
    public $groupDecorators = array('FormElements', array('HtmlTag', array('tag' =>
'td', 'colspan' => '2', 'class' => 'buttons')), 'DtDdWrapper');

    public $elementDecorators = array('ViewHelper', 'Description', 'Errors',
array(array('data' => 'HtmlTag'), array('tag' => 'td', 'class' => 'element')),
array(array('label' => 'Label'), array('tag' => 'td')), array(array('row' =>
'HtmlTag'), array('tag' => 'tr')));

    public $buttonHidden = array('ViewHelper');

    public $formDecorator = array('FormElements', array('HtmlTag', array('tag' =>
'table', 'style' => 'width:98%', 'class' => 'borders')), 'Form');
    /* /DECORATORS CURRENTLY IN BVB_GRID_FORM */

    public function init()
    {
          // all of theses decorators will be apply to future elements
          $this->setElementDecorators($this->elementDecorators)

          // add these own decorators (or by a loadDefaultDecorators() overriding)
          $this->setDecorators($this->formDecorators)
    }
}

With this, all elements will benefit from predefined decorators (defined in 
init() of
the form) without having to use a addDecorators() on each elements (it will be 
automatic), and the form manage itself its own decorators, overridable by user 
in its
Zend_Form implementation.

While a search in code I just found some code in Bvb_Grid_Deploy_Table. In line 
1625,
there is a setDecorators() which not use Bvb_Grid_Form ones, is it normal ?

All of this will allow user to define properly its own decorators without 
extending
Bvb_Grid_Form whose purpose is not really to manage these things I think.

(hope to be clear...)

Original comment by tribal...@gmail.com on 2 Mar 2010 at 4:02

GoogleCodeExporter commented 9 years ago 
That could be an interface that forms must implemented.

class Bvb_Grid_Form
{
    // all current properties and methods are kept,
    // <!> exept $*Decorators ones <!>

    // constructor use default implementation of Zend_Form
    function __construct ($formClass = 'My_Grid_Form', $formOptions = array())
}

interface Bvb_Grid_Form_Implementation_Interface
{
    public function getGroupDecorators();
    public function getElementDecorators();
    public function getHiddenButtonDecorators();
    public function getFormDecorator();
}

class My_Grid_Form implements Bvb_Grid_Form_Interface
{
    // define the 4 interface methods, usable by the component when it build the form

    // user stuff : method init(), methods callable through datagrid controller so
that overriding elements added by the grid, ...
}

Other thing : regarding to issue 198, Bvb_Grid_Form property "form" should be
protected. In this way, this class should implements a getForm() method.

Also, $bvbGridForm->form->* would be depreciated :

For calling form object methods, the fix of comment 4 give the solution : doing 
a
$bvbGridForm->method() will call the method of the form object if exists.

Logically, form object properties has already getter/setter, so no need of 
magical
__get() and __set() in Bvb_Grid_Form to access to form object properties.

Perhaps I do not imagine bad consequences, but I do not see them for the 
moment. If
you do, do not hesitate to tell me about.

Original comment by tribal...@gmail.com on 3 Mar 2010 at 8:47

GoogleCodeExporter commented 9 years ago 
I do not quite understand what is the problem here. OK, it would be maybe 
easier if
Bvb_Grid_Form proxies to $form, but this way we have inheritance. 

If custom form (My_Form) is derived from Zend_Form and all of our forms (not 
just
zfDatagrid forms) would benefit from it, than we give Bvb_Grid_Form that form 
as the
base form.

If, on the other hand, Bvb_Grid_Form has some functionality we do not like, 
than we
derive our form from Bvb_Grid_Form, and change not desired behaviour. And all 
is solved.

Or, am I missing something here?

Original comment by vlatko.b...@gmail.com on 3 Mar 2010 at 11:05

GoogleCodeExporter commented 9 years ago 
Hi vlatko.b,

I am not sure to be able to explain this differently at this moment. Maybe 
later,
just some words :

* With the fix of comment 4 we have inheritance on methods (but in fact not on
getter/setter).

* At least, Bvb_Grid_Form should implements getter for each decorators,
get<Specific>Decorators().

In my case the getter will retrieve decorators from my form object and not from
Bvb_Grid_Form. If I have a Zend_Form subclass common to all forms of my 
application
and it have its own decorators as properties I do not want to rewrite them in
Bvb_Grid_Form (why doing two times the same think whereas the possibility to 
let them
as an unique place is possible ?)

It would be extremely practical (that would imply to change all
$bvbForm-><specific>Decorators by $bvbForm->get<Specific>Decorators()

* Bvb_Grid_Deploy_Table (line 1614) : decorators are written in the class, they
should be retrieved from Bvb_Grid_Form.

Original comment by tribal...@gmail.com on 3 Mar 2010 at 11:38

GoogleCodeExporter commented 9 years ago 
So, if I get it right, you mean that in Bvb_Grid_Form there should be 

protected $_groupDecorators;
protected $_elementDecorators;
protected $_buttonHidden;
protected $_formDecorators;  // I added plural here

public (get|set)groupDecorators;
public (get|set)elementDecorators;
public (get|set)buttonHidden;
public (get|set)formDecorators;

And on line 1614 (also on line 1624, 1628, 1630, 1631, 1635, 1636, 1640 - please
check, it'll simplify to Pao):

instead of $crud->form->setDecorators(array('FormElements', .....);
should be  $crud->form->setDecorators($crud->getFormDecorators());

That would solve the inheritance problems, correct?

Original comment by vlatko.b...@gmail.com on 3 Mar 2010 at 12:52

GoogleCodeExporter commented 9 years ago 
I'm ok with all.

I add just some things :

- $_buttonHidden could become $_hiddenButtonDecorator (more explicit and 
homogeneous)
- we can find calls to decorator in Bvb_Grid_Source_Zend_Select too, in
buildFormElements(). They should be replaced too.

I have not understood the inheritance problem yet. We talk about but I think 
nobody
gives me the keys to understand it (and perhaps found a solution... or not !)

Using getters here just allows us to return decorators from... where we want. We
could keep on doing a return $this->_elementDecorators, but if we want (like 
me) on
could do a return $this->form->getDecoratorsIwant() or something else...

Original comment by tribal...@gmail.com on 3 Mar 2010 at 1:18

GoogleCodeExporter commented 9 years ago

Original comment by bento.vi...@gmail.com on 3 Mar 2010 at 4:56

GoogleCodeExporter commented 9 years ago 
Hi,

@tribal: Do you mean you do not understand the need for inheritance in here, 
i.e. why
Bvb form should inherit not directly from Zend_Form, but from a custom form?
Or something else is in question?

Original comment by vlatko.b...@gmail.com on 4 Mar 2010 at 9:17

GoogleCodeExporter commented 9 years ago 
Hi vlatko.b,

Yes in fact I do not understand what are the reasons for which Bvb_Form could 
not
inherit directly from Zend_Form and precisely what are linked problems.

(But, I am agree with the fact it is better to proxy a Zend_Form implementation,
because callbacks methods, in theory, have nothing to do in a Zend_Form at 
first sight)

Original comment by tribal...@gmail.com on 4 Mar 2010 at 9:33

GoogleCodeExporter commented 9 years ago 
That is just so that developers can use their own Zend_Form implementation, and 
that,
that could not be possible with direct inheritance. Here is the reason ?

Original comment by tribal...@gmail.com on 4 Mar 2010 at 10:26

GoogleCodeExporter commented 9 years ago 
Hi,

Please take a look at Issue 190. I have detailed reasons for inheritance there.

Original comment by vlatko.b...@gmail.com on 4 Mar 2010 at 10:26

GoogleCodeExporter commented 9 years ago 
Indeed ! From now on, I see the light at the end of the tunnel ! Thanks.

Original comment by tribal...@gmail.com on 4 Mar 2010 at 10:46

GoogleCodeExporter commented 9 years ago 
Hi,

Made some changes.

Bvb_Grid_Source_Zend_* does not deal with decorators

Implemented the method getters|setters for decorators

$form->(get|set)ElementDecorator()
$form->(get|set)GroupDecorator()
$form->(get|set)ButtonHiddenDecorator()
$form->(get|set)FormDecorator()

Best Regards,
Bento Vilas Boas

Original comment by bento.vi...@gmail.com on 6 Mar 2010 at 9:42