public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details
@ 2023-03-08 12:09 Aaron Lauterer
  2023-03-08 12:09 ` [pve-devel] [PATCH v3 manager 1/2] ui: ceph: make the warning detail button stand out more Aaron Lauterer
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Aaron Lauterer @ 2023-03-08 12:09 UTC (permalink / raw)
  To: pve-devel

The goal for this small series is to make it easier/more obvious to see
that there are potentially more details for a warning ceph is showing
by:
* having a tooltip with the details (limited in length)
* making the detail/info button more visible

changes since

v2:
* don't use arrow functions -> rework context in many places. mainly use
  'this' now
* destroy tooltip when no detail text is present. This can easily be
  tested by setting 'ceph osd set noout' for example
* guard show/hide on info button with '?'

v1:
* change creation of info window to use autoShow
* rework tooltip to:
  * only destroy it when leaving the whole grid, use show/hide for all
    other situations
  * factor out text handling of newlines and max lines
  * factor out updating and destroying into their own functions
  * add more listeners
  * hide it when hovering the info button
  * auto update it whenever the store is updated

Aaron Lauterer (2):
  ui: ceph: make the warning detail button stand out more
  ui: ceph status: add tooltip with details to warnings

 www/manager6/ceph/Status.js | 129 ++++++++++++++++++++++++++++--------
 1 file changed, 103 insertions(+), 26 deletions(-)

-- 
2.30.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH v3 manager 1/2] ui: ceph: make the warning detail button stand out more
  2023-03-08 12:09 [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details Aaron Lauterer
@ 2023-03-08 12:09 ` Aaron Lauterer
  2023-03-08 12:09 ` [pve-devel] [PATCH v3 manager 2/2] ui: ceph status: add tooltip with details to warnings Aaron Lauterer
  2023-03-08 16:05 ` [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details Thomas Lamprecht
  2 siblings, 0 replies; 6+ messages in thread
From: Aaron Lauterer @ 2023-03-08 12:09 UTC (permalink / raw)
  To: pve-devel

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>
---

changes since
v2: none
v1:
- use autoShow in the Ext.Create call

 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..b223ab35 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();
+				    Ext.create('Ext.window.Window', {
+					title: gettext('Detail'),
+					autoShow: true,
+					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>`,
+					    ],
+					}],
+				    });
 				},
-			    ],
+			    },
 			},
 		    ],
 		},
-- 
2.30.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH v3 manager 2/2] ui: ceph status: add tooltip with details to warnings
  2023-03-08 12:09 [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details Aaron Lauterer
  2023-03-08 12:09 ` [pve-devel] [PATCH v3 manager 1/2] ui: ceph: make the warning detail button stand out more Aaron Lauterer
@ 2023-03-08 12:09 ` Aaron Lauterer
  2023-03-08 16:05 ` [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details Thomas Lamprecht
  2 siblings, 0 replies; 6+ messages in thread
From: Aaron Lauterer @ 2023-03-08 12:09 UTC (permalink / raw)
  To: pve-devel

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>
---
changes since
v2:
* don't use arrow functions -> rework context in many places. mainly use
  'this' now
* destroy tooltip when no detail text is present. This can easily be
  tested by setting 'ceph osd set noout' for example
* guard show/hide on info button with '?'

v1:
* only destroy it when leaving the whole grid, use show/hide for all
other situations
* factor out text handling of newlines and max lines
* factor out updating and destroying into their own functions
* add more listeners
* hide it when hovering the info button
* auto update it whenever the store is updated to either hide it
  (nothing under the mouse) or set the content

 www/manager6/ceph/Status.js | 75 +++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/www/manager6/ceph/Status.js b/www/manager6/ceph/Status.js
index b223ab35..46ced651 100644
--- a/www/manager6/ceph/Status.js
+++ b/www/manager6/ceph/Status.js
@@ -76,6 +76,72 @@ Ext.define('PVE.node.CephStatus', {
 			trackRemoved: false,
 			data: [],
 		    },
+		    generateTooltipText: function(text) {
+			text = text.trimStart();
+			if (text.length > 500) {
+			    text = `${text.substring(0, 500)}…`;
+			}
+			return text.replaceAll('\n', '<br>');
+		    },
+		    updateTooltip: function(isLeave) {
+			let me = this;
+			if (!me.tooltip) {
+			    return;
+			}
+			if (me.store.data.length - 1 < me.tooltip.gridIndex || isLeave) {
+			    me.tooltip.hide();
+			    return;
+			}
+			let data = me.store.getData().items[me.tooltip.gridIndex].data;
+			if (!data.detail) {
+			    me.destroyTooltip();
+			    return;
+			}
+			let text = me.generateTooltipText(data.detail);
+			me.tooltip.setData({ text });
+			me.tooltip.show();
+		    },
+		    destroyTooltip: function() {
+			this.tooltip?.destroy();
+			delete this.tooltip;
+		    },
+
+		    listeners: {
+			destroy: function() {
+			    this.destroyTooltip();
+			},
+			itemmouseenter: function(view, record, item, index) {
+			    let me = this;
+			    if (!view) {
+				return;
+			    }
+			    if (!record.data.detail) {
+				if (me.tooltip) {
+				    me.destroyTooltip();
+				}
+				return;
+			    }
+			    let text = me.generateTooltipText(record.data.detail);
+			    if (!me.tooltip) {
+				 me.tooltip = Ext.create('Ext.tip.ToolTip', {
+				    target: view,
+				    trackMouse: true,
+				    dismissDelay: 0,
+				    tpl: '{text}',
+				    renderTo: Ext.getBody(),
+				});
+			    }
+			    me.tooltip.gridIndex = index;
+			    me.tooltip.setData({ text });
+			    me.tooltip.show();
+			},
+			itemmouseleave: function() {
+			    this.updateTooltip(true);
+			},
+			containermouseout: function() {
+			    this.destroyTooltip();
+			},
+		    },
 		    emptyText: gettext('No Warnings/Errors'),
 		    columns: [
 			{
@@ -133,6 +199,14 @@ Ext.define('PVE.node.CephStatus', {
 					}],
 				    });
 				},
+				listeners: {
+				    mouseover: function() {
+					this.up('#warnings').tooltip?.hide();
+				    },
+				    mouseout: function() {
+					this.up('#warnings').tooltip?.show();
+				    },
+				},
 			    },
 			},
 		    ],
@@ -286,6 +360,7 @@ Ext.define('PVE.node.CephStatus', {
 	me.down('#overallhealth').updateHealth(PVE.Utils.render_ceph_health(rec.data.health || {}));
 	// add errors to gridstore
 	me.down('#warnings').getStore().loadRawData(me.generateCheckData(rec.data.health || {}), false);
+	me.down('#warnings').updateTooltip();
 
 	// update services
 	me.getComponent('services').updateAll(me.metadata || {}, rec.data);
-- 
2.30.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details
  2023-03-08 12:09 [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details Aaron Lauterer
  2023-03-08 12:09 ` [pve-devel] [PATCH v3 manager 1/2] ui: ceph: make the warning detail button stand out more Aaron Lauterer
  2023-03-08 12:09 ` [pve-devel] [PATCH v3 manager 2/2] ui: ceph status: add tooltip with details to warnings Aaron Lauterer
@ 2023-03-08 16:05 ` Thomas Lamprecht
  2023-03-08 16:38   ` Aaron Lauterer
  2 siblings, 1 reply; 6+ messages in thread
From: Thomas Lamprecht @ 2023-03-08 16:05 UTC (permalink / raw)
  To: Proxmox VE development discussion, Aaron Lauterer

Am 08/03/2023 um 13:09 schrieb Aaron Lauterer:
> The goal for this small series is to make it easier/more obvious to see
> that there are potentially more details for a warning ceph is showing
> by:
> * having a tooltip with the details (limited in length)
> * making the detail/info button more visible
> 

One bug I see is that if I open the detail window, the next extjs (probably store)
refresh seemingly refocuses the some part of the row and then the tooltip pops up
too for the element I'm already having the full detail window open.

That said, why not avoid the tooltip but inline the info by moving the severity to
the right, dropping the info button completely and making the grid row expandable,
i.e., like the PMG tracking center grid does?

Would feel more integrated and less duplicate (tooltip + detail window)
information.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details
  2023-03-08 16:05 ` [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details Thomas Lamprecht
@ 2023-03-08 16:38   ` Aaron Lauterer
  2023-03-08 16:43     ` Thomas Lamprecht
  0 siblings, 1 reply; 6+ messages in thread
From: Aaron Lauterer @ 2023-03-08 16:38 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion



On 3/8/23 17:05, Thomas Lamprecht wrote:
> Am 08/03/2023 um 13:09 schrieb Aaron Lauterer:
>> The goal for this small series is to make it easier/more obvious to see
>> that there are potentially more details for a warning ceph is showing
>> by:
>> * having a tooltip with the details (limited in length)
>> * making the detail/info button more visible
>>
> 
> One bug I see is that if I open the detail window, the next extjs (probably store)
> refresh seemingly refocuses the some part of the row and then the tooltip pops up
> too for the element I'm already having the full detail window open.

Thanks for catching that! :-/

> 
> That said, why not avoid the tooltip but inline the info by moving the severity to
> the right, dropping the info button completely and making the grid row expandable,
> i.e., like the PMG tracking center grid does?

We thought about it, but initially I wasn't too happy about it, because if the 
detailed error message has many lines and/or long lines, it would be nicer to 
just open the window and making it as large as possible.
With the expandable row, the visible area can be rather small.

But with that unfortunate behavior with the detail window opened, I am more 
inclined to make the rows expandable.

I'll give it a try and will send a v4 :)

> 
> Would feel more integrated and less duplicate (tooltip + detail window)
> information.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details
  2023-03-08 16:38   ` Aaron Lauterer
@ 2023-03-08 16:43     ` Thomas Lamprecht
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2023-03-08 16:43 UTC (permalink / raw)
  To: Proxmox VE development discussion, Aaron Lauterer

Am 08/03/2023 um 17:38 schrieb Aaron Lauterer:
>> That said, why not avoid the tooltip but inline the info by moving the severity to
>> the right, dropping the info button completely and making the grid row expandable,
>> i.e., like the PMG tracking center grid does?
> 
> We thought about it, but initially I wasn't too happy about it, because if the detailed error message has many lines and/or long lines, it would be nicer to just open the window and making it as large as possible.

You could still have a open-in window there if we really want, copy-button can
help too for some "copy to support/forum/text-editor-for-searching" - as at least
I use this mostly to get a quick overview or to extract info.

One advantage of expansion: you can have all errors in view, not just one at a
time (or well, per tab ^^)




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-03-08 16:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-08 12:09 [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details Aaron Lauterer
2023-03-08 12:09 ` [pve-devel] [PATCH v3 manager 1/2] ui: ceph: make the warning detail button stand out more Aaron Lauterer
2023-03-08 12:09 ` [pve-devel] [PATCH v3 manager 2/2] ui: ceph status: add tooltip with details to warnings Aaron Lauterer
2023-03-08 16:05 ` [pve-devel] [PATCH v3 manager 0/2] ui: ceph: improve discoverability of warning details Thomas Lamprecht
2023-03-08 16:38   ` Aaron Lauterer
2023-03-08 16:43     ` Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal