public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons
@ 2025-08-04 10:50 Shannon Sterz
  2025-08-04 11:20 ` Lukas Wagner
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Shannon Sterz @ 2025-08-04 10:50 UTC (permalink / raw)
  To: pve-devel

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',
                 editor: mpeditor,
                 header: header,
                 renderer: Ext.htmlEncode,
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons
  2025-08-04 10:50 [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons Shannon Sterz
@ 2025-08-04 11:20 ` Lukas Wagner
  2025-08-04 11:48 ` Fiona Ebner
  2025-08-26 15:00 ` Shannon Sterz
  2 siblings, 0 replies; 6+ messages in thread
From: Lukas Wagner @ 2025-08-04 11:20 UTC (permalink / raw)
  To: Proxmox VE development discussion, Shannon Sterz

On Mon Aug 4, 2025 at 12:50 PM CEST, Shannon Sterz wrote:
> 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.
>

LGTM!

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>

Verified that these three icons are indeed replaced with the
high-resolution version from FA:

Tested-by: Lukas Wagner <l.wagner@proxmox.com>


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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons
  2025-08-04 10:50 [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons Shannon Sterz
  2025-08-04 11:20 ` Lukas Wagner
@ 2025-08-04 11:48 ` Fiona Ebner
  2025-08-04 12:10   ` Shannon Sterz
  2025-08-26 15:00 ` Shannon Sterz
  2 siblings, 1 reply; 6+ messages in thread
From: Fiona Ebner @ 2025-08-04 11:48 UTC (permalink / raw)
  To: Proxmox VE development discussion, Shannon Sterz

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?

>                  editor: mpeditor,
>                  header: header,
>                  renderer: Ext.htmlEncode,

Can we get rid of the related CCS classes and PNG images after this change?


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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons
  2025-08-04 11:48 ` Fiona Ebner
@ 2025-08-04 12:10   ` Shannon Sterz
  2025-08-04 12:28     ` Fiona Ebner
  0 siblings, 1 reply; 6+ messages in thread
From: Shannon Sterz @ 2025-08-04 12:10 UTC (permalink / raw)
  To: Fiona Ebner, Proxmox VE development discussion

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.

>>                  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.


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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons
  2025-08-04 12:10   ` Shannon Sterz
@ 2025-08-04 12:28     ` Fiona Ebner
  0 siblings, 0 replies; 6+ messages in thread
From: Fiona Ebner @ 2025-08-04 12:28 UTC (permalink / raw)
  To: Shannon Sterz, Proxmox VE development discussion

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons
  2025-08-04 10:50 [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons Shannon Sterz
  2025-08-04 11:20 ` Lukas Wagner
  2025-08-04 11:48 ` Fiona Ebner
@ 2025-08-26 15:00 ` Shannon Sterz
  2 siblings, 0 replies; 6+ messages in thread
From: Shannon Sterz @ 2025-08-26 15:00 UTC (permalink / raw)
  To: Shannon Sterz, pve-devel

superseded-by: https://lore.proxmox.com/all/20250826145836.302748-1-s.sterz@proxmox.com/T/#t

On Mon Aug 4, 2025 at 12:50 PM CEST, Shannon Sterz wrote:
> 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',
>                  editor: mpeditor,
>                  header: header,
>                  renderer: Ext.htmlEncode,



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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-08-26 15:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-04 10:50 [pve-devel] [PATCH manager] fix #6599: ui: use font-awesome instead of png icons Shannon Sterz
2025-08-04 11:20 ` Lukas Wagner
2025-08-04 11:48 ` Fiona Ebner
2025-08-04 12:10   ` Shannon Sterz
2025-08-04 12:28     ` Fiona Ebner
2025-08-26 15:00 ` Shannon Sterz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal