kristijanhusak / laravel-form-builder

Laravel Form builder for version 5+!
https://packagist.org/packages/kris/laravel-form-builder
MIT License
1.7k stars 297 forks source link

Value callback string #195

Open rudiedirkx opened 8 years ago

rudiedirkx commented 8 years ago

I have a form that adds date fields:

->add('client_since', 'date', [
  'label' => trans('ORGANIZATION.CLIENT_SINCE'),
  'rules' => 'date_format:Y-m-d',
])
->add('client_until', 'date', [
  'label' => trans('ORGANIZATION.CLIENT_UNTIL'),
  'rules' => 'date_format:Y-m-d',
])

The db columns are default Laravel timestamps with Carbon init for date objects (protected $dates).

To make the HTML form understand the date, I have to add a value callback to the form to rewrite it to Y-m-d at the very last second, for the default value:

->add('client_until', 'date', [
  'label' => trans('ORGANIZATION.CLIENT_UNTIL'),
  'rules' => 'date_format:Y-m-d',
  'value' => function($value) {
    return Form::transformDateFieldValue($value); // Form is an app override that extends your Form
  },
])

I have several forms with several date fields. There must be a better way.

I was very sad to learn this does not work:

'value' => 'Form::transformDateFieldValue',

because that's not considered a callback. (Laravel's value() function, but also your own code, specifically check for Closure.)

Is there a good way to do this?

kristijanhusak commented 8 years ago

I can think of 2 solutions, not sure which one is better:

1) Using custom accessor & mutator (link in docs) that will handle transforming Carbon instance to string and vice versa. Something like this:

public function setClientSinceAttribute($value)
{
    $this->attributes['client_since'] = Carbon::parse($value);
}

public function getCientSinceAttribute()
{
    return $this->client_since->format('Y-m-d');
}

Pros: No need for creating custom field type Cons: Needs to be added for each Model

2) Creating custom date field type that will have predefined closure:


class CustomDateType extends FormField
{
    public function __construct($name, $type, Form $parent, array $options = [])
    {
        $this->valueClosure = function($value) {
            return $value->format('Y-m-d');
        }
    }
}

Pros: Good for multiple models, keeps code DRY. Cons: Not really needed when converting to Carbon is required for only one Model

I haven't tested any of these solutions, but they should work. Hope I helped.

Let me know which one better works for u.

rudiedirkx commented 8 years ago

There is of course a 3rd option: your date field could support Carbon instances and print the right format in the value attribute. Since many/most/some date columns in Laravel are Carbon, I think that's not a bad idea... I might be biased, and lazy, because it's easier for me.

CaporalDead commented 8 years ago

@kristijanhusak @rudiedirkx FYI, I'm working on a PR to add "filters" on forms. That way, we could format/escape... values.

rudiedirkx commented 8 years ago

Sounds good. Can we track it somewhere?

I created the custom date field type to fix my problem, because it'll be used a lot.

CaporalDead commented 8 years ago

Not now, still having issues to fetch data after isValid. I'll try to push it this week and let you know.