Re: [pve-devel] [PATCH widget-toolkit] fix #5831: ui: right-align s.m.a.r.t numerical table data

[Dominik Csapak <d.csapak@proxmox.com>]

one comment inline

On 11/11/24 10:40, Timothy Nicholson wrote:
> This patch right-aligns the numerical values of the table displaying
> the S.M.A.R.T values in the Web UI.
> 
> Signed-off-by: Timothy Nicholson <t.nicholson@proxmox.com>
> ---
> 
> For better readability, numerical values in tables should be
> right-aligned, not like previously (left-aligned).
> https://bugzilla.proxmox.com/show_bug.cgi?id=5831
> 
>   src/window/DiskSmart.js | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/src/window/DiskSmart.js b/src/window/DiskSmart.js
> index 9096fec..363ae28 100644
> --- a/src/window/DiskSmart.js
> +++ b/src/window/DiskSmart.js
> @@ -29,6 +29,7 @@ Ext.define('Proxmox.window.DiskSmart', {
>   		    text: 'ID',
>   		    dataIndex: 'id',
>   		    width: 50,
> +		    align: 'right',
>   		},
>   		{
>   		    text: gettext('Attribute'),
> @@ -40,21 +41,25 @@ Ext.define('Proxmox.window.DiskSmart', {
>   		    text: gettext('Value'),
>   		    dataIndex: 'real-value',
>   		    renderer: Ext.String.htmlEncode,
> +		    align: 'right',)

this (real-value) is the raw value from smartctl, and this is not a "numerical" value,
but more or less a freeform field. (e.g. i have some here with: '35 (Min/Max 17/35)')

So I'd argue that this should not be right aligned

looking at smartctl output, only ID is right aligned
and normalized/threshold/worst are 0 padded
(so those are fine to right align)

>   		},
>   		{
>   		    text: gettext('Normalized'),
>   		    dataIndex: 'real-normalized',
>   		    width: 60,
> +		    align: 'right',
>   		},
>   		{
>   		    text: gettext('Threshold'),
>   		    dataIndex: 'threshold',
>   		    width: 60,
> +		    align: 'right',
>   		},
>   		{
>   		    text: gettext('Worst'),
>   		    dataIndex: 'worst',
>   		    width: 60,
> +		    align: 'right',
>   		},
>   		{
>   		    text: gettext('Flags'),



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