Re: [pve-devel] [PATCH v7 pve-manager 12/15] sdn: display pending values

[alexandre derumier <aderumier@odiso.com>]

On 25/11/2020 17:17, Thomas Lamprecht wrote:
> On 24.11.20 14:52, Alexandre Derumier wrote:
>> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
>> ---
>>   www/manager6/Utils.js              | 29 ++++++++++++++++++
>>   www/manager6/sdn/ControllerView.js | 39 +++++++++++++++++++++---
>>   www/manager6/sdn/SubnetView.js     | 49 +++++++++++++++++++++++++++---
>>   www/manager6/sdn/VnetView.js       | 31 +++++++++++++++++--
>>   www/manager6/sdn/ZoneView.js       | 47 +++++++++++++++++++++++++---
>>   5 files changed, 181 insertions(+), 14 deletions(-)
>>
>> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
>> index 5440b972..257af3fd 100644
>> --- a/www/manager6/Utils.js
>> +++ b/www/manager6/Utils.js
>> @@ -175,6 +175,35 @@ Ext.define('PVE.Utils', { utilities: {
>>   	'HEALTH_ERR':'critical'
>>       },
>>   
>> +    render_sdn_pending: function(rec,value,key, index) {
>> +        if (rec.data.state === 'deleted') {
>> +            if (value === undefined) {
>> +                return '';
>> +            } else {
>> +                return '<div style="text-decoration: line-through;">'+ value +'</div>';
>> +            }
>> +
>> +        } else if (rec.data.state === 'new') {
>> +            if(index === undefined) {
>> +                value = rec.data.pending[key];
>> +            }
>> +            if (value === undefined || value === null) {
>> +                value = '';
>> +            }
>> +            return '<div style="color:green">' + value + '</div>';
>> +        } else if (rec.data.state === 'changed') {
>> +            if (value === undefined || value === null) {
>> +                value = '<br>';
>> +            }
>> +            if (rec.data.pending[key] === undefined || rec.data.pending[key] === null) {
>> +                rec.data.pending[key] = value;
>> +            }
>> +           return '<div style="text-decoration: line-through;">'+ value +'</div>' + '<div style="color:orange">' + rec.data.pending[key] + '</div>';
>> +        } else {
>> +            return value;
>> +        }
>> +    },
>> +
>>       render_ceph_health: function(healthObj) {
>>   	var state = {
>>   	    iconCls: PVE.Utils.get_health_icon(),
>
> This feels really subtle as one initially has no idea why all is green, for
> example. Visually and contrast wise it looks also a bit weird, IMO.
>
> The strike-through for pending changes looks OK, I'd avoid the red though.
>
> Further, I'd avoid to use style on the whole row to mark something as
> pending-new or pending-change.
>
> How about using a dedicated Pending column, which shows the state in text
> form. You could checkout the render_backup_encryption helper[0] for how one
> can produce a combination of text, icons and even tooltips.
> (albeit the icon choice would be a bit hard here, so just text is fine too
> IMO).
>
> [0]: https://git.proxmox.com/?p=pve-manager.git;a=blob;f=www/manager6/Utils.js;h=6e6498a21919ca4dd2cb65ab02a1a1730d4c2d36;hb=HEAD#l224
>
>
Hi Thomas,

sorry it seem that I have some email client problem, and didn't see your 
message.

I'll rework it,with a pending state column.

I was thinking to display the pending changes with a tooltip on hover of 
this state.

(instead displaying a raw json)

I will look at render_backup_encryption too.

I'll try to send a new version this week.

thanks for the review !

Alexandre