Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
richard-ejem opened this issue Aug 11, 2016 · 3 comments
Milestone

Comments

@richard-ejem
Copy link

richard-ejem commented Aug 11, 2016

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
Copy link
Author

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.

@dg dg added this to the v3.0 milestone Jun 21, 2017
@dakur
Copy link

dakur commented Mar 26, 2020

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.

@dg
Copy link
Member

dg commented Mar 26, 2020

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 dg modified the milestones: v3.0, v4.0 Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants