Revert action icon changes back to fa for web-actions templates

[loppear]

Since #28 only intended to make nav icons replaceable in templates, revert back to specifying the actual styles for icons to web-actions rather than needing to update web-actions templates to be aware of icon names and renaming.

I think this is the least change to get instance action buttons working again, but if we like the generic icon names -> template rewriting, the alternative is to make web-actions templates aware of these mappings.

[davidkellerman]

There's got to be a better way to do this.

What motivated me to make the original changes was wanting to re-style the admin pages. That involved swapping out page templates, and changing from FontAwesome to Simple Line icons.

I think the core of the problem here is that AdminController claims to be a controller module, but it's mostly view code. The Actions module from nxus-web perpetuates this – the action data structure contains display settings, but no attribute that identifies the action logically (e.g. "this is a save action").

I think the long-term direction should be for the controller modules to build an action tree that specifies what the actions are, and separate out the code that decorates the actions with display settings.

In retrospect, it was a mistake to try to hijack the icon property to become a logical action type.

As an alternative, suppose we keep what are essentially logical action types as a new action option, and revert the meaning of the icon option? (So, for example {action: 'add', icon: 'fa fa-plus'}.)

That would let old code continue unchanged, but allow new code to move the display settings into the view code (templates and partials, I think).

[loppear]

I agree that the intent of splitting view decisions (what icon css to use) from controller decisions (what actions and nav does this entail?) is the right path forward. Note that this PR is intended not to require you to change anything you've done with project nav, just narrows the earlier scope of this split to only be related to nav and saves the further refactor of actions for later.

Your logical action type property sounds like a fine enhancement to file with nxus-web/actions, if we stick with it.

[davidkellerman]

The 100mhl code in question is here: modules/theme/partials/admin-nav-menu.ejs (at the top of the file). I haven't looked at it in a while, but it looks like I was covering my bases – translating both the fa icons and the logical names to new icon names.

100mhl may be a lost cause, but I really don't want to propagate this sort of code to other projects.

I know what will happen to a "a fine enhancement to file." Unless there's something controversial to the action structure having a "type" property (whatever you want to call it), I'd like to have that available now.

[loppear]

If it's a good idea, we should implement it in nxus-web/actions, at a time when something is going to make use of it, that's all I mean by filing it over there. This is a bugfix PR, is it obvious that should contribute a new undocumented and unused extension of actions? If you affirm it again, I'll add it to move this forward, but I really just wanted to unbreak this today.

[loppear]

nxus/web#52

[davidkellerman]

Well, go ahead and commit it then. Thanks for putting together web#52. The "unused extension" is a bit of chicken-and-egg – I would like to clean up the admin page display, but maybe we just accept that the admin pages are ugly. As for "undocumented," we could document any changes, I suppose...

[ScottMaxson]

I'm OK with this reversion for now. Agree with discussion here about separation of view config from controller config, in general. Moving forward, could the icon-type just be CSS definitions?

[alexjreich]

👍