adamwathan / bootforms

Rapid form generation with Bootstrap 3 and Laravel.
MIT License
417 stars 103 forks source link

Add form-group methods #78

Closed Propaganistas closed 8 years ago

Propaganistas commented 8 years ago

This PR adds some handy methods to interact with the created FormGroup.

In my case this is particularly useful for setting the form-group-lg class to the form group.

adamwathan commented 8 years ago

Nice thanks! What do you think about adding a method to basically change the target of __call? Right now it always just goes through to the input element, but maybe it would be smarter to track a variable on the GroupWrapper like callTarget and use that to control which element the calls get applied to?

Could do like:

BootForm::text('Name', 'name')->addClass('foo')->group()->addClass('form-group-lg')->label()->addClass('sr-only');

It would always still have to return the whole GroupWrapper so that things keep rendering as expected, but would give you complete flexibility. Any opinions?

Propaganistas commented 8 years ago

I see where you're going. Yet that would break the current fluent behavior to some extent: the target calls need to precede their called method. Moreover, what if multiple calls need to be made to a specific target? You're going to need to call the target caller every time, which isn't very clean in my opinion...

How about taking the magic way (this could be improved, just to show the idea)?

public function __call($method, $parameters)
{
    if ($group = strpos($method, 'ToGroup', strlen($method) - strlen('ToGroup'))) {
        call_user_func_array(array($this->formGroup, substr($method,0,$group)), $parameters);
    } else {
        call_user_func_array(array($this->formGroup->control(), $method), $parameters);
    }

    return $this;
}

BootForm::text('Name','name')
    ->addClass('add-input-class')
    ->addClassToGroup('this-is-a-group-class')
    ->data('add-input-data')
    ->dataToGroup('add-group-data')
adamwathan commented 8 years ago

The way I was thinking it would work you wouldn't need to precede every call with the target call, it would just flip the group wrapper to be looking at that target until you flipped it again.

PR'd it quickly here: https://github.com/adamwathan/bootforms/pull/79

Here's the test that shows the usage: https://github.com/adamwathan/bootforms/blob/aw-group-wrapper-targets/tests/BasicFormBuilderTest.php#L609-L618

What do you think?

Propaganistas commented 8 years ago

This would still imply the called methods to be ordered in some way (in contrast to current BootForm and Form behavior).

Anyhow, since you're the package maintainer it's totally up to you to decide how to continue with this.

Propaganistas commented 8 years ago

Any updates on this?

adamwathan commented 8 years ago

Do you ever use this for anything besides adding the form-group-lg class? Maybe it we should expose a specific method for that since it is a Bootstrap feature and this is a Bootstrap package. Will merge this anyways.

Propaganistas commented 8 years ago

For the moment I don't.

Anyhow, when exposing this into a method, you'd need to be consistent and have available: