[pve-devel] [PATCH v2 manager] ui: backup: replication: replace non-clickable checkbox with icons

[Thomas Lamprecht]

Am 23/01/2023 um 14:17 schrieb Lukas Wagner:
> From a usability view, having a checkbox that is not clickable is pretty
> misleading, especially if the visual style is exactly the same as in
> other places in the UI where the checkbox is functional.
> 
> Signed-off-by: Lukas Wagner <l.wagner at proxmox.com>
> ---
>  www/manager6/dc/Backup.js        | 11 ++++++-----
>  www/manager6/grid/Replication.js | 11 +++++++----
>  2 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
> index 9d305984..8a306330 100644
> --- a/www/manager6/dc/Backup.js
> +++ b/www/manager6/dc/Backup.js
> @@ -784,12 +784,13 @@ Ext.define('PVE.dc.BackupView', {
>  		{
>  		    header: gettext('Enabled'),
>  		    width: 80,
> -		    dataIndex: 'enabled',
> -		    xtype: 'checkcolumn',
> +		    dataindex: 'enabled',

you break setting dataIndex by writing it all lowercase ?!

> +		    align: 'center',
> +		    renderer: function(enabled, cell, record) {
> +			return Proxmox.Utils.render_enabled_icon(record.data.enabled);
> +		    },


with data index fixed, this could be:


renderer: Proxmox.Utils.renderEnabledIcon,

Or if you really want to avoid passing the remaining params to the renderer:

renderer: enabled => Proxmox.Utils.renderEnabledIcon(enabled),

but normally that's really not a problem for renders.

>  		    sortable: true,
> -		    disabled: true,
> -		    disabledCls: 'x-item-enabled',
> -		    stopSelection: false,
> +
>  		},
>  		{
>  		    header: gettext('ID'),
> diff --git a/www/manager6/grid/Replication.js b/www/manager6/grid/Replication.js
> index b17288b9..261c2755 100644
> --- a/www/manager6/grid/Replication.js
> +++ b/www/manager6/grid/Replication.js
> @@ -279,11 +279,14 @@ Ext.define('PVE.grid.ReplicaView', {
>  
>  	me.columns = [
>  	    {
> -		text: gettext('Enabled'),
> -		dataIndex: 'enabled',
> -		xtype: 'checkcolumn',
> +		header: gettext('Enabled'),
> +		width: 80,
> +		dataindex: 'enabled',

same here w.r.t. breaking dataIndex

> +		align: 'center',
> +		renderer: function(enabled, cell, record) {
> +		    return Proxmox.Utils.render_enabled_icon(record.data.enabled);
> +		},

same here w.r.t. shorter direct renderer

>  		sortable: true,
> -		disabled: true,
>  	    },
>  	    {
>  		text: 'ID',