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.laute...@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: [
                        {



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

Reply via email to