Reprioritize icons and images for GTLs, prefer images first

[skateman]

When both a fonticon and a fileicon is defined for an item, the preferred one was the fonticon, except of some special cases when there's a picture attribute available. This was unnecessarily complicated and as all the unnecessary PNG files and their references have been removed, we can simplify this a little.

So I'm proposing the same preference as we have for treeviews: fileicon over fonticon and also dropping the picture attribute.

UI components PR: ManageIQ/ui-components#308

@miq-bot add_reviewer @epwinchell
@miq-bot add_reviewer @karelhala
@miq-bot add_reviewer @martinpovolny
@miq-bot add_labels gaprindashvili/no, GTLs, graphics

[Fryguy]

How will Generic Objects work?

[skateman]

@Fryguy same as before:

• if they have a custom image set -> it will be displayed
• if they don't have a custom image -> the fonticon will be displayed

The only difference is that in GTLs we will use 2 parameters instead of 3, same as in trees.

(but yes, I had it swapped in the description ... sorry)

[Fryguy]

I mixed up "picture" and "custom image", which is why I was confused....thanks!

[himdel]

Right now, there are only 4 decorators which can return a picture in fileicon - GenericObjectDefinition, Service, ServiceTemplate and GenericObject.

But there are 6 classes using picture - the remaining two being ServiceTemplateProvisionRequest and OrchestrationTemplate.

@skateman you sure we're not missing decorators for those two?
Or is the intent to remove picture support from those?

[skateman]

@himdel the answer is very simple, if we don't have a decorator, we couldn't display those items in the UI using GTLs. If they have a different way of rendering, then probably the picture attribute is being called directly. So you should not worry about those until I unify them...

[himdel]

Um... unless that :picture => ActionController::Base.helpers.image_path(picture.to_s), in view_to_hash was dead code, that can't really be true.

Please check :)

EDIT: ... aah, never mind, seeing where that picture comes from there.

Still... this didn't use to be this way, the relation was a bit more direct. So maybe this PR doesn't break it, and some earlier one did. Either way, it's broken now.

[skateman]

@epwinchell you also need the UI components PR to have this working...

[skateman]

@himdel that picture variable you're referring to is being generated by item.try(:picture) ? decorated.try(:fileicon) : nil 😉 so nothing is broken

[himdel]

OK, here's my take on this:

The original GTL never used decorators, and always showed picture when available.
There are 2 classes which support picture and don't have the appropriate decorator now.

It feels like we're just arguing over when we broke it.
Can you perhaps fix it instead? :)

[skateman]

I don't see any logic in the UI that can upload pictures for the OrchestrationTemplate, but of course we can add it, but please not in this PR. With the ServiceTemplateProvisionRequest, those are only shown on the Services -> Requests screen where the request status is definitely not a custom uploaded picture. The listicon is currently missing from these pages, but that's a different bug unrelated to the picture. So IMO we don't need to fix anything here.

[miq-bot]

This pull request is not mergeable. Please rebase and repush.

[epwinchell]

Tested. Looks good.

[miq-bot]

Checked commits skateman/manageiq-ui-classic@d5e2eed~...cfa947d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 👍