[Suggestions]

[Frondor]

Well, lately I've been quite interested in this project and I try to contribute as much as I can. My memory tends to fail very often, so I'll write a list of suggestions I CONSIDER necessary.

:construction:

1. :warning: Remove resources/views/users views so it makes use of default BREAD views, and properly configure the data rows to make use of Input details JSON api.
2. Allow "server-paginated" BREAD tables to be ordered by column
3. Implement the use of filters for "server-paginated" BREAD tables
4. This
5. Update the docs. And probably include a new section for "Input JSON API" or something like that, since the number of available options are increasing.
6. Implement some library like a little code editor, to make the writting of JSON code while creating BREADs, less painful. #697
7. Make use of bootstrap-datetimepicker (or better) library... It is there! But we are not using it at all :sweat_smile:
8. Make login screen responsive, add bootstrap library, and use Voyager::setting('logo') (if isset) instead of the default blank wheel #697
9. ~Think on something to allow administrator changing loading icon.~ It has always been there
10. all the image inputs, should read from the media manager instead of using the default html file uploader.
11. show view/edit/delete according to permissions, so the user dont click on it and get an error which is bad user exp.
12. Create TestCase for parsed/resolved relationships #638
13. Enabling selecting items from the Media Manager in rich_text_area
14. :warning: The new way to name BREAD files media/{BREAD}/589c6c66.jpg.
15. ~:warning: Ability to register formFields like Voyager::addFormField(DateHandler::class);~

What do you think?

[marktopper]

You have done some wonderful pull requests @Frondor. A big thanks for that.

1. I believe removing all the extra BREAD views might be a good idea to make it easier to maintain since they are all using the same BREAD view. However as it is now it should still be possible to overwrite the BREAD views for specific tables. But I do not believe it's needed to be done by default as it is now for users and posts. I am not quite sure I get your plans for configuring the data rows to make use of Input details JSON api, can you please explain in further details?
2. 👍 Could be turned into a option, maybe a option in the model or somewhere, open for ideas.
3. 👍
4. Indeed a performance issue. 👍
5. The documentation does indeed have some issues, there are lots of features that are not explained yet in the documentation. I have a lot of works to do there.
6. Do you know any editor for this purpose?
7. We should indeed be using libraries to format inputs to make everything easier for everyone. 👍

[Frondor]

Thank you Mark, I appreciate your appreciation :laughing: I try to contribute as much as I can.

I agree with everything, about the code editor I like CodeMirror and Ace. In my next PR I may implement one of them.

[marktopper]

Wonderful. I personally have only experience with Ace Editor, haven't tried CodeMirror. Since it seems that we are only gonna need it for JSON for now, I think we should test out which works with JSON and their performance.

[Frondor]

Both work almost the same... Ace seems a bit more advanced than CodeMirror, and both works with "modes". since we only need JSON mode, either of those two would be fine.

[marktopper]

I am open for both ;)

[fletch3555]

@marktopper, perhaps we make the code editor as a hook? Seems small/simple enough to build out that way. It's not like functionality wouldn't work without it.

[marktopper]

Well, not quite sure whether or not this should be in a hook, since it would really help people enter the correct JSON into the BREAD fields to have some kind of editor and better validation.

I'm open for a discussion on this 👍

[hazem1taha]

I just want to add more suggestions (not sure whether to put them here or not):

1. Using CKeditor instead of the current one or event make a setting for selecting the suitable editor.
2. Exporting Database functionality.
3. Creating full migration including the table fields instead of just a migration and the increments.

[marktopper]

@EngHazem great suggestions, here are some comments:

1. CKEditor could be the default rich text editor for Voyager, but could be editable by maybe using a hook (Voyagers plugins system). Then the hook could replace the rich text handler. Then there are no limits for the editors that can be added later.
2. Do you wish to export the whole database or just some specific parts of it? Cause if this is for migrating local application to production, I think we should start a separate discussion for that.
3. Full migrations are already planned and I do believe that @abdgad have been digging into this.

[hazem1taha]

@marktopper

Great! Do you mean with Voyagers plugins system, the idea of overriding views in the resources/vendor/ ?

[marktopper]

@EngHazem: Voyagers plugins system, it however haven't been announced yet since there are still some testing going on.

[hazem1taha]

It will be a great addon! I was going to suggest this feature. What if I want to run a specific action after saving the settings form ? Does Voyager currently support that ?

[marktopper]

@EngHazem You can do this using events.

[Frondor]

This is what I've done so far:

You can no longer submit the form with invalid json fields.

[marktopper]

Good idea with the last check before submission. Ensures that people does not create BREAD with invalid JSON. 👍

[Frondor]

I believe removing all the extra BREAD views might be a good idea to make it easier to maintain since they are all using the same BREAD view. However as it is now it should still be possible to overwrite the BREAD views for specific tables. But I do not believe it's needed to be done by default as it is now for users and posts. I am not quite sure I get your plans for configuring the data rows to make use of Input details JSON api, can you please explain in further details?

@marktopper If we're going to get rid of those overrides (views for users BREAD, posts, etc...), they should be "configured" by default, for example, the fields we are showing/hidding for each B.R.E.A.D, fields details, etc... (otherwise, if we simply delete the users folder, the users BREAD shall look messed up). I believe that's part of the installation process :thinking:

[marktopper]

Good point @Frondor. I think that is a better approach to make some kind of configurations for the layout of the BREAD. So then there can be a set of "layouts" to use for a BREAD.

[ctf0]

a couple more features to add

• all the image inputs, should read from the media manager instead of using the default html file uploader.
• show view/edit/delete according to permissions, so the user dont click on it and get an error which is bad user exp. https://github.com/the-control-group/voyager/pull/695
• add away to support packages like https://github.com/spatie/laravel-translatable which allow us to have multi locale without too much work, as atm all i can think of to having something like that is to have different inputs for different locals.

[marktopper]

Good suggestions @ctf0. However I am already working on a system like laravel-translatable for Voyager. But will be turning it into a Voyager plugin that can easily be installed for those needing it.

Reference #561

[ctf0]

awesome, am also thinking of adding http://plugins.krajee.com/file-input for the images & even have an example https://gist.github.com/ctf0/8cdad76b8b8a0c80f2241d47ab430402 so we dont need to use ajax calls on each upload

[Frondor]

awesome, am also thinking of adding http://plugins.krajee.com/file-input for the images & even have an example https://gist.github.com/ctf0/8cdad76b8b8a0c80f2241d47ab430402 so we dont need to use ajax calls on each upload

In my opinion, we should wait until L5.4 is supported, so we can create an image_uploader blade component to load in edit-add.blade.php views, and use the media manager just like wordpress does.

I'm updating the main post.

[marktopper]

Reference according to Laravel 5.4 support #626

[Frondor]

@marktopper I wonder how all the changes we've been committing to master would merge to v0.11 branch or vice versa... I'm not a git advanced user but to be honest, I'm afraid of the merge breaking too many things :laughing:

[marktopper]

Actually I did most of them yesterday 😆

[marktopper]

https://github.com/the-control-group/voyager/commit/20604a5c835384722c4ef0549498d9c645f0c0ce

[ctf0]

a continuation for the image , maybe a little bit later but am just adding for discussion

• use https://packagist.org/packages/spatie/laravel-medialibrary instead of normal intervention
• use https://github.com/kblais/laravel-uuid for the image names instead of hashing.

on other hand for the Additional Field Options, like Generating Slugs

• maybe we can have the ability where we can add a method to the model then call it on the input, like

  {
  "cleanHtml": {
      "origin": "self", 
  }
  }

  and in the model

  public function cleanHtml($input)
  {
      // do stuff
  }

• for the Relationships not sure if possible or not, maybe we can register the relation on the fly through the options aswell

  {
  "relationship": {
      "key": "id",
      "label": "name",
      "type": "belongsTo",
      "model": "App\\User"
  }
  }

  or even better, describe that inside the database table creation view.

[Frondor]

I don't know about @marktopper but I'd like to keep BREAD relationships as simple as possible. Also there's no point to specify another model different from the one already used by current BREAD, and the eloquent relationships has to be manually added so I wouldn't consider that suggestion.

[marktopper]

I must agree with @Frondor on that one.

However about the libraries:

use https://packagist.org/packages/spatie/laravel-medialibrary instead of normal intervention use https://github.com/kblais/laravel-uuid for the image names instead of hashing.

Then what would they give or improve to this package? Maybe we should open up a different issue for discussing those.

[ctf0]

@marktopper

• the usage of the medialib package will make it much easier to generate different sizes for the image through using glide. it also support working with pdf/svg/vid files which i believe would be a feature that users would like to have. i would highly recommend giving the docs https://docs.spatie.be/laravel-medialibrary/v4/converting-images/defining-conversions a quick read

• about the uuid its also recommended over the hash, not sure how u r generating the hash atm but maybe its based on the file name + the current timestamp?, but uuid doesnt require such a pipe-line + its more secure, however the downside that its bigger than the hash and doesnt play well with indexing.

---

a side from those 2, i would suggest adding a btn to the rich text inputs toolbar that show the media manager in a pop-up so the user can select any images/vids/etc... he wants and once clicked it gets added to the input, so now we can 1- add a direct link to that file and get the benefits of cdn/cloud storage like s3 2- the db will be much smaller cuz now instead of adding a dataURI for the images "which is bad for big files", we instead use links 3- the user can now upload any videos or files he wants through the media manager and access them directly without going back & forth between the view and the media manager to get the direct links of those files.

[marktopper]

@ctf0

• I like the way conversions work. I'm open for pulling in this library for Voyager v0.11. However I think it might require some work in other to get it to work properly with Voyager.

• Currently the filename is random-20-characters + timestamp i believe. Which is no good. I however believe that the uuid would be a bit to long. I am open for solutions here. However thoughts?

[marktopper]

About the button for the rich text input toolbar for enabling selecting items from the Media Manager, then a pull request is more than welcome 👍

[marktopper]

I have added this to the milestone of v0.11. Hopefully we can get those things done in time.

[Frondor]

@marktopper Wow! This is such a big list for a milestone :laughing: but meh, I think it's a decent challenge.

I can create a Vue.js component for the embedded media manager if needed, but first we may need an AJAX api (and with this I simply mean a structure for controllers) so we can use it for select2 inputs that actually, query the whole list of records for a relationship.

Regarding to uuids, they are large indeed and people tend to make bad uses of them. Indexing large strings significantly increases mysql memory usage, and perform badly in comparison to ints.

If you don't want to use auto_increment, we can simply convert a timestamp to hexadecimal like dechex(time());. and result on something like media/{BREAD}/589c6c66.jpg (because storage doesn't sounds good :D)

I'm updating the main post.

[marktopper]

@Frondor: It might take some longer time for the v0.11 release. But I prefer the release to have some good improvements.

Nice, let me know if you need help with the Vue.js component for the Mdeia Manager or the API Controllers. And the API controllers would indeed be quite a nice improvement, then we could start moving things over to use AJAX requests instead of reloading the page every time something is done.

I like the idea about 'media/{BREAD}/589c6c66.jpg'. I can look into this if you aren't got time?

[Frondor]

@marktopper please, go ahead. I'll create a codepen with the vue component so we can review it here.

Regarding to the AJAX support for bread, we can always make use of request->expetsJson() to handle responses. But regarding to the issue with select_dropdown querying all() records of relationships, probably we may need to address the separation of input types as modules first and create a relationship-field-type module/package so all the logic goes there.

[marktopper]

Good idea @Frondor.

[marktopper]

@Frondor: I have been working on the new locations for BREAD files with their new filename.

However, I came to a problem with this, when uploading multiple files are uploaded at the sae page. Since the filename is made from the current time.

Any ideas how to solve this?

[marktopper]

I have decided that the following things should be merged into v0.11 before its released.

:construction:

1. Remove resources/views/users views so it makes use of default BREAD views, and properly configure the data rows to make use of Input details JSON api.
2. The new way to name BREAD files media/{BREAD}/589c6c66.jpg.

I believe the rest of them can be merged later without issues. Thoughts?

[marktopper]

Think on something to allow administrator changing loading icon.

This is already possible in settings.

[Frondor]

However, I came to a problem with this, when uploading multiple files are uploaded at the same page. Since the filename is made from the current time.

Sorry but I can't replicate the case in my mind... The problem here is that every file has the same name?

I have decided that the following things should be merged into v0.11 before its released. :construction:

1. Remove resources/views/users views so it makes use of default BREAD views, and properly configure the data rows to make use of Input details JSON api.
2. The new way to name BREAD files media/{BREAD}/589c6c66.jpg.

I agree. Do I need to edit my PR or something?

Also, I've been seeing some PRs lately, and I think that we need to separate the each input fields into its own view and controller as soon as possible. The code is getting really messy and hard to maintain (every time I try to understand a PR I struggle trying to figuring out each line).

For example #715 actually can break the whole parsing of relationships (linked anchors), and VoyagerBreadController is now huge. Shouldn't we consider addressing this at master as well?

[marktopper]

However, I came to a problem with this, when uploading multiple files are uploaded at the same page. Since the filename is made from the current time.

Sorry but I can't replicate the case in my mind... The problem here is that every file has the same name?

Uploading multiple files at the exact same time (lets say time() = 1486910263) it generates the same filename (in this case 58a07337.jpg).

I agree. Do I need to edit my PR or something?

Your pull request is fine and may include additional stuff. I however just focus on the things that must be merged before the release of v0.10.

Also, I've been seeing some PRs lately, and I think that we need to separate the each input fields into its own view and controller as soon as possible. The code is getting really messy and hard to maintain (every time I try to understand a PR I struggle trying to figuring out each line).

In v0.11 the types have already been separated into it own views. See https://github.com/the-control-group/voyager/tree/release/v0.11/resources/views/formfields

[Frondor]

Uploading multiple files at the exact same time (lets say time() = 1486910263) it generates the same filename (in this case 58a07337.jpg)

What about dechex(time() + $fileIndex) where $fileIndex should be the index in the incoming files' array. Or even dechex(time()) + $fileIndex

In v0.11 the types have already been separated into it own views. See https://github.com/the-control-group/voyager/tree/release/v0.11/resources/views/formfields

Yes, yes, but the logic is still in the view. I'm suggesting moving that logic to its own field controller and then using something like Voyager::formField('select_dropdown') which uses the controller and calls to the proper view. Probably something like resources/views/formfields/Controllers/CheckboxController.php ?

[marktopper]

Will work it out with the filenames and send a pull request and ping you.

How about the form fields are separated into each class and view and then it can be added to Voyager this way (will also make it able to add new types in extensions)

Add type

Voyager::addType('date', DateHandler::class);

Handler class

class DateHandler extends Handler
{
    protected $view = 'voyager::formfields.date';

    public function getViewArguments()
    {
        return [
            // an array of arguments that will be sent to the view?
        ];
    }
}

Just an idea.

[Frondor]

Just updated the main post and marked urgent things with a :warning:

[marktopper]

Removing this from milestone v0.11 since all v0.11 changes are covered in #718.

[ctf0]

• can we make the json gutter thinner ?
• what happend to add timestamp columns btn under db ?
• show the error msg in a div & persist it until the user clicks the x/close btn.

also the img folder under assets is only called inside publishable/assets/css/style.css but without any impact whether its avail or not, so what is it for ?

[ctf0]

• for image hashing, we can try https://github.com/jenssegers/imagehash which can give us another option to check if the newly uploaded image is already in the storage, and if so we can give the user the option to discard or to continu.

[arrilot]

Hi, do you have any news according bread table filters?

[ibourgeois]

WIP: but what do you think so far? This is a reusable modal in a partial that can be injected anywhere, including as a tinymce toolbar addition

[fletch3555]

@marktopper, any idea what's left here? Can this be closed?

@iBourgeois, that looks great! It would be worth tracking that in it's own issue/pull request.