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

Nullable support #14

Merged
merged 1 commit into from
Sep 16, 2020
Merged

Nullable support #14

merged 1 commit into from
Sep 16, 2020

Conversation

royduin
Copy link
Contributor

@royduin royduin commented Dec 6, 2019

Support for the ->nullable(), see: https://nova.laravel.com/docs/2.0/resources/fields.html#nullable-fields. So null can be stored instead of an empty array [].

To get this working we need to use one of these:

  • ->nullable(true, '[]')
  • ->nullable()->nullValues('[]')

For reference: https://github.com/laravel/nova/blob/91301a38949885f0c29a9b962d193ca77cf5ee50/src/Fields/Field.php#L363

@dillingham
Copy link
Owner

This seems to change the default behavior which is that this field acts on arrays primarily and firstly. Making users now have to define that the default null value is an array seems off to me. Thanks for the time / thought put in though.

@dillingham dillingham closed this Dec 19, 2019
@royduin
Copy link
Contributor Author

royduin commented Dec 19, 2019

It doesn't change the default behavior. It only changes from [] to null if you add one of the nullable options mentioned. The default is still an empty array: [].

Just like any other field in Nova; with the default Text field it saves an empty string '' by default when nothing is filled. With ->nullable() it saved null when empty.

Please reconsider, merge and create a new release.

@dillingham dillingham reopened this Dec 19, 2019
@dillingham
Copy link
Owner

I will pull this down and verify, sorry just skimmed

@royduin
Copy link
Contributor Author

royduin commented Dec 19, 2019

Awesome, thanks in advance!

@royduin
Copy link
Contributor Author

royduin commented Sep 16, 2020

Any updates?

@dillingham dillingham merged commit c93dc29 into dillingham:master Sep 16, 2020
@dillingham
Copy link
Owner

sorry @royduin, published a release

@royduin
Copy link
Contributor Author

royduin commented Sep 17, 2020

Thanks 💯

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

Successfully merging this pull request may close these issues.

None yet

2 participants