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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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