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

Customizable details #750

Closed
wants to merge 2 commits into from
Closed

Customizable details #750

wants to merge 2 commits into from

Conversation

AbbyJanke
Copy link
Contributor

Similar to customizable fields within Backpack this allows you to customize the 'show' views with two options (statistics and details)

Statistics are free flowing small info boxes on the left, regular details are placed within a table. If no statistics are defined the table is full-width.

see issue #749 for discussion

screen shot 2017-06-20 at 6 35 22 pm

Information on how to define each detail option is located within the view until we get better documentation for it.

@tabacitu
Copy link
Member

tabacitu commented Jul 7, 2017

Hi @AbbyJanke ,

Wooww. This looks like a lot of work, thanks for the PR.

I do think Backpack\CRUD needs to include only broad functionality by default, though, and I don't think the stats on the left would fit many projects...

I do love how the right-hand-side info is being displayed, though. That's pretty much what I had in mind for the SHOW functionality (or PREVIEW, whatever). Putting it in a zebra table makes so much sense, you're right. What stopped me from developing the preview functionality so far was that:

  • I think it makes a lot of sense to use the same code to show the entries as "columns" and as "preview", for code not to be duplicated (and us not having to maintain the same thing twice); if it does the same thing, it should be one thing;
  • this brings a naming convention challenge: how do you call something that's also a "column" in the LIST functionality, but also works in the "preview", but not as a column? Now that I see you've put them in a table (which makes a lot of sense), maybe "cell" would make sense?
  • I think the preview SHOULD work without having to mention every little field; that's "the dream" :-) So something like setFromDb(), but only for the preview fields, should probably be implemented by default, and you'd only need to change the "type" for a few columns, if you're actually using it;

What do you think?

@tabacitu
Copy link
Member

tabacitu commented Jul 7, 2017

Also, I probably should not be saying this because I'm overcomplicating it, but in the medium to long future (maybe Backpack\CRUD v4), since:

  • there are a bunch of operations you can do (LIST, CREATE, UPDATE, PREVIEW)
  • ideally their fields/columns/cells use the same names
  • ideally setFromDb() will "just work"

You normally won't have to specify the field/column/preview types at all. But when you do, you'd be able to "overwrite" the types be specifying something like this on your Model:

class Monster extends Model
{
    use CrudTrait;

     /*
    |--------------------------------------------------------------------------
    | GLOBAL VARIABLES
    |--------------------------------------------------------------------------
    */

    protected $table = 'monsters';
    protected $primaryKey = 'id';
    public $timestamps = true;
    // protected $guarded = ['id'];
    protected $fillable = ['address', 'base64_image', 'browse', 'checkbox', 'wysiwyg', 'color', 'color_picker', 'date', 'date_picker', 'start_date', 'end_date', 'datetime', 'datetime_picker', 'email', 'hidden', 'icon_picker', 'image', 'month', 'number', 'float', 'password', 'radio', 'range', 'select', 'select_from_array', 'select2', 'select2_from_ajax', 'select2_from_array', 'simplemde', 'summernote', 'table', 'textarea', 'text', 'tinymce', 'upload', 'upload_multiple', 'url', 'video', 'week', 'extras'];
    // protected $hidden = [];
    // protected $dates = [];
    protected $casts = [
        'address' => 'array',
    ];
    protected $crudTypes = [
        'address' => 'address',
        'something' => 'email',
        'description' => 'tinyMCE'
    ];

Maybe, I don't know if it's doable or such a great idea. Just wanted to let you in on it, maybe it will be relevant in thinking this PREVIEW functionality through.

@AbbyJanke
Copy link
Contributor Author

That functionality should be doable to be honest. Would take a lot of work to be honest and a major breaking change.

The stats on the left are basically just model relations, things that I could display within the table but wanted to make it look prettier, it allows the option to add them or just the table. Will see what i can do working on your suggestions.

@tabacitu
Copy link
Member

tabacitu commented Jul 7, 2017

A LOT of work. I know. But I don't think it should be a breaking change, as right now we do have the CrudController::show() method, that just dumps the info from the DB. It would just need to... you know... not do that and do what it's supposed to do :-))

You can enable the "show" method with $this->crud->allowAccess('show');

Ok, it's doable, I agree, but... what do you think about it? Good/bad course of action? Any way we can improve on it?

@tabacitu
Copy link
Member

tabacitu commented Jul 7, 2017

This is a primitive attempt at improving the "show" view: #308

@AbbyJanke
Copy link
Contributor Author

The table for the preview of an item would not be the breaking change, but your ultimate goal of moving from defining each column in my opinion would be as you are automatically doing setFromDB() and moving the field customizations from the controller to the model. I think that is a great method to be honest and even better as I have manually set all of the columns myself for better control, but also need an array such as $crudHides = ['id', 'updated_at']; to hide anything you don't want to show. but my concern is also, what if you want one thing to show in preview but not within the list?

@AbbyJanke
Copy link
Contributor Author

Closing PR, working on better method to do this automatically and removing left statistic options.

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.

2 participants