[pve-devel] [PATCH manager 4/5] Add icon for audio device

[Thomas Lamprecht]

On 7/16/19 5:12 PM, Aaron Lauterer wrote:
> On 7/16/19 3:43 PM, Aaron Lauterer wrote:
>> On 7/16/19 1:48 PM, Thomas Lamprecht wrote:
>>> On 7/15/19 3:34 PM, Aaron Lauterer wrote:
>>>>   www/images/icon-audio.svg | 2 ++
>>> couldn't we just use existing font awesome icons, for example, "fa-volume-up"[0]
>>> or eventual candidates could also, be "fa-headphones", "fa-music".
>>
>> This would indeed be a sensible thing to do. I looked into it and if I change the row configuration in `HardwareView.js` from
>> "tdCls: 'pve-itype-icon-audio'," to "iconCls: 'volume-up'," it does show the icon using font-awesome nicely.
>>
>> The only downside is that the remove dialog shows the icon in the text:
>>  > Are you sure you want to remove entry '<icon> Audio Device'
>>
>> This is because the remove dialog is calling the same `renderKey` function. My guess is that this has not been noticed yet as the `iconCls` property is used only by items that cannot be removed like BIOS or Memory. All the other hardware items use separate icons that are then handled with the `tdCls` property.
>>
>> How should I proceed as we most likely do not want to have the icon in the middle of the remove dialog?
>>
> 
> One possible way would be to set a property in the `metaData` object that is passed to the `renderKey` function to trigger a return value without the icon.
> 

I independently saw that the cloud icon for cloudinit was provided as SVG,
and changed that, I come up with following solution and then checked your
proposed. The fact that they are very similar in principle made me confident
enough to straight apply it:

----8<----
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 20f54cbe..94324323 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -33,7 +33,13 @@ Ext.define('PVE.qemu.HardwareView', {
            icon = "<i class='pve-grid-fa fa fa-fw fa-" + iconCls + "'></i>";
            metaData.tdCls += " pve-itype-fa";
        }
-       return icon + txt;
+
+       // only return icons in grid but not remove dialog
+       if (rowIndex !== undefined) {
+           return icon + txt;
+       } else {
+           return txt;
+       }
     },
 
     initComponent : function() {
--

I like above a bit more as it needs no additional parameter and the icon
really only makes sense in the grid view (where rowIndex is always defined).

Anyway, it should work now, thanks for the notice and highlighting the issue
with already having a solution pronto!

> 
> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
> index 09d5d41f..8d58fd3d 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -33,6 +33,9 @@ Ext.define('PVE.qemu.HardwareView', {
>             icon = "<i class='pve-grid-fa fa fa-fw fa-" + iconCls + "'></i>";
>             metaData.tdCls += " pve-itype-fa";
>         }
> +       if (metaData.pveIcon === false) {
> +           return txt;
> +       }
>         return icon + txt;
>      },
> 
> @@ -438,7 +439,7 @@ Ext.define('PVE.qemu.HardwareView', {
> 
>                 var entry = rec.data.key;
>                 var msg = Ext.String.format(warn, "'"
> -                   + me.renderKey(entry, {}, rec) + "'");
> +                   + me.renderKey(entry, {'pveIcon': false}, rec) + "'");
> 
>                 if (entry.match(/^unused\d+$/)) {
>                     msg += " " + gettext('This will permanently erase all data.');
>