* [pve-devel] [PATCH v4 manager] ui: ceph: improve discoverability of warning details
@ 2023-03-15 13:09 Aaron Lauterer
2023-05-26 8:39 ` Aaron Lauterer
0 siblings, 1 reply; 4+ messages in thread
From: Aaron Lauterer @ 2023-03-15 13:09 UTC (permalink / raw)
To: pve-devel
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 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 a9ead5d3..012c5534 100644
--- a/www/css/ext6-pve.css
+++ b/www/css/ext6-pve.css
@@ -700,3 +700,9 @@ table.osds td:first-of-type {
cursor: pointer;
padding-left: 2px;
}
+
+.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.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH v4 manager] ui: ceph: improve discoverability of warning details
2023-03-15 13:09 [pve-devel] [PATCH v4 manager] ui: ceph: improve discoverability of warning details Aaron Lauterer
@ 2023-05-26 8:39 ` Aaron Lauterer
2023-05-28 17:32 ` Thomas Lamprecht
0 siblings, 1 reply; 4+ messages in thread
From: Aaron Lauterer @ 2023-05-26 8:39 UTC (permalink / raw)
To: pve-devel
ping? Considering that some users are rather hesitant to upgrade to a major
version, it might be a good idea to still get this into Proxmox VE 7 to make it
easier for users to discover more details about any issues they experience.
On 3/15/23 14:09, Aaron Lauterer wrote:
> 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 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 a9ead5d3..012c5534 100644
> --- a/www/css/ext6-pve.css
> +++ b/www/css/ext6-pve.css
> @@ -700,3 +700,9 @@ table.osds td:first-of-type {
> cursor: pointer;
> padding-left: 2px;
> }
> +
> +.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>',
> + },
> + ],
> },
> ],
> },
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH v4 manager] ui: ceph: improve discoverability of warning details
2023-05-26 8:39 ` Aaron Lauterer
@ 2023-05-28 17:32 ` Thomas Lamprecht
2023-05-30 9:33 ` Aaron Lauterer
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2023-05-28 17:32 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
Am 26/05/2023 um 10:39 schrieb Aaron Lauterer:
> ping? Considering that some users are rather hesitant to upgrade to a major version, it might be a good idea to still get this into Proxmox VE 7 to make it easier for users to discover more details about any issues they experience.
I think that's probably the best argument can make here, as this is to much
in "new feature" territory for my taste.
But, wouldn't this be possible besser solved in the checker script though?
If one gets as far as seeing warnings in the UI overview but just can't figure
out how to view details on them I'd think that they either ask on a support
channel or could see more details in the checker script which they should run
anyway, avoiding any breakage potential.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH v4 manager] ui: ceph: improve discoverability of warning details
2023-05-28 17:32 ` Thomas Lamprecht
@ 2023-05-30 9:33 ` Aaron Lauterer
0 siblings, 0 replies; 4+ messages in thread
From: Aaron Lauterer @ 2023-05-30 9:33 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
On 5/28/23 19:32, Thomas Lamprecht wrote:
> Am 26/05/2023 um 10:39 schrieb Aaron Lauterer:
>> ping? Considering that some users are rather hesitant to upgrade to a major version, it might be a good idea to still get this into Proxmox VE 7 to make it easier for users to discover more details about any issues they experience.
>
> I think that's probably the best argument can make here, as this is to much
> in "new feature" territory for my taste.
> But, wouldn't this be possible besser solved in the checker script though?
> If one gets as far as seeing warnings in the UI overview but just can't figure
> out how to view details on them I'd think that they either ask on a support
> channel or could see more details in the checker script which they should run
> anyway, avoiding any breakage potential.
If you mean the pve7to8 script with the checker script, then yeah, having that
in there too is of course a good idea, if it isn't yet.
My argument is that with the details for warnings easier to discover, we could
reduce the amount of people asking in the forum or opening support tickets.
Going from experience with PVE 6, PVE 7 will (unfortunately) still be in use for
quite a while after 8 has been released. That is why I would like to still see
this change in PVE 7.
But as you see fit.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-30 9:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-15 13:09 [pve-devel] [PATCH v4 manager] ui: ceph: improve discoverability of warning details Aaron Lauterer
2023-05-26 8:39 ` Aaron Lauterer
2023-05-28 17:32 ` Thomas Lamprecht
2023-05-30 9:33 ` Aaron Lauterer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox