From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 719498228 for ; Thu, 2 Mar 2023 14:15:14 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5B4053F45 for ; Thu, 2 Mar 2023 14:15:14 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 2 Mar 2023 14:15:13 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E89CC48D50 for ; Thu, 2 Mar 2023 14:15:12 +0100 (CET) Message-ID: <5bae3008-0d48-41be-59cd-a9553c466fc5@proxmox.com> Date: Thu, 2 Mar 2023 14:15:12 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:110.0) Gecko/20100101 Thunderbird/110.0 Content-Language: en-US To: pve-devel@lists.proxmox.com References: <20230222093655.2645787-3-a.lauterer@proxmox.com> <20230222093930.2659526-1-a.lauterer@proxmox.com> From: Dominik Csapak In-Reply-To: <20230222093930.2659526-1-a.lauterer@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.106 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.09 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH manager follow-up 2/2] ui: ceph status: add tooltip with details to warnings X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Mar 2023 13:15:14 -0000 one bug did occur when trying it out: if i had my mouse over a warning, and then the warnings vanished all, the tooltip was still there and i could trigger it with hovering over the grid so we want to clean the tooltips up on every load anyway, or at least when the number of items is smaller than the position my mouse is over ;) some comments inline On 2/22/23 10:39, Aaron Lauterer wrote: > This is another step to make it easier for admins to discover more > information for a warning or problem that is shown in the Ceph health > panel. > > The length is limited to give a first glimpse. For the full details one > can click on the info/detail button. > > Signed-off-by: Aaron Lauterer > --- > Please use this follow up! > > I found a small bug just after sending it regarding the max length. > The length check and substring did have different values. > > www/manager6/ceph/Status.js | 41 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/www/manager6/ceph/Status.js b/www/manager6/ceph/Status.js > index 45583f51..19a1243a 100644 > --- a/www/manager6/ceph/Status.js > +++ b/www/manager6/ceph/Status.js > @@ -76,6 +76,47 @@ Ext.define('PVE.node.CephStatus', { > trackRemoved: false, > data: [], > }, > + > + listeners: { > + destroy: function() { > + let view = this.getView(); > + if (view.tooltip) { > + view.tooltip.destroy(); > + delete view.tooltip; > + } you could also do : view.tooltip?.destroy(); delete view.tooltip; without the if also the function is the same on itemmouseleave and destroy, so please refactor that into a function (since we also have to use it elsewhere too, see my comment at the top) > + }, > + itemmouseenter: function(record, item) { > + let view = this.getView(); > + if (!view) { > + return; > + } > + if (!item.data.detail) { > + return; > + } > + let text = item.data.detail.trimStart().replaceAll('\n', '
'); any reason why we don't also use the 
 tag here like in the popup?
would at least be less computational intensive for long errors

also i'd check the length before replacing the newlines, otherwise
it can happen that you cut the text of in the middle of a 
 tag ;)


> +			    if (text.length > 500) {
> +			        text = `${text.substring(0, 500)}…`;

nit: any special reason for the ellipsis glyph instead of 3 dots?
not that it's a problem, just want to know

> +			    }
> +			    if (!view.tooltip) {
> +				view.tooltip = Ext.create('Ext.tip.ToolTip', {
> +				    target: view,
> +				    trackMouse: true,
> +				    dismissDelay: 0,
> +				    tpl: '{text}',
> +				    renderTo: Ext.getBody(),
> +				});
> +			    }
> +			    view.tooltip.setData({ text });
> +			    view.tooltip.show();
> +			},
> +			itemmouseleave: function(record, item) {
> +			    let view = this.getView();
> +			    if (view.tooltip) {
> +				view.tooltip.destroy();
> +				delete view.tooltip;
> +			    }
> +			},
> +		    },
>   		    emptyText: gettext('No Warnings/Errors'),
>   		    columns: [
>   			{