[pve-devel] [PATCH manager v5] ui: ceph: improve discoverability of warning details

[pve-devel] [PATCH manager v5] ui: ceph: improve discoverability of warning details

[Aaron Lauterer]

by
* replacing the info button with expandable rows that contain the
  details of the warning
* adding two action buttons to copy the summary and details
* making the text selectable

The row expander works like the one in the mail gateway tracking center
-> doubleclick only opens it.

The height of the warning grid is limited to not grow too large.
A Diffstore is used to avoid expanded rows being collapsed on an update.

The rowexpander cannot hide the toggle out of the box. Therefore, if
there is no detailed message for a warning, we show a placeholder text.
We could consider extending it in the future to only show the toggle if
a defined condition is met.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---

changes since
v4:
* rebased so the patch applies again

v3:
* change the whole approach from tooltips and info window to integrating
it into the grid itself

 www/css/ext6-pve.css        |  6 +++
 www/manager6/ceph/Status.js | 89 +++++++++++++++++++++++++------------
 2 files changed, 67 insertions(+), 28 deletions(-)

diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
index edae462b..837b2210 100644
--- a/www/css/ext6-pve.css
+++ b/www/css/ext6-pve.css
@@ -709,3 +709,9 @@ table.osds td:first-of-type {
     opacity: 0.0;
     cursor: default;
 }
+
+.pve-ceph-warning-detail {
+    overflow: auto;
+    margin: 0;
+    padding-bottom: 10px;
+}
diff --git a/www/manager6/ceph/Status.js b/www/manager6/ceph/Status.js
index 46338b4a..6bbe33b4 100644
--- a/www/manager6/ceph/Status.js
+++ b/www/manager6/ceph/Status.js
@@ -1,3 +1,10 @@
+Ext.define('pve-ceph-warnings', {
+    extend: 'Ext.data.Model',
+    fields: ['id', 'summary', 'detail', 'severity'],
+    idProperty: 'id',
+});
+
+
 Ext.define('PVE.node.CephStatus', {
     extend: 'Ext.panel.Panel',
     alias: 'widget.pveNodeCephStatus',
@@ -70,35 +77,51 @@ Ext.define('PVE.node.CephStatus', {
 		    xtype: 'grid',
 		    itemId: 'warnings',
 		    flex: 2,
+		    maxHeight: 430,
 		    stateful: true,
 		    stateId: 'ceph-status-warnings',
+		    viewConfig: {
+			enableTextSelection: true,
+		    },
 		    // we load the store manually, to show an emptyText specify an empty intermediate store
 		    store: {
+			type: 'diff',
 			trackRemoved: false,
 			data: [],
+			rstore: {
+			    storeid: 'pve-ceph-warnings',
+			    type: 'update',
+			    model: 'pve-ceph-warnings',
+			},
 		    },
 		    updateHealth: function(health) {
 			let checks = health.checks || {};
 
 			let checkRecords = Object.keys(checks).sort().map(key => {
 			    let check = checks[key];
-			    return {
+			    let data = {
 				id: key,
 				summary: check.summary.message,
-				detail: check.detail.reduce((acc, v) => `${acc}\n${v.message}`, ''),
+				detail: check.detail.reduce((acc, v) => `${acc}\n${v.message}`, '').trimStart(),
 				severity: check.severity,
 			    };
+			    if (data.detail.length === 0) {
+				data.detail = "no additional data";
+			    }
+			    return data;
 			});
 
-			this.getStore().loadRawData(checkRecords, false);
+			let rstore = this.getStore().rstore;
+			rstore.loadData(checkRecords, false);
+			rstore.fireEvent('load', rstore, checkRecords, true);
 		    },
 		    emptyText: gettext('No Warnings/Errors'),
 		    columns: [
 			{
 			    dataIndex: 'severity',
-			    header: gettext('Severity'),
+			    tooltip: gettext('Severity'),
 			    align: 'center',
-			    width: 70,
+			    width: 38,
 			    renderer: function(value) {
 				let health = PVE.Utils.map_ceph_health[value];
 				let icon = PVE.Utils.get_health_icon(health);
@@ -118,38 +141,48 @@ Ext.define('PVE.node.CephStatus', {
 			},
 			{
 			    xtype: 'actioncolumn',
-			    width: 40,
+			    width: 50,
 			    align: 'center',
-			    tooltip: gettext('Detail'),
+			    tooltip: gettext('Actions'),
 			    items: [
 				{
-				    iconCls: 'x-fa fa-info-circle',
+				    iconCls: 'x-fa fa-files-o',
+				    tooltip: gettext('Copy summary'),
+				    handler: function(grid, rowindex, colindex, item, e, record) {
+					navigator.clipboard.writeText(record.data.summary);
+				    },
+				},
+				{
+				    iconCls: 'x-fa fa-clipboard',
+				    tooltip: gettext('Copy details'),
 				    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();
+					navigator.clipboard.writeText(record.data.detail);
 				    },
 				},
 			    ],
 			},
 		    ],
+		    listeners: {
+			itemdblclick: function(view, record, row, rowIdx, e) {
+			    // inspired by RowExpander.js
+
+			    let rowNode = view.getNode(rowIdx); let
+			    normalRow = Ext.fly(rowNode);
+
+			    let collapsedCls = view.rowBodyFeature.rowCollapsedCls;
+
+			    if (normalRow.hasCls(collapsedCls)) {
+				view.rowBodyFeature.rowExpander.toggleRow(rowIdx, record);
+			    }
+			},
+		    },
+		    plugins: [
+			{
+			    ptype: 'rowexpander',
+			    expandOnDblClick: false,
+			    rowBodyTpl: '<pre class="pve-ceph-warning-detail">{detail}</pre>',
+			},
+		    ],
 		},
 	    ],
 	},
-- 
2.39.2

[pve-devel] applied: [PATCH manager v5] ui: ceph: improve discoverability of warning details

[Thomas Lamprecht]

Am 02/10/2023 um 11:00 schrieb Aaron Lauterer:
> by
> * replacing the info button with expandable rows that contain the
>   details of the warning
> * adding two action buttons to copy the summary and details
> * making the text selectable
> 
> The row expander works like the one in the mail gateway tracking center
> -> doubleclick only opens it.
> 
> The height of the warning grid is limited to not grow too large.
> A Diffstore is used to avoid expanded rows being collapsed on an update.
> 
> The rowexpander cannot hide the toggle out of the box. Therefore, if
> there is no detailed message for a warning, we show a placeholder text.
> We could consider extending it in the future to only show the toggle if
> a defined condition is met.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> 
> changes since
> v4:
> * rebased so the patch applies again
> 
> v3:
> * change the whole approach from tooltips and info window to integrating
> it into the grid itself
> 
>  www/css/ext6-pve.css        |  6 +++
>  www/manager6/ceph/Status.js | 89 +++++++++++++++++++++++++------------
>  2 files changed, 67 insertions(+), 28 deletions(-)
> 
>

applied, with a few follow-ups as talked off list, thanks!

The biggest change is that we now only have one copy button
that copies all (severity, summary and details) at once.