Re: [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons

[Fiona Ebner <f.ebner@proxmox.com>]

Am 04.08.25 um 2:10 PM schrieb Shannon Sterz:
> On Mon Aug 4, 2025 at 1:48 PM CEST, Fiona Ebner wrote:
>> Am 04.08.25 um 12:50 PM schrieb Shannon Sterz:
>>> previously we used custom png rendered icons for guests and storages
>>> in the add menu of a pool, as well as for mountpoints for containers.
>>> these appeared blurry on high resolution displays and since they were
>>> just the same as the font-awesome icons anyway, use those directly.
>>> the ui already loads font-awesome regardless, so there are no
>>> down-sides here.
>>>
>>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>>> ---
>>>  www/manager6/grid/PoolMembers.js | 4 ++--
>>>  www/manager6/lxc/Resources.js    | 2 +-
>>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/www/manager6/grid/PoolMembers.js b/www/manager6/grid/PoolMembers.js
>>> index f025a1380..83046ee0e 100644
>>> --- a/www/manager6/grid/PoolMembers.js
>>> +++ b/www/manager6/grid/PoolMembers.js
>>> @@ -243,7 +243,7 @@ Ext.define('PVE.grid.PoolMembers', {
>>>                          items: [
>>>                              {
>>>                                  text: gettext('Virtual Machine'),
>>> -                                iconCls: 'pve-itype-icon-qemu',
>>> +                                iconCls: 'fa fa-desktop',
>>>                                  handler: function () {
>>>                                      var win = Ext.create('PVE.pool.AddVM', { pool: me.pool });
>>>                                      win.on('destroy', reload);
>>> @@ -252,7 +252,7 @@ Ext.define('PVE.grid.PoolMembers', {
>>>                              },
>>>                              {
>>>                                  text: gettext('Storage'),
>>> -                                iconCls: 'pve-itype-icon-storage',
>>> +                                iconCls: 'fa fa-hdd-o',
>>>                                  handler: function () {
>>>                                      var win = Ext.create('PVE.pool.AddStorage', { pool: me.pool });
>>>                                      win.on('destroy', reload);
>>> diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
>>> index 9f6df3a58..3defb8003 100644
>>> --- a/www/manager6/lxc/Resources.js
>>> +++ b/www/manager6/lxc/Resources.js
>>> @@ -128,7 +128,7 @@ Ext.define('PVE.lxc.RessourceView', {
>>>              rows[confid] = {
>>>                  group: group,
>>>                  order: i,
>>> -                tdCls: 'pve-itype-icon-storage',
>>> +                iconCls: 'hdd-o',
>>
>> This changes from 'tdCls' to 'iconCls' and does not use 'fa fa-hdd-o'
>> like above. Is that intentional?
> 
> yes, this is to keep it in-line with other iconCls definitions here. see
> for example the definition for rootfs (which uses the exact same icon).
> 
> note that these get rendered differently, even though both are called
> `iconCls`. the one in the resources table outputs a proper `<i>` element
> with the appropriate classes set already (here Ext.js seems to expand
> `hdd-o` to `fa fa-hdd-o` automatically). the other just adds the classes
> directly to the menu item in the pool's add menu. so it needs the `fa
> fa-desktop` and `fa fa-hdd-o` classes, in order for the icons to be set
> via pseudo elements properly.

Thank you for the detailed explanation! Would be great to have in the
commit message as context for people like me, who are not as familiar
with those internals and as a reminder for people who are :)

> 
>>>                  editor: mpeditor,
>>>                  header: header,
>>>                  renderer: Ext.htmlEncode,
>>
>> Can we get rid of the related CCS classes and PNG images after this change?
> 
> i grepped over our repos and it seems that these are the only places
> that make use of these classes. so yes removing them now *should* be
> safe. let me know if you want a follow up for that.
> 
> although, if we go down this path i'd suggest we remove all
> `pve-itype-icon-*` classes with font-awesome equivalents. which would be
> a bit more extensive and touch all our current ext.js front-end repos.
> 
> however, this patch would still be the same, the follow-up would just be
> additional clean-up.

Sure, doesn't have to be the same patch and is not that urgent. There's
just the risk that we forget otherwise. A larger series cleaning
all/most of them up would be nice, but again, nothing that urgent.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel