[5.0][Feature request] Fields as Objects

[AbbyJanke]

This is a suggestion coming from Gareth on the unofficial slack channel:

Instead of using arrays for creating fields, make them as objects.

From this:

// Single Field
$this->crud->addField([
   'name' => 'name',
   'label' => 'Label',
 ]);

// Multiple Fields
$this->crud->addFields(
 [
   'name' => 'name',
   'label' => 'Label',
 ].
 [
   'name' => 'name2',
   'label' => 'Label2',
 ]
);

To something like this:

// Single Field
$this->crud->addField( (new Field() )->name('name')->label('label') );

// Multiple Fields
$this->crud->addFields(
 [
   (new Field() )->name('name')->label('label'),
   (new Field() )->name('name')->label('label')
 ]
);

[tabacitu]

TLDR: Yup, I agree.

---

Yup, totally agree with this - I think this should be standard for fields, columns, filters, buttons, etc. And I believe we can make the syntax backwards-compatible, as in:

// Single Field
$this->crud->addField([
   'name' => 'name',
   'type' => 'text',
   'label' => 'Label',
 ]);

to actually make a new TextField(). You can pass an array to addField() and it will create the object, or you can pass the object directly and it'll just add it to the create/update array. Polymorphism ftw!

---

This would be a huge change across the package though, so I think it's more likely for the 4.0 branch, when we get to refactor everything. But it's one I'm pretty excited about, because this will also allow us to also have an inheritance system in place for fields; so TextField extends Field; Something like:

<?php

abstract class Field 
{
    // properties
    private $value;
    private $name;
    private $label;
    private $hint;
    private $prefix;
    private $suffix;

    // getters
    public function getCurrentValue() {}
    public function getCurrentValueWithFallback() {}
    public function getOldValue() {}
    public function getDefaultValue() {}
    public function getName() {}
    public function getLabel() {}
    public function getRelationModel() {}
    public function getHint() {}
    public function getPrefix() {}
    public function getSuffix() {}
    public function getAttributes() {}
    public function getWrapperAttributes() {}
    public function getView() {}

    // checks
    public function hasRelation() {}

    // setters
    public function setName()
    public function setDefaultValue()

    // aliases for setters
    public function name($args) { $this->setName($args); }
    public function default($args) { $this->setDefaultValue($args); }
    public function label($args) { $this->setLabel($args); }
    public function attributes($args) { $this->setAttributes($args); }
    public function wrapperAttributes($args) { $this->setWrapperAttributes($args); }
}

And then we can move a bunch of logic out of the blade files; so we'd do things like this:

<input
            type="text"
-            name="{{ $field['name'] }}"
+            name="{{ $field->getName() }}"
-            value="{{ old($field['name']) ? old($field['name']) : (isset($field['value']) ? $field['value'] : (isset($field['default']) ? $field['default'] : '' )) }}"
+            value="{{ $field->getCurrentValueWithFallback() }}"
            @include('crud::inc.field_attributes')
        >

or the longer

+            value="{{ $field->getOldValue() ?? $field->getCurrentValue() ?? $field->getDefaultValue() ?? '' }}"

The only thing I haven't cleared up yet in the implementation is how can we make it super easy for developers to overwrite the behaviour of a method, for a certain field type. I'm going to dump my thoughts below. Let me know what you think, if you have some time and headspace for this. I do think I have a way to do it, but I'm not sure it's the best way :-)

Premise:

• developer should be able to easily overwrite the behaviour of any field method, say getCurrentValue(), for an entire field type, say TextField, wherever it's used;
• developer should be able to easily overwrite the behaviour of any field method, say getCurrentValue(), for a field type, , say TextField, for the current CRUD Panel only;
• when creating a new field type, the developer should NOT be required to create a new class for that field type; just the blade file will do; So if the blade file name is fields/custom.php the field will use the Field object type by default, unless the app\Providers\Backpack\CRUD\Fields\CustomField class exists; modifying anything inside that class will apply the changes for that field type across all CRUD Panels;

For the above to happen:

• We should have a list of all registered fields inside the CrudPanel object;
• Since we're refactoring, that list should be a collection; All lists should be collections;
• We'd have a method to retrieve all registered field types; so something like CrudPanel::getFieldTypes() which will
  • look inside the resources/views/vendor/backpack/crud/fields folder for the views;
  • look inside the app/Providers/Backpack/CRUD/Fields folder for the classes;
  • return a collection of all field objects, each extending the Field abstract class;
  • so the result could be something like this:

    'text' => Backpack\Crud\app\Fields\Field::class,
    'number' => Backpack\Crud\app\Fields\NumberField::class,
    'custom' => App\Providers\Backpack\CRUD\Fields\CustomField::class,
    'phone' => App\Providers\Custom\Fields\PhoneField::class,

• We'd have a method to allow the developer to customize the class name / path for a certain field, or for all fields; so something like $this->crud->loadFieldType('custom', App\Providers\Custom\CustomField::class);;

My thoughts are all over the place. I hope I get to formulate these thoughts as a PR soon, so we see if it makes sense or not. That being said, this is a big change, with small benefits:

• the views would be a little cleaner;
• a little easier to type fields in IDEs;
• a little easier to debug bad usage of our methods;
• it's a more "proper" way to do things;
• it's a better way to make sure all fields have at least some logic handled;

So I'd say this is more of an ego change, rather than a true functionality. But I do agree it should be done.

[AbbyJanke]

It is a big change with small benefits up front. But another benefit in the end with the cleaner views is that we can make Backpack easier to customize with themes. Even if we don't want to add themes to be built in, it would allow others to create snippets easier so can can just be a "drop in replacement."

[AbbyJanke]

Some code provided by Gareth today with some modifications by myself to better stick with current naming conventions:

namespace Backpack\CRUD\Objects\Fields;

abstract class BaseField {
    public function __call( $name, $args ) {
        $this->{$name} = $args;
        return $this;
    }
}

namespaceBackpack\CRUD\Objects\Fields;
class FieldTypes {
    const text = "text";
}

namespace Backpack\CRUD\Objects\Fields;
/**
 *
 * @method Text    name(string $name)
 * @method Text    label(string $label)
 *
 */
class Text extends BaseField {
    public $type = Backpack\CRUD\Objects\Fields\FieldTypes::text;
}

$var = new Backpack\CRUD\Objects\Fields\FieldTypes::Text;
$var->label('Label')->name('name');

[tswonke]

A few weeks ago I started a package to introduce a fluent-syntax workaround for Backpack Crud (https://github.com/webfactor/laravel-backpack-fluentsyntax). It uses a trait which provides new methods for adding fields and columns as objects. There are still some fields missing.

Theses methods are generating the required arrays and are calling original crud methods.

Main aspect here was to force required parameters to be set in the constructor of each field/column, provide fluent-syntax for optional parameters and only show available methods for each field/column in the autocompletion of the IDE.

Example for Columns from a project:

Of course there is no further benefit currently without refactoring Backpack Crud to use these objects directly (in the views). But if there is anything in my small package that could help you in this topic - feel free to use it.

[tabacitu]

Hi @tswonke - I do know about your fluent syntax package, and I like it, but it's difficult decision whether to (A) go with a fluent syntax, then refactor Backpack fields/columns/filters/etc to objects, or (B) refactor first, and the fluent syntax will be a necessary change.

On the one hand, option (A) is more incremental, and it would help us have something faster. Especially since you've already done the heavy lifting (thank you so much). It's also the way we've been doing things for the past 2 years. And incremental progress has helped us A LOT in discovering what to build, what not to, and better ways to do stuff.

However, option (A) also means changing the syntax twice. Once with the Fluent Syntax, once with The Rewrite. Most likely it's not going to be exactly the same - there are always things you want different when refactoring. And that scares me - it means two major breaking changes, so two different times when people have to manually upgrade their apps... Food for thought...

[tswonke]

@tabacitu I totally understand your concerns!

Maybe there is a way to introduce fluent syntax as a kind of (non-breaking) "beta" functionality (most users should use arrays till v4.0). Then we have a longterm chance to test and enhance it.

I will think about it and talk to @OliverZiegler as well.

[tswonke]

As an addition: My package also supports the intended syntax from post 1:

The (helper) methods were just a test. I think I will remove them.

[tswonke]

One more thing, because I'm not sure if this is mentioned here in some way.

My "ultimate" wish:
I would love to be able to write packages (private or public) which introduce new fields/columns/filters/buttons AND the corresponding blades.

If this can be achieved, I don't think this is "small benefit" but a huge one.

[AbbyJanke]

Introducing new views is already a possibility. The way i did it is this:

src/resources/views/vendor/backpack/crud{fields|buttons|colums|etc}
src/resources/views/vendor/mypackage/{my other custom views}

Then when publishing views just have it publish the resources folder and it publishes the new views into the /app/resources folder.

[tabacitu]

@AbbyJanke - I think what @tswonke was saying that he'd like Backpack to pick up the package fields/columns/etc without him having to publish the files. So he can push updates to those files if necessary (since they're still in vendor/domain/package).

I did have another conversation about this issue (and as I remember a fix for it too), but I can't seem to find it... I'm pretty sure I had a fix implemented, or at least thought about a fix... Damn it... I'll get back to you on this one.

[tabacitu]

@AbbyJanke , @tswonke I've created a separate issue for that feature here Laravel-Backpack/CRUD#1385

[tabacitu]

Got to think about this again a bit. Dumping my thoughts for you guys, and for future-me. Pros and cons of the fluent syntax:

PROs:

• developers could use IDE auto-complete when typing addField() statements, thanks to the fact that $fields would be an object;
• thanks to the fact that each field would be an object, each FieldType could have some mandatory attributes; so if you add a select2 field but don't specify a relationship, it would throw a nice error saying you need a relationship;

CONs:

• difficult/impossible to implement in a non-breaking way;
• the fluent syntax is not prettier on the eyes, imo;

[tswonke]

@ all: Please feel free to try my kind of playground-proof-of-concept-package to try the fluent-syntax: https://github.com/webfactor/laravel-backpack-fluentsyntax which I presented above (https://github.com/Laravel-Backpack/CRUD/issues/1351#issuecomment-386882705).

Attention: I removed the helper methods, you have to use the Classes (see https://github.com/Laravel-Backpack/CRUD/issues/1351#issuecomment-387078543, in each case the first line).

My approach was to say, that in the constructor all mandatory fields have to be defined. The autocompletion works fine for me. There are not all field/column types available yet as this was only an experiment.

This is not a "real" object implementation because the objects are transformed to arrays backpack requires. It's just a step to check, whether it's a way to handle this.

Regarding "not prettier":

I had the same thoughts as @tabacitu in the last days when I used fluent syntax!
But I assume that we can adapt to it and the pros overbalance this.

[AbbyJanke]

So playing around with this a little bit to create something viable and had some thoughts:

First of @tswonke I am using your code as basis seems to work well (submitted a PR for a new field type, have more i can submit as well)

But also had another idea for a way to make it work. Say we add a new trait to the controller called "FieldTypes" and then in the setup() we add lines such as this:

$this->enableAddress();
$this->enableText();
$this->enableEmail();

then instead of having to type out new TextField() every single time we can just do something like this:

$this->crud->addField($this->fields->address->name('test')->label('Example'));
$this->crud->addField($this->fields->address->name('test2')->label('Example2'));

Or does this seem kind of redundant and and more complicated? Just looking for some feedback.

Reason I say have a function called enableAddress() etc for each field is it's best not to load every single field type every page load for no reason. Something I'd also like to consider is we create additional second functions called fields() and and columns() and we can load those within the specific page functions index() & show() for columns create() and edit() for fields. will likely help speed up page loads, less useless code being run on pages that don't need it and what not.

@tabacitu thoughts?

[tswonke]

thank you @AbbyJanke for the pull request, I just made a little change request ;-)

concerning your post:

In my eyes the advantage of using new Textfield() is that one can easily use own fields/columns... separate packages or even private.

[sergeyugai]

@ShamarKellman added this to my https://github.com/sergeyugai/backpack-fields-as-classes

@tabacitu I was wondering if you got a moment to check this out. This is my package to address this particular issue in object-oriented way, which is NON-breaking way, making all the existing code compatible.
To cut story short, my package is a set of classes that one can use with backpack - it makes no difference for backpack since all classes are Arrayable - yet it provides all those nice methods for IDE autocomplete. It also allows to use Fields as Classes in custom blade views.

What's even cooler is that this is automatically generated from your current backpack documentation. With a few updates to its structure, we could achieve 100% accurate fields/column classes.

[tswonke]

@tabacitu

is there any progress on this topic? I'm asking, because I see many examples in other 4.0 topics but they are (as far as I have seen) still in array style. Also widgets could/should be designed as objects I think...

[sergeyugai]

@tswonke hi Thomas, was wondering if you checked out my solution for this.
I actually use my "backpack fields as classes" in one of my projects and I am fairly satisfied with result - code becomes more readable, autocomplete helps a lot + I added ability to have fields collections that allow to apply same attributes ("tab" for instance) to a group of fields.
And this still works without any modification to Backpack since internally every objects, when passed to Backpack, gets converted to array (because base class implements Arrayable interface).

[tswonke]

@sergeyugai
Yes I have checked your solution and it is mainly the same "solution" (as a result) like my one:
Using a wrapper object that generates Arrays so that we can chain and use autocompletion.

But both packages doesn't tackle the main underlaying backpack issue: In my eyes fields, columns, filters... + new things like cards, widgets... they all should be objects for any number of reasons.

And because @tabacitu is currently working on BP4 I just wanted to know whether this is still "up".

[sergeyugai]

@tswonke yes, I agree that all that stuff must be represented with classes/objects rather than array.
I just figured that, to get there, introducing arrayable-based classes would be step 1 - this would allow the change to be not-so-breaking, if breaking at all (for all the existing code backpack code).

[tswonke]

This was also my intend while creating the package - something like a proof of concept.
But because BP4 will definitively be a breaking change it could/should be implemented in the core.

[AbbyJanke]

@tabacitu @pxpm So I know you guys worked hard and got at least filters done, and I'm seeing the files for the other options in v4.1 Just wondering if it actually made it in and stuff, do we just need to do documentation?

[pxpm]

fields, filters, columns etc have now an fluent syntax.

check it out in demo the file: app\Http\Controllers\Admin\FluentMonsterCrudController.php

Documentation: https://backpackforlaravel.com/docs/4.1/crud-fluent-syntax

[AbbyJanke]

@pxpm Thank you :) Missed that in the docs, expected it to be in the individual pages with 4.1 to push more towards that rather than a separate page.. Maybe something I can work on to help more. Still adjusting from 3.4 to 4.1 :)

My main issue is i was trying to understand the logic for an ajax filter but wasn't clear to me at the time without seeing that page. I want to convert my current project to use it cause obviously i liked the idea brought it to the table here. But thanks for all the hard work will look into more in the morning 💤 time for me.

[tabacitu]

Yeah like Pedro said, there's all done 🥳 I've decided not to push the fluent syntax too too hard right now because I didn't find a right way to explain/present both. It could be really confusing to first-timers to present both syntaxes, and it would bloat up the docs too. When in doubt, I defaulted to the one that's easier to understand 😄 aaand the one that we have 100% confidence in - the fluent syntax might still have a few quirks, because of the way it works.

I imagine in the near future we'll be able to create sort of tabs for each field definition - one for fluent one for non-fluent syntax. Or a general setting per user, "show fluent syntax" that would show one or the other, but not both.

[AbbyJanke]

I'm converting my my current project to use it now so will test it out as much as I can and add stuff as I can. I was thinking about working out Widgets in fluent syntax since that's not implemented just yet :) Good to close this issue i'm thinking? You have it tagged 5.0 but if majority of it is in there?

[tabacitu]

Hmm... let's leave it open, I think there are a few good ideas here that haven't been implemented yet. I'd call what we have now a "first step in transitioning to objects" 😄 but there's still a lot of work to be done - ideally we'd move pretty much all logic that's inside the Fields, Columns, Buttons, Filters traits into their respective objects - CrudField, CrudColumn, CrudButton, CrudFilter. I think this thread will be useful when doing that, so let's leave it open.

Yes please, it's very helpful if you use the fluent syntax a lot, I plan to do that myself in a few projects to triple-check.

Heads-up @AbbyJanke - widgets are already fluent in 4.1. It's actually "the default" - check out the Widget docs.

[jcastroa87]

Hi.

Due to not activity on many years i will close this discussion, if there is needed feel free to reopen or create a new one.

Cheers.