Resource icons in resource groups tree #16099
Conversation
… for specific resource
|
Unfortunately, the resource status is not displayed in the group tree (publishing, deleting and displaying in the menu).
|
There was a problem hiding this comment.
I've tested this and it fixes the missing icons issue. As @Ruslan-Aleev pointed out, the resource status is not shown. That's easy to add in here, but it occurred to me that a case can be made for not showing it; it potentially adds a bunch of visual clutter not necessarily needed for the function of this area (organizing the resources into groups).
Note that @theboxer's solution here is better than #16011, as the abstraction into a new function is more lean and Ruslan's PR adds a whole bunch of unneeded css classes used in the main resource tree for permissions. (The only operations available in the groups tree is to add or remove a resource from a group.)
I can go either way here. We could introduce a system setting and leave it up to users whether statuses are shown in resource groups.
| @@ -85,13 +85,13 @@ public function process() | |||
| $resources = $resourceGroup->getResources(); | |||
| /** @var modResource $resource */ | |||
| foreach ($resources as $resource) { | |||
| $icons = $resource->getIconClasses(); | |||
There was a problem hiding this comment.
If electing to add statuses:
| $icons = $resource->getIconClasses(); | |
| $icons = $resource->getIconClasses(); | |
| $cls = []; | |
| if (!$resource->get('published')) { | |
| $cls[] = 'unpublished'; | |
| } | |
| if ($resource->get('deleted')) { | |
| $cls[] = 'deleted'; | |
| } | |
| if ($resource->get('hidemenu')) { | |
| $cls[] = 'hidemenu'; | |
| } |
There was a problem hiding this comment.
Would be better to put this logic in the getIconClasses() method, no?
There was a problem hiding this comment.
Actually now that I think about it longer than 5 seconds, no, because cls is applied somewhere else than iconCls. So support for this suggestion.
| $list[] = [ | ||
| 'text' => $resource->get('pagetitle') . ' (' . $resource->get('id') . ')', | ||
| 'id' => 'n_' . $resource->get('id') . '_' . $resourceGroup->get('id'), | ||
| 'leaf' => true, | ||
| 'type' => modResource::class, | ||
| 'cls' => 'icon-' . $resource->get('class_key'), | ||
| 'iconCls' => 'icon-file', | ||
| 'iconCls' => implode(' ', $icons), |
There was a problem hiding this comment.
If adding statuses:
| 'iconCls' => implode(' ', $icons), | |
| 'iconCls' => implode(' ', $icons), | |
| 'cls' => implode(' ', $cls) |
|
|
||
| $contentType = $this->getOne('ContentType'); | ||
| if ($contentType && $contentType->get('icon')) { | ||
| $iconCls = [$contentType->get('icon')]; |
There was a problem hiding this comment.
Shouldn't this (and the same logic on line 1572) be $iconCls[] = $contentType->get('icon') or is the point that we can only have a single icon between the content type and template?
This does seem to be the same in the original code so happy to let it slide if it's just the way things used to be.
|
@theboxer - bump - wondering if you're still interested in this or want someone else to pick it up? |
Mark-H
added
blocked
What does it do?
Abstract logic for getting icon classes from
Resource\GetNodesprocessor to themodResource::getIconClasses.Use the new method in
Resource\GetNodesandResourceGroup\GetNodes.Lock icon is not part of this - as it's specific for resource tree and has no real benefit in resource group tree, same with classes around permissions.
Why is it needed?
Fixes the BUG that resource group's tree doesn't show resource icons. (100% viable for 3.0)
How to test
Create resource group, add resource, check if it has an icon :)
Related issue(s)/PR(s)
Different approach in #16011