From: Dominik Csapak <d.csapak@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Timothy Nicholson <t.nicholson@proxmox.com>
Subject: Re: [pve-devel] [PATCH widget-toolkit] fix #5831: ui: right-align s.m.a.r.t numerical table data
Date: Mon, 11 Nov 2024 14:19:04 +0100 [thread overview]
Message-ID: <456382dd-aece-4686-9d36-50164100ce89@proxmox.com> (raw)
In-Reply-To: <20241111094050.22619-1-t.nicholson@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
prev parent reply other threads:[~2024-11-11 13:19 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-11 9:40 Timothy Nicholson
2024-11-11 13:19 ` Dominik Csapak [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=456382dd-aece-4686-9d36-50164100ce89@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=t.nicholson@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox