Re: [pve-devel] [PATCH manager follow-up 2/2] ui: ceph status: add tooltip with details to warnings

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

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 <a.lauterer@proxmox.com>
> ---
> 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', '<br>');

any reason why we don't also use the <pre> 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 <br> 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: [
>   			{