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