all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Aaron Lauterer <a.lauterer@proxmox.com>
Subject: Re: [pve-devel] [PATCH manager 1/2] ui: ceph: make the warning detail button stand out more
Date: Thu, 2 Mar 2023 14:15:10 +0100	[thread overview]
Message-ID: <a6785294-25ea-37fe-1c90-bd0396c4cd22@proxmox.com> (raw)
In-Reply-To: <20230222093655.2645787-2-a.lauterer@proxmox.com>

one nit inline:

On 2/22/23 10:36, Aaron Lauterer wrote:
> The button for more details is barely noticable as something one can
> click on. By making it more obvious that it is a button, users will
> hopefully notice it easier.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> 
> While moving code around I also changed:
> - var -> let
> - string concat to template literals
> 
>   www/manager6/ceph/Status.js | 56 +++++++++++++++++++------------------
>   1 file changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/www/manager6/ceph/Status.js b/www/manager6/ceph/Status.js
> index bdcf3f1b..45583f51 100644
> --- a/www/manager6/ceph/Status.js
> +++ b/www/manager6/ceph/Status.js
> @@ -101,37 +101,39 @@ Ext.define('PVE.node.CephStatus', {
>   			    flex: 1,
>   			},
>   			{
> -			    xtype: 'actioncolumn',
> +			    xtype: 'widgetcolumn',
>   			    width: 40,
>   			    align: 'center',
>   			    tooltip: gettext('Detail'),
> -			    items: [
> -				{
> -				    iconCls: 'x-fa fa-info-circle',
> -				    handler: function(grid, rowindex, colindex, item, e, record) {
> -					var win = Ext.create('Ext.window.Window', {
> -					    title: gettext('Detail'),
> -					    resizable: true,
> -					    modal: true,
> -					    width: 650,
> -					    height: 400,
> -					    layout: {
> -						type: 'fit',
> -					    },
> -					    items: [{
> -						scrollable: true,
> -						padding: 10,
> -						xtype: 'box',
> -						html: [
> -						    '<span>' + Ext.htmlEncode(record.data.summary) + '</span>',
> -						    '<pre>' + Ext.htmlEncode(record.data.detail) + '</pre>',
> -						],
> -					    }],
> -					});
> -					win.show();
> -				    },
> +			    widget: {
> +				xtype: 'button',
> +				baseCls: 'x-btn',
> +				userCls: 'x-btn-default-toolbar-small',
> +				iconCls: 'fa fa-fw fa-info-circle x-btn-icon-el-default-toolbar-small',
> +				handler: function() {
> +				    let record = this.getWidgetRecord();
> +				    let win = Ext.create('Ext.window.Window', {
> +					title: gettext('Detail'),
> +					resizable: true,
> +					modal: true,
> +					width: 650,
> +					height: 400,
> +					layout: {
> +					    type: 'fit',
> +					},
> +					items: [{
> +					    scrollable: true,
> +					    padding: 10,
> +					    xtype: 'box',
> +					    html: [
> +						`<span>${Ext.htmlEncode(record.data.summary)}</span>`,
> +						`<pre>${Ext.htmlEncode(record.data.detail)}</pre>`,
> +					    ],
> +					}],
> +				    });
> +				    win.show();

when touching this, you could add 'autoShow: true' to the properties,
then we don't have to save the window in a variable at all (and don't call .show())

not a reason to send a v2 though

>   				},
> -			    ],
> +			    },
>   			},
>   		    ],
>   		},





  reply	other threads:[~2023-03-02 13:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-22  9:36 [pve-devel] [PATCH manager 0/2] ui: ceph: improve discoverability of warning details Aaron Lauterer
2023-02-22  9:36 ` [pve-devel] [PATCH manager 1/2] ui: ceph: make the warning detail button stand out more Aaron Lauterer
2023-03-02 13:15   ` Dominik Csapak [this message]
2023-02-22  9:36 ` [pve-devel] [PATCH manager 2/2] ui: ceph status: add tooltip with details to warnings Aaron Lauterer
2023-02-22  9:39   ` [pve-devel] [PATCH manager follow-up " Aaron Lauterer
2023-03-02 13:15     ` Dominik Csapak
2023-03-02 13:22       ` Aaron Lauterer

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=a6785294-25ea-37fe-1c90-bd0396c4cd22@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=a.lauterer@proxmox.com \
    --cc=pve-devel@lists.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal