forms
forms copied to clipboard
Form render fails with minimal control implementation on missing get/setOption()
See test that confirms the failure: http://pastebin.com/Xwpha0v2
my suggestions:
- Nette\Forms\IControl should extend Nette\ComponentModel\IComponent (shouldn't even break BC of any existing controls, because $form->addComponent() receives only IComponent anyway)
- wouldn't it be better if DefaultFormRenderer holds rendered components list in SplObjectStorage rather than setting an option to the control object?
By the way, there are a lot more methods missing from the minimal interface implementation to satisfy DefaultFormRenderer:
- getForm
- isRequired
- getLabel
- getControl
- hasErrors
- getErrors
I think all these should be also included in IControl interface. EDIT: or create IRenderableControl interface extending IControl for these to keep the IControl simple
If you agree with my suggestions, I can implement and pullrequest it.
Another problem - $control->getOption('type') === 'hidden' check is done by the renderer.
I suggest to make the check using $control instanceof Nette\Forms\Controls\HiddenField as it does not make sense to place anything other than input[type=hidden] to the hidden controls container. And we get rid of another usage of the not-oop-and-typehint-friendly method getOption().
Also, getOption() is used for id, class, description and type===button. getHtmlId() (already in BaseControl), getClass() getDescription() and isButton() would be also useful in IRenderableControl interface.
I've just ran into this issue as well. Was kind of confused what happened.
I render form with <form n:name="form">, fields with <input n:name="field">, but I have one special composite field which groups RadioList and TextInput into one. This composite field extends Nette\Forms\Container and implements Nette\Forms\IControl. I render it with {control $form['deadlineDateField']} and on rendering it fails with Call to undefined method DeadlineDateField::setOption()
Implementing these stubs "fix" (read: it does not scream) it, but proper solution would be better.
public function setOption($key, $value)
{}
public function getOption($key, $default = null)
{}
public function getOptions(): array
{
return [];
}
I'm sorry, but I'm just starting to understand what's going on in component model so I don't feel strong enough to make PR.
I know about these issues, it is not possible to solve it gradually to maintain backward compatibility, so I leave it to the next larger version.