search

contributte / forms

:sparkles: Extra contrib to nette/forms (@nette)
https://contributte.org/packages/contributte/forms.html
MIT License
9 stars 4 forks source link

Improvement suggestions #29

Closed MartinMystikJonas closed 2 years ago

MartinMystikJonas commented 2 years ago

Hi once again we are migration legacy project to new packages and this time we are looking at contributte form renderers as replacement for instante/bootstrap3renderer. But at this point there are few things we would need to customize. We can make our own version but I think some of our changes could be useful for others so I have some suggestions. If you like it I can prepare PRs.

1) Customizable label and control columns count. At this point if is hardcoded to 3:9 but in our project we often needs to customize it. I propose to add either public properties or setter to set column counts, replace hardcoded values in $wrappers with placeholders and replace placeholders by real values in DefaultFormRenderer::getValue.

2) Current renderer always make first button without any class to btn-primary. It does not make much sense in most of our forms so we would have to explicitly add btn-* to all buttons. What about change it to make button primary only if it is only button in form? Or if no button in form has btn-* class?

3) I do not know why some renderers do this? Is it really necessary?

$form->getElementPrototype()->setNovalidate(true);
f3l1x commented 2 years ago

Hi. 1 + 2 agreed. If you can prepare PR with change I would be glad. Point 3, I am not quite sure. Maybe it's not useful anymore.

MartinMystikJonas commented 2 years ago

ad 2) Which version do you preffer? If it is only button or if no button has btn class?

f3l1x commented 2 years ago

I think What about change it to make button primary only if it is only button in form might be good option.

MartinMystikJonas commented 2 years ago

Could I bump minimal version of Nette Forms to 3.1? There were some BC breaks and it would be way easier to require 3.1 than trying to support both.

MartinMystikJonas commented 2 years ago

Never mind it seems only incompatibility was test so I added skipping with older versions of Nette forms

MartinMystikJonas commented 2 years ago

Will be implemented in https://github.com/contributte/forms/pull/30