forms icon indicating copy to clipboard operation
forms copied to clipboard

Form render fails with minimal control implementation on missing get/setOption()

Open richard-ejem opened this issue 8 years ago • 3 comments

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.

richard-ejem avatar Aug 11 '16 08:08 richard-ejem

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.

richard-ejem avatar Aug 11 '16 08:08 richard-ejem

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.

dakur avatar Mar 26 '20 20:03 dakur

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.

dg avatar Mar 26 '20 21:03 dg