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

Value callback string #195

Open
rudiedirkx opened this issue Dec 28, 2015 · 5 comments
Open

Value callback string #195

rudiedirkx opened this issue Dec 28, 2015 · 5 comments

Comments

@rudiedirkx
Copy link
Collaborator

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

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

  1. 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
Copy link
Collaborator Author

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

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

@rudiedirkx
Copy link
Collaborator Author

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants