Added toolbar editing

[techanon]

Adapted the navigation editor and put some restrictions on the JS for intuitive (and more accurate) usage. If the user is allowed to edit the toolbar (via ACL), an edit link will appear (eithernext to the dropdown arrow or in the tools list dropdown , depending on whether there is a custom tools list available or not, respectively).

Did a bit of code refactoring and cleanup as well.

[jbroadway]

I went ahead and made one change that's included here already, which was adding the setting to the global [Paths]. Wasn't thinking about that breaking the automatic merge, so sorry about causing something extra for you to fix.

A few comments, to keep them all together:

1) The admin/toolbar ACL setting should be added to apps/admin/conf/acl.php so it shows up in the role options, something like this:

admin/toolbar = "Modify the admin toolbar"

2) The "Edit" link would look better in either the top or bottom right corner of the menu itself, instead of next to the arrow. Should also be called something more descriptive like "Edit Toolbar".

3) The sections on the left don't need the [ ] braces around them in the editor.

4) Can we rename admin\API to admin\Toolbar\API and the REST endpoint /admin/api/toolbar?

5) The style of the add dialogs doesn't match Elefant's overall style. You could try using the admin/util/modal helper to make them fit better. They could also use a cancel option.

6) I would remove 'Click here to' from the start of the add section and add resource links.

7) I see you got rid of the Extras section where new apps will appear automatically. I'd like to keep that by default, so newly-installed apps are visible without having to add them manually.

I think those are all the suggestions I have for now. Thanks, this is pretty cool!

[techanon]

1) Could have sworn I put that in at first... oh well. 2) Though I still think that it would work better being next to the arrow as 'Edit Toolbar'. 3) I know they don't need it, I just put them in for aesthetics. I can remove them if you still want me to. 4) Done. 5) Heh, forgot about that helper. 6) Done. 7) Made a 'special' case option in the editor for the extras section(s) (took a bit of tape and glue, but I think it turned out decent).

[techanon]

Now that I've worked out a number of bugs, here's a demo video: https://dl.dropboxusercontent.com/u/18208651/ElefantCMS-Toolbar_Editor.mp4

[AxorB]

Very convenient, thanks!

[evan70]

Pretty cool :)

[jbroadway]

It's really coming together now! A few suggestions to improve user-friendliness:

• I do think removing the [ ] from categories would be friendlier, since those are just INI formatting conventions a user doesn't need to see.
• In the Add Category dialog, "Display Text" could be "Category Name" which is a little less technical (I'm imagining total non-techies using this too :))
• "Toolbar Tree" could be renamed "Toolbar Menu", and "Unused Resources" could be "Available Resources".

One thing I'm thinking is that adding resources is something that requires knowledge of how apps and handlers work. What if the available resources were auto-discovered based on each app's configuration? The old menu did that by reading the [Admin] handler setting from each app's conf/config.php, but the issue there is that each app can only have one handler included in the toolbar.

I recently added an Appconf::options() method that makes it easier to move settings like that to custom conf files, so for example we could move that setting to a conf/tools.php file in each app that publishes a list of available handlers that can be included in the toolbar. Something like this:

; <?php /*

admin/pages = "Web Pages"
admin/versions = "Versions"

; */ ?>

We can fetch all of these via:

$available_handlers = Appconf::options ('tools');

Then the onus would be on the app developer to specify which handlers can be included in the toolbar, but there wouldn't be the limitation of one-per-app that currently exists. That would eliminate the need for the "Add Resource" dialog in the toolbar editor too, I think.

What do you think?

[techanon]

Point one shall be done.

Ah yea, I ran across Appconf::options recently. That would definitely be preferable for auto-loading and have it default to the [Admin] handler if tools.php doesn't exist. I'll get on that.

[techanon]

Had to duplicate Appconf::options functionality in the method itself so I would know what app directories have been searched. Made a few CSS improvements. Switched out [ ] for BOLD type. Added what I hope is proper ACL checks (I've tested it, but I've probably missed something...). Added a couple toolbar states depending on the available permissions and what is in the tools.php.

[techanon]

Ok... I think that's all of the commits for the needed changes...

[jbroadway]

I'm working on putting this through its paces, and I think I found a small bug but I'm not 100% sure if my fix covers it. Basically, I created an admin role that doesn't have permission to edit the toolbar, and when I log in with an account of that role I don't see the admin menu. I see it returns the right content from /admin/head/links but it's not showing the drop down, while the other links show correctly.

If I comment out the if (!res.is_apps) { condition on line 192 of top-bar.js that seems to fix it, but I wanted to clarify what that variable actually means now and if we can't just take that condition out completely (along with the whole else section that renders the menu the old way, which can probably be safely retired at this point).

Let me know if you think I should just take that condition out, or if you think there will be any side-effects to it, like needing extra permission checking to compensate.

[jbroadway]

Actually, just changing line 48 of apps/admin/handlers/head/links.php to this seems to fix it too:

'is_apps' => $is_apps,

What's the need for the extra || (count($tools) && !$editable) in there?

[jbroadway]

For now I've changed the is_apps line in apps/admin/handlers/head/links.php and I'm going to merge the changes. Permissions seem fine with it. We can always tweak it more as needed once it's merged.

[jbroadway]

Btw, thanks for the patience with me getting around to testing everything. This is an awesome new feature! :)

[jbroadway]

One issue I just found (might be related to the is_apps change I made): Apps with install or upgrade scripts to be run still point to the regular handler, even though they show the visual "(click to install)" changes.

[techanon]

Let's see if I can remember the logic...

In links.html, is_apps determines what the HTML is to be returned. In top-bar.js, is_apps determines whether or not to inject the new format's dropdown arrow at the top. The || (count($tools) && !$editable) (which actually needs to be || (count($tools) === 0 && !$editable)) is for forcing the js to use the old method since that doesn't inject the top arrow. The reason is to "hide" the toolbar from those without proper permissions for any available tool options (including any from the autofill) and lack the permission to edit the toolbar. Without any tool access or editing permission, the toolbar would just be an empty, useless toolbar, so I opted to make it where the arrow doesn't show itself under that condition. (I can explain further if needed).

In relation to the "click to install" issue, I forgot to add the install handler into the version check logic for the tools autofill. This issue also brought up a couple more bugs that needed squashing, so that's good.