* [pbs-devel] [PATCH proxmox-backup v2] ui: tape/BackupOverview: unify and improve tape restore window
@ 2021-05-12 11:49 Dominik Csapak
2021-05-12 19:34 ` Thomas Lamprecht
0 siblings, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2021-05-12 11:49 UTC (permalink / raw)
To: pbs-devel
adds a snapshot list to the restore window where the user can
select distinct snapshots to restore
this also replaces the restore / restore single button by
inline actions for the media-set, backup groups and snapshots
when clicking the action for a group/snapshot, the snapshotselector
gets activated and preselected/filtered so that it includes those
snapshots (can be overridden from the user)
this change means that we now load the media set everytime we open
the window, since we want to have the list of snapshots.
(we could build that from the tree again, or hold it there somewhere as
list, but i do not think we gain much with that, and it simplifies the
datastore selection code a bit)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* fixed snapshot selection checkbox (was in wrong context, looup needs
to happen on the referenceHolder)
* removed unnecessary 'node' variable in 'restore' function
* fixed 'media-set' text in window (was the text of the node we clicked on)
www/tape/BackupOverview.js | 91 ++++----------
www/tape/window/TapeRestore.js | 211 ++++++++++++++++++++++++++-------
2 files changed, 197 insertions(+), 105 deletions(-)
diff --git a/www/tape/BackupOverview.js b/www/tape/BackupOverview.js
index c028d58d..97725600 100644
--- a/www/tape/BackupOverview.js
+++ b/www/tape/BackupOverview.js
@@ -16,58 +16,16 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
}).show();
},
- restoreSingle: function(button, record) {
+ restore: function(view, rI, cI, item, e, rec) {
let me = this;
- let view = me.getView();
- let selection = view.getSelection();
- if (!selection || selection.length < 1) {
- return;
- }
-
- let node = selection[0];
- if (node.data.restoreid === undefined) {
- return;
- }
- let restoreid = node.data.restoreid;
- let mediaset = node.data.text;
- let uuid = node.data['media-set-uuid'];
- let datastores = [node.data.store];
-
- Ext.create('PBS.TapeManagement.TapeRestoreWindow', {
- mediaset,
- uuid,
- list: [
- restoreid,
- ],
- datastores,
- listeners: {
- destroy: function() {
- me.reload();
- },
- },
- }).show();
- },
- restore: function(button, record) {
- let me = this;
- let view = me.getView();
- let selection = view.getSelection();
- if (!selection || selection.length < 1) {
- return;
- }
-
- let node = selection[0];
- let mediaset = node.data.text;
- let uuid = node.data['media-set-uuid'];
- let datastores = node.data.datastores;
- while (!datastores && node.get('depth') > 2) {
- node = node.parentNode;
- datastores = node.data.datastores;
- }
+ let mediaset = rec.data.is_media_set ? rec.data.text : rec.data['media-set'];
+ let uuid = rec.data['media-set-uuid'];
+ let restoreid = rec.data.restoreid;
Ext.create('PBS.TapeManagement.TapeRestoreWindow', {
mediaset,
uuid,
- datastores,
+ prefilter: restoreid,
listeners: {
destroy: function() {
me.reload();
@@ -99,6 +57,7 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
if (data[pool][media_set] === undefined) {
data[pool][media_set] = entry;
data[pool][media_set].text = media_set;
+ data[pool][media_set].restore =true;
data[pool][media_set].tapes = 1;
data[pool][media_set]['seq-nr'] = undefined;
data[pool][media_set].is_media_set = true;
@@ -155,14 +114,15 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
let view = me.getView();
Proxmox.Utils.setErrorMask(view, true);
- const media_set = node.data['media-set-uuid'];
+ const media_set_uuid = node.data['media-set-uuid'];
+ const media_set = node.data.text;
try {
let list = await PBS.Async.api2({
method: 'GET',
url: `/api2/extjs/tape/media/content`,
params: {
- 'media-set': media_set,
+ 'media-set': media_set_uuid,
},
});
@@ -179,9 +139,11 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
for (let entry of list.result.data) {
entry.text = entry.snapshot;
+ entry.restore = true;
entry.leaf = true;
entry.children = [];
entry.restoreid = `${entry.store}:${entry.snapshot}`;
+ entry['media-set'] = media_set;
let iconCls = PBS.Utils.get_type_icon_cls(entry.snapshot);
if (iconCls !== '') {
entry.iconCls = `fa ${iconCls}`;
@@ -217,6 +179,9 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
text,
'media-set-uuid': entry['media-set-uuid'],
leaf: false,
+ restore: true,
+ restoreid: `${store}:${text}`,
+ 'media-set': media_set,
iconCls: `fa ${iconCls}`,
children: [],
});
@@ -287,22 +252,6 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
text: gettext('New Backup'),
handler: 'backup',
},
- {
- xtype: 'proxmoxButton',
- disabled: true,
- text: gettext('Restore Media Set'),
- handler: 'restore',
- parentXType: 'treepanel',
- enableFn: (rec) => !!rec.data['media-set-uuid'],
- },
- {
- xtype: 'proxmoxButton',
- disabled: true,
- text: gettext('Restore Snapshot'),
- handler: 'restoreSingle',
- parentXType: 'treepanel',
- enableFn: (rec) => !!rec.data.restoreid,
- },
],
columns: [
@@ -313,6 +262,18 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
sortable: false,
flex: 3,
},
+ {
+ header: gettext('Actions'),
+ xtype: 'actioncolumn',
+ items: [
+ {
+ handler: 'restore',
+ tooltip: gettext('Restore'),
+ getClass: (v, m, rec) => rec.data.restore ? 'fa fa-fw fa-undo' : 'pmx-hidden',
+ isDisabled: (v, r, c, i, rec) => !rec.data.restore,
+ },
+ ],
+ },
{
text: gettext('Tapes'),
dataIndex: 'tapes',
diff --git a/www/tape/window/TapeRestore.js b/www/tape/window/TapeRestore.js
index 7e4f5cae..1844e5eb 100644
--- a/www/tape/window/TapeRestore.js
+++ b/www/tape/window/TapeRestore.js
@@ -10,14 +10,13 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
showTaskViewer: true,
isCreate: true,
+ allowSnapshots: false,
+
cbindData: function(config) {
let me = this;
- me.isSingle = false;
- me.listText = "";
- if (me.list !== undefined) {
- me.isSingle = true;
- me.listText = me.list.join('<br>');
- me.title = gettext('Restore Snapshot');
+ if (me.prefilter !== undefined) {
+ me.allowSnapshots = true;
+ me.title = gettext('Restore Snapshot(s)');
}
return {};
},
@@ -45,8 +44,8 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
}
delete values.mapping;
- if (me.up('window').list !== undefined) {
- values.snapshots = me.up('window').list;
+ if (Ext.isString(values.snapshots)) {
+ values.snapshots = values.snapshots.split(',');
}
values.store = datastores.join(',');
@@ -71,15 +70,6 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
value: '{uuid}',
},
},
- {
- xtype: 'displayfield',
- fieldLabel: gettext('Snapshot(s)'),
- submitValue: false,
- cbind: {
- hidden: '{!isSingle}',
- value: '{listText}',
- },
- },
{
xtype: 'pbsDriveSelector',
fieldLabel: gettext('Drive'),
@@ -138,6 +128,30 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
defaultBindProperty: 'value',
hidden: true,
},
+ {
+ boxLabel: gettext('Snapshot Selection'),
+ labelWidth: 200,
+ xtype: 'proxmoxcheckbox',
+ isFormField: false,
+ cbind: {
+ value: '{allowSnapshots}',
+ },
+ listeners: {
+ change: function(cb, value) {
+ let me = this;
+ me.up('window').lookup('snapshotGrid').setDisabled(!value);
+ },
+ },
+ },
+ {
+ xtype: 'pbsTapeSnapshotGrid',
+ reference: 'snapshotGrid',
+ name: 'snapshots',
+ cbind: {
+ disabled: '{!allowSnapshots}',
+ prefilter: '{prefilter}',
+ },
+ },
],
},
],
@@ -171,29 +185,39 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
let me = this;
me.callParent();
- if (me.datastores) {
- me.setDataStores(me.datastores);
- } else {
- // use timeout so that the window is rendered already
- // for correct masking
- setTimeout(function() {
- Proxmox.Utils.API2Request({
- waitMsgTarget: me,
- url: `/tape/media/content?media-set=${me.uuid}`,
- success: function(response, opt) {
- let datastores = {};
- for (const content of response.result.data) {
- datastores[content.store] = true;
- }
- me.setDataStores(Object.keys(datastores));
- },
- failure: function() {
- // ignore failing api call, maybe catalog is missing
- me.setDataStores();
- },
- });
- }, 10);
- }
+ // use timeout so that the window is rendered already
+ // for correct masking
+ setTimeout(function() {
+ Proxmox.Utils.API2Request({
+ waitMsgTarget: me,
+ url: `/tape/media/content?media-set=${me.uuid}`,
+ success: function(response, opt) {
+ let datastores = {};
+ for (const content of response.result.data) {
+ datastores[content.store] = true;
+ }
+ me.setDataStores(Object.keys(datastores));
+ let grid = me.lookup('snapshotGrid');
+ let store = grid.getStore();
+ store.setData(response.result.data);
+ if (me.prefilter !== undefined) {
+ store.each((rec) => {
+ let restoreid = `${rec.data.store}:${rec.data.snapshot}`;
+ if (restoreid.indexOf(me.prefilter) !== -1) {
+ rec.set('include', true);
+ }
+ });
+
+ grid.setFilter(me.prefilter);
+ }
+ store.sort('snapshot');
+ },
+ failure: function() {
+ // ignore failing api call, maybe catalog is missing
+ me.setDataStores();
+ },
+ });
+ }, 10);
},
});
@@ -202,6 +226,10 @@ Ext.define('PBS.TapeManagement.DataStoreMappingGrid', {
alias: 'widget.pbsDataStoreMappingField',
mixins: ['Ext.form.field.Field'],
+ maxHeight: 150,
+ scrollable: true,
+ margin: '0 0 10 0',
+
getValue: function() {
let me = this;
let datastores = [];
@@ -308,3 +336,106 @@ Ext.define('PBS.TapeManagement.DataStoreMappingGrid', {
},
],
});
+
+Ext.define('PBS.TapeManagement.SnapshotGrid', {
+ extend: 'Ext.grid.Panel',
+ alias: 'widget.pbsTapeSnapshotGrid',
+ mixins: ['Ext.form.field.Field'],
+
+ getValue: function() {
+ let me = this;
+ let snapshots = [];
+
+ me.getStore().each((rec) => {
+ if (rec.data.include) {
+ let store = rec.data.store;
+ let snap = rec.data.snapshot;
+ snapshots.push(`${store}:${snap}`);
+ }
+ });
+
+ return snapshots;
+ },
+
+ setValue: function(value) {
+ let me = this;
+ // not implemented
+ return me;
+ },
+
+ getErrors: function(value) {
+ let me = this;
+ let firstSelected = me.getStore().findBy((rec) => !!rec.data.include);
+
+ if (firstSelected === -1) {
+ me.addCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
+ let errorMsg = gettext("Need at least one snapshot");
+ me.getActionEl().dom.setAttribute('data-errorqtip', errorMsg);
+
+ return [errorMsg];
+ }
+ me.removeCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
+ me.getActionEl().dom.setAttribute('data-errorqtip', "");
+ return [];
+ },
+
+ scrollable: true,
+ height: 200,
+
+ viewConfig: {
+ emptyText: gettext('No Snapshots'),
+ markDirty: false,
+ },
+
+ setFilter: function(value) {
+ let me = this;
+ me.down('textfield').setValue(value);
+ },
+
+ tbar: [
+ {
+ fieldLabel: gettext('Filter'),
+ xtype: 'textfield',
+ isFormField: false,
+ listeners: {
+ change: function(field, value) {
+ let me = this;
+ let grid = me.up('grid');
+ let store = grid.getStore();
+ store.clearFilter();
+ store.filterBy((rec) => {
+ let restoreid = `${rec.data.store}:${rec.data.snapshot}`;
+ return restoreid.indexOf(value) !== -1;
+ });
+ grid.checkChange();
+ },
+ },
+ },
+ ],
+
+ store: { data: [] },
+
+ columns: [
+ {
+ xtype: 'checkcolumn',
+ text: gettext('Include'),
+ dataIndex: 'include',
+ listeners: {
+ checkchange: function(cb, value) {
+ let grid = this.up('grid');
+ grid.checkChange();
+ },
+ },
+ },
+ {
+ text: gettext('Source Datastore'),
+ dataIndex: 'store',
+ flex: 1,
+ },
+ {
+ text: gettext('Snapshot'),
+ dataIndex: 'snapshot',
+ flex: 4,
+ },
+ ],
+});
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v2] ui: tape/BackupOverview: unify and improve tape restore window
2021-05-12 11:49 [pbs-devel] [PATCH proxmox-backup v2] ui: tape/BackupOverview: unify and improve tape restore window Dominik Csapak
@ 2021-05-12 19:34 ` Thomas Lamprecht
2021-05-14 6:29 ` Dominik Csapak
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2021-05-12 19:34 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
On 12.05.21 13:49, Dominik Csapak wrote:
> adds a snapshot list to the restore window where the user can
> select distinct snapshots to restore
>
> this also replaces the restore / restore single button by
> inline actions for the media-set, backup groups and snapshots
>
> when clicking the action for a group/snapshot, the snapshotselector
> gets activated and preselected/filtered so that it includes those
> snapshots (can be overridden from the user)
>
> this change means that we now load the media set everytime we open
> the window, since we want to have the list of snapshots.
> (we could build that from the tree again, or hold it there somewhere as
> list, but i do not think we gain much with that, and it simplifies the
> datastore selection code a bit)
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * fixed snapshot selection checkbox (was in wrong context, looup needs
> to happen on the referenceHolder)
> * removed unnecessary 'node' variable in 'restore' function
> * fixed 'media-set' text in window (was the text of the node we clicked on)
>
Recently it seem to me like we alternate between "place every new function and
it's use in 42 separate patches" and "dump^W squash three-zillion changes into
one" like a metronome, none of those is ideal IMO.
IOW, this really needs to be two patches, one for the content overview changes
and the restore widow.
Also, from my last reply to the v1 thread:
> IMO slightly better than radio-group/checkbox as it removes a UI control
> element completely, so less choice/confusion, but without scarifying flexibility,
> so IMO a good idea.
Now you added the extra checkbox... Why not just use the grid-header checkbox like
PVE backup-job edit-window and tick it by default when restoring a whole set?
As said, reduction by one knob without losing any flexibility - seems like a win
win to me, or do I overlook something?
With the list below the window's aspect ration feels wrong and the list a bit
squished, I'd add a few 100s px to the window height to improve that.
> www/tape/BackupOverview.js | 91 ++++----------
> www/tape/window/TapeRestore.js | 211 ++++++++++++++++++++++++++-------
> 2 files changed, 197 insertions(+), 105 deletions(-)
>
> diff --git a/www/tape/BackupOverview.js b/www/tape/BackupOverview.js
> index c028d58d..97725600 100644
> --- a/www/tape/BackupOverview.js
> +++ b/www/tape/BackupOverview.js
> @@ -16,58 +16,16 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
> }).show();
> },
>
> - restoreSingle: function(button, record) {
> + restore: function(view, rI, cI, item, e, rec) {
> let me = this;
> - let view = me.getView();
> - let selection = view.getSelection();
> - if (!selection || selection.length < 1) {
> - return;
> - }
> -
> - let node = selection[0];
> - if (node.data.restoreid === undefined) {
> - return;
> - }
> - let restoreid = node.data.restoreid;
> - let mediaset = node.data.text;
> - let uuid = node.data['media-set-uuid'];
> - let datastores = [node.data.store];
> -
> - Ext.create('PBS.TapeManagement.TapeRestoreWindow', {
> - mediaset,
> - uuid,
> - list: [
> - restoreid,
> - ],
> - datastores,
> - listeners: {
> - destroy: function() {
> - me.reload();
> - },
> - },
> - }).show();
> - },
>
> - restore: function(button, record) {
> - let me = this;
> - let view = me.getView();
> - let selection = view.getSelection();
> - if (!selection || selection.length < 1) {
> - return;
> - }
> -
> - let node = selection[0];
> - let mediaset = node.data.text;
> - let uuid = node.data['media-set-uuid'];
> - let datastores = node.data.datastores;
> - while (!datastores && node.get('depth') > 2) {
> - node = node.parentNode;
> - datastores = node.data.datastores;
> - }
> + let mediaset = rec.data.is_media_set ? rec.data.text : rec.data['media-set'];
> + let uuid = rec.data['media-set-uuid'];
> + let restoreid = rec.data.restoreid;
> Ext.create('PBS.TapeManagement.TapeRestoreWindow', {
> mediaset,
> uuid,
> - datastores,
> + prefilter: restoreid,
> listeners: {
> destroy: function() {
> me.reload();
> @@ -99,6 +57,7 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
> if (data[pool][media_set] === undefined) {
> data[pool][media_set] = entry;
> data[pool][media_set].text = media_set;
> + data[pool][media_set].restore =true;
> data[pool][media_set].tapes = 1;
> data[pool][media_set]['seq-nr'] = undefined;
> data[pool][media_set].is_media_set = true;
> @@ -155,14 +114,15 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
> let view = me.getView();
>
> Proxmox.Utils.setErrorMask(view, true);
> - const media_set = node.data['media-set-uuid'];
> + const media_set_uuid = node.data['media-set-uuid'];
> + const media_set = node.data.text;
>
> try {
> let list = await PBS.Async.api2({
> method: 'GET',
> url: `/api2/extjs/tape/media/content`,
> params: {
> - 'media-set': media_set,
> + 'media-set': media_set_uuid,
> },
> });
>
> @@ -179,9 +139,11 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>
> for (let entry of list.result.data) {
> entry.text = entry.snapshot;
> + entry.restore = true;
> entry.leaf = true;
> entry.children = [];
> entry.restoreid = `${entry.store}:${entry.snapshot}`;
> + entry['media-set'] = media_set;
> let iconCls = PBS.Utils.get_type_icon_cls(entry.snapshot);
> if (iconCls !== '') {
> entry.iconCls = `fa ${iconCls}`;
> @@ -217,6 +179,9 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
> text,
> 'media-set-uuid': entry['media-set-uuid'],
> leaf: false,
> + restore: true,
> + restoreid: `${store}:${text}`,
> + 'media-set': media_set,
> iconCls: `fa ${iconCls}`,
> children: [],
> });
> @@ -287,22 +252,6 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
> text: gettext('New Backup'),
> handler: 'backup',
> },
> - {
> - xtype: 'proxmoxButton',
> - disabled: true,
> - text: gettext('Restore Media Set'),
> - handler: 'restore',
> - parentXType: 'treepanel',
> - enableFn: (rec) => !!rec.data['media-set-uuid'],
> - },
> - {
> - xtype: 'proxmoxButton',
> - disabled: true,
> - text: gettext('Restore Snapshot'),
> - handler: 'restoreSingle',
> - parentXType: 'treepanel',
> - enableFn: (rec) => !!rec.data.restoreid,
> - },
> ],
>
> columns: [
> @@ -313,6 +262,18 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
> sortable: false,
> flex: 3,
> },
> + {
> + header: gettext('Actions'),
> + xtype: 'actioncolumn',
> + items: [
> + {
> + handler: 'restore',
> + tooltip: gettext('Restore'),
> + getClass: (v, m, rec) => rec.data.restore ? 'fa fa-fw fa-undo' : 'pmx-hidden',
> + isDisabled: (v, r, c, i, rec) => !rec.data.restore,
> + },
> + ],
> + },
> {
> text: gettext('Tapes'),
> dataIndex: 'tapes',
> diff --git a/www/tape/window/TapeRestore.js b/www/tape/window/TapeRestore.js
> index 7e4f5cae..1844e5eb 100644
> --- a/www/tape/window/TapeRestore.js
> +++ b/www/tape/window/TapeRestore.js
> @@ -10,14 +10,13 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
> showTaskViewer: true,
> isCreate: true,
>
> + allowSnapshots: false,
> +
> cbindData: function(config) {
> let me = this;
> - me.isSingle = false;
> - me.listText = "";
> - if (me.list !== undefined) {
> - me.isSingle = true;
> - me.listText = me.list.join('<br>');
> - me.title = gettext('Restore Snapshot');
> + if (me.prefilter !== undefined) {
> + me.allowSnapshots = true;
> + me.title = gettext('Restore Snapshot(s)');
> }
> return {};
> },
> @@ -45,8 +44,8 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
> }
> delete values.mapping;
>
> - if (me.up('window').list !== undefined) {
> - values.snapshots = me.up('window').list;
> + if (Ext.isString(values.snapshots)) {
> + values.snapshots = values.snapshots.split(',');
> }
>
> values.store = datastores.join(',');
> @@ -71,15 +70,6 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
> value: '{uuid}',
> },
> },
> - {
> - xtype: 'displayfield',
> - fieldLabel: gettext('Snapshot(s)'),
> - submitValue: false,
> - cbind: {
> - hidden: '{!isSingle}',
> - value: '{listText}',
> - },
> - },
> {
> xtype: 'pbsDriveSelector',
> fieldLabel: gettext('Drive'),
> @@ -138,6 +128,30 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
> defaultBindProperty: 'value',
> hidden: true,
> },
> + {
> + boxLabel: gettext('Snapshot Selection'),
> + labelWidth: 200,
> + xtype: 'proxmoxcheckbox',
> + isFormField: false,
> + cbind: {
> + value: '{allowSnapshots}',
> + },
> + listeners: {
> + change: function(cb, value) {
> + let me = this;
> + me.up('window').lookup('snapshotGrid').setDisabled(!value);
> + },
> + },
> + },
> + {
> + xtype: 'pbsTapeSnapshotGrid',
> + reference: 'snapshotGrid',
> + name: 'snapshots',
> + cbind: {
> + disabled: '{!allowSnapshots}',
> + prefilter: '{prefilter}',
> + },
> + },
> ],
> },
> ],
> @@ -171,29 +185,39 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
> let me = this;
>
> me.callParent();
> - if (me.datastores) {
> - me.setDataStores(me.datastores);
> - } else {
> - // use timeout so that the window is rendered already
> - // for correct masking
> - setTimeout(function() {
> - Proxmox.Utils.API2Request({
> - waitMsgTarget: me,
> - url: `/tape/media/content?media-set=${me.uuid}`,
> - success: function(response, opt) {
> - let datastores = {};
> - for (const content of response.result.data) {
> - datastores[content.store] = true;
> - }
> - me.setDataStores(Object.keys(datastores));
> - },
> - failure: function() {
> - // ignore failing api call, maybe catalog is missing
> - me.setDataStores();
> - },
> - });
> - }, 10);
> - }
> + // use timeout so that the window is rendered already
> + // for correct masking
> + setTimeout(function() {
> + Proxmox.Utils.API2Request({
> + waitMsgTarget: me,
> + url: `/tape/media/content?media-set=${me.uuid}`,
> + success: function(response, opt) {
> + let datastores = {};
> + for (const content of response.result.data) {
> + datastores[content.store] = true;
> + }
> + me.setDataStores(Object.keys(datastores));
> + let grid = me.lookup('snapshotGrid');
> + let store = grid.getStore();
> + store.setData(response.result.data);
> + if (me.prefilter !== undefined) {
> + store.each((rec) => {
> + let restoreid = `${rec.data.store}:${rec.data.snapshot}`;
> + if (restoreid.indexOf(me.prefilter) !== -1) {
> + rec.set('include', true);
> + }
> + });
> +
> + grid.setFilter(me.prefilter);
> + }
> + store.sort('snapshot');
> + },
> + failure: function() {
> + // ignore failing api call, maybe catalog is missing
> + me.setDataStores();
> + },
> + });
> + }, 10);
> },
> });
>
> @@ -202,6 +226,10 @@ Ext.define('PBS.TapeManagement.DataStoreMappingGrid', {
> alias: 'widget.pbsDataStoreMappingField',
> mixins: ['Ext.form.field.Field'],
>
> + maxHeight: 150,
> + scrollable: true,
> + margin: '0 0 10 0',
> +
> getValue: function() {
> let me = this;
> let datastores = [];
> @@ -308,3 +336,106 @@ Ext.define('PBS.TapeManagement.DataStoreMappingGrid', {
> },
> ],
> });
> +
> +Ext.define('PBS.TapeManagement.SnapshotGrid', {
> + extend: 'Ext.grid.Panel',
> + alias: 'widget.pbsTapeSnapshotGrid',
> + mixins: ['Ext.form.field.Field'],
> +
> + getValue: function() {
> + let me = this;
> + let snapshots = [];
> +
> + me.getStore().each((rec) => {
> + if (rec.data.include) {
> + let store = rec.data.store;
> + let snap = rec.data.snapshot;
> + snapshots.push(`${store}:${snap}`);
> + }
> + });
> +
> + return snapshots;
> + },
> +
> + setValue: function(value) {
> + let me = this;
> + // not implemented
> + return me;
> + },
> +
> + getErrors: function(value) {
> + let me = this;
> + let firstSelected = me.getStore().findBy((rec) => !!rec.data.include);
> +
> + if (firstSelected === -1) {
> + me.addCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
> + let errorMsg = gettext("Need at least one snapshot");
> + me.getActionEl().dom.setAttribute('data-errorqtip', errorMsg);
> +
> + return [errorMsg];
> + }
> + me.removeCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
> + me.getActionEl().dom.setAttribute('data-errorqtip', "");
> + return [];
> + },
> +
> + scrollable: true,
> + height: 200,
> +
> + viewConfig: {
> + emptyText: gettext('No Snapshots'),
> + markDirty: false,
> + },
> +
> + setFilter: function(value) {
> + let me = this;
> + me.down('textfield').setValue(value);
> + },
> +
> + tbar: [
> + {
> + fieldLabel: gettext('Filter'),
> + xtype: 'textfield',
> + isFormField: false,
> + listeners: {
> + change: function(field, value) {
> + let me = this;
> + let grid = me.up('grid');
> + let store = grid.getStore();
> + store.clearFilter();
> + store.filterBy((rec) => {
> + let restoreid = `${rec.data.store}:${rec.data.snapshot}`;
> + return restoreid.indexOf(value) !== -1;
> + });
> + grid.checkChange();
> + },
> + },
> + },
> + ],
> +
> + store: { data: [] },
> +
> + columns: [
> + {
> + xtype: 'checkcolumn',
> + text: gettext('Include'),
> + dataIndex: 'include',
> + listeners: {
> + checkchange: function(cb, value) {
> + let grid = this.up('grid');
> + grid.checkChange();
> + },
> + },
> + },
> + {
> + text: gettext('Source Datastore'),
> + dataIndex: 'store',
> + flex: 1,
> + },
> + {
> + text: gettext('Snapshot'),
> + dataIndex: 'snapshot',
> + flex: 4,
> + },
> + ],
> +});
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v2] ui: tape/BackupOverview: unify and improve tape restore window
2021-05-12 19:34 ` Thomas Lamprecht
@ 2021-05-14 6:29 ` Dominik Csapak
2021-05-14 6:37 ` Thomas Lamprecht
0 siblings, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2021-05-14 6:29 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox Backup Server development discussion
On 5/12/21 21:34, Thomas Lamprecht wrote:
> On 12.05.21 13:49, Dominik Csapak wrote:
>> adds a snapshot list to the restore window where the user can
>> select distinct snapshots to restore
>>
>> this also replaces the restore / restore single button by
>> inline actions for the media-set, backup groups and snapshots
>>
>> when clicking the action for a group/snapshot, the snapshotselector
>> gets activated and preselected/filtered so that it includes those
>> snapshots (can be overridden from the user)
>>
>> this change means that we now load the media set everytime we open
>> the window, since we want to have the list of snapshots.
>> (we could build that from the tree again, or hold it there somewhere as
>> list, but i do not think we gain much with that, and it simplifies the
>> datastore selection code a bit)
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> changes from v1:
>> * fixed snapshot selection checkbox (was in wrong context, looup needs
>> to happen on the referenceHolder)
>> * removed unnecessary 'node' variable in 'restore' function
>> * fixed 'media-set' text in window (was the text of the node we clicked on)
>>
>
> Recently it seem to me like we alternate between "place every new function and
> it's use in 42 separate patches" and "dump^W squash three-zillion changes into
> one" like a metronome, none of those is ideal IMO.
>
> IOW, this really needs to be two patches, one for the content overview changes
> and the restore widow.
ok will do
>
> Also, from my last reply to the v1 thread:
>
>> IMO slightly better than radio-group/checkbox as it removes a UI control
>> element completely, so less choice/confusion, but without scarifying flexibility,
>> so IMO a good idea.
>
> Now you added the extra checkbox... Why not just use the grid-header checkbox like
> PVE backup-job edit-window and tick it by default when restoring a whole set?
>
> As said, reduction by one knob without losing any flexibility - seems like a win
> win to me, or do I overlook something?
>
> With the list below the window's aspect ration feels wrong and the list a bit
> squished, I'd add a few 100s px to the window height to improve that.
>
ok this was a misunderstanding then...
yeah using the 'check all' checkbox makes sense, but then i am unsure
how to handle filtering.
now only visible entries get restored, so if a user would (with the
check all) filter, it would seem like still all are selected and
we generate wrong assumptions?
we *should* be able to uncheck the check all box though to reduce
confusion, and this seems to me the best way forward.
the alternative is that we restore *all* selected entries, filtered or
not, but this can be similarly confusing since things get restored that
weren't visible
in pve's 'mass *' windows we have a behaviour like the first suggestion,
but i just noticed, depending on which column gets filtered, the
'check all' box sometimes stays checked and sometimes not...
as for the height, if there are multiple datastores on the media-set
(idk if you tested with this), there is another grid for the
datastore mapping, with that the window is already quite tall
maybe a multi-step 'wizard' would be better?
let the user first select the basic choices (drive,user,etc.)
then a window with a full-size grid of the snapshots, and then
depending on what he selected, either a target datastore
or datastore mapping input?
(or the snapshots first, and the rest of the info on the second panel?)
>> www/tape/BackupOverview.js | 91 ++++----------
>> www/tape/window/TapeRestore.js | 211 ++++++++++++++++++++++++++-------
>> 2 files changed, 197 insertions(+), 105 deletions(-)
>>
>> diff --git a/www/tape/BackupOverview.js b/www/tape/BackupOverview.js
>> index c028d58d..97725600 100644
>> --- a/www/tape/BackupOverview.js
>> +++ b/www/tape/BackupOverview.js
>> @@ -16,58 +16,16 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>> }).show();
>> },
>>
>> - restoreSingle: function(button, record) {
>> + restore: function(view, rI, cI, item, e, rec) {
>> let me = this;
>> - let view = me.getView();
>> - let selection = view.getSelection();
>> - if (!selection || selection.length < 1) {
>> - return;
>> - }
>> -
>> - let node = selection[0];
>> - if (node.data.restoreid === undefined) {
>> - return;
>> - }
>> - let restoreid = node.data.restoreid;
>> - let mediaset = node.data.text;
>> - let uuid = node.data['media-set-uuid'];
>> - let datastores = [node.data.store];
>> -
>> - Ext.create('PBS.TapeManagement.TapeRestoreWindow', {
>> - mediaset,
>> - uuid,
>> - list: [
>> - restoreid,
>> - ],
>> - datastores,
>> - listeners: {
>> - destroy: function() {
>> - me.reload();
>> - },
>> - },
>> - }).show();
>> - },
>>
>> - restore: function(button, record) {
>> - let me = this;
>> - let view = me.getView();
>> - let selection = view.getSelection();
>> - if (!selection || selection.length < 1) {
>> - return;
>> - }
>> -
>> - let node = selection[0];
>> - let mediaset = node.data.text;
>> - let uuid = node.data['media-set-uuid'];
>> - let datastores = node.data.datastores;
>> - while (!datastores && node.get('depth') > 2) {
>> - node = node.parentNode;
>> - datastores = node.data.datastores;
>> - }
>> + let mediaset = rec.data.is_media_set ? rec.data.text : rec.data['media-set'];
>> + let uuid = rec.data['media-set-uuid'];
>> + let restoreid = rec.data.restoreid;
>> Ext.create('PBS.TapeManagement.TapeRestoreWindow', {
>> mediaset,
>> uuid,
>> - datastores,
>> + prefilter: restoreid,
>> listeners: {
>> destroy: function() {
>> me.reload();
>> @@ -99,6 +57,7 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>> if (data[pool][media_set] === undefined) {
>> data[pool][media_set] = entry;
>> data[pool][media_set].text = media_set;
>> + data[pool][media_set].restore =true;
>> data[pool][media_set].tapes = 1;
>> data[pool][media_set]['seq-nr'] = undefined;
>> data[pool][media_set].is_media_set = true;
>> @@ -155,14 +114,15 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>> let view = me.getView();
>>
>> Proxmox.Utils.setErrorMask(view, true);
>> - const media_set = node.data['media-set-uuid'];
>> + const media_set_uuid = node.data['media-set-uuid'];
>> + const media_set = node.data.text;
>>
>> try {
>> let list = await PBS.Async.api2({
>> method: 'GET',
>> url: `/api2/extjs/tape/media/content`,
>> params: {
>> - 'media-set': media_set,
>> + 'media-set': media_set_uuid,
>> },
>> });
>>
>> @@ -179,9 +139,11 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>>
>> for (let entry of list.result.data) {
>> entry.text = entry.snapshot;
>> + entry.restore = true;
>> entry.leaf = true;
>> entry.children = [];
>> entry.restoreid = `${entry.store}:${entry.snapshot}`;
>> + entry['media-set'] = media_set;
>> let iconCls = PBS.Utils.get_type_icon_cls(entry.snapshot);
>> if (iconCls !== '') {
>> entry.iconCls = `fa ${iconCls}`;
>> @@ -217,6 +179,9 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>> text,
>> 'media-set-uuid': entry['media-set-uuid'],
>> leaf: false,
>> + restore: true,
>> + restoreid: `${store}:${text}`,
>> + 'media-set': media_set,
>> iconCls: `fa ${iconCls}`,
>> children: [],
>> });
>> @@ -287,22 +252,6 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>> text: gettext('New Backup'),
>> handler: 'backup',
>> },
>> - {
>> - xtype: 'proxmoxButton',
>> - disabled: true,
>> - text: gettext('Restore Media Set'),
>> - handler: 'restore',
>> - parentXType: 'treepanel',
>> - enableFn: (rec) => !!rec.data['media-set-uuid'],
>> - },
>> - {
>> - xtype: 'proxmoxButton',
>> - disabled: true,
>> - text: gettext('Restore Snapshot'),
>> - handler: 'restoreSingle',
>> - parentXType: 'treepanel',
>> - enableFn: (rec) => !!rec.data.restoreid,
>> - },
>> ],
>>
>> columns: [
>> @@ -313,6 +262,18 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>> sortable: false,
>> flex: 3,
>> },
>> + {
>> + header: gettext('Actions'),
>> + xtype: 'actioncolumn',
>> + items: [
>> + {
>> + handler: 'restore',
>> + tooltip: gettext('Restore'),
>> + getClass: (v, m, rec) => rec.data.restore ? 'fa fa-fw fa-undo' : 'pmx-hidden',
>> + isDisabled: (v, r, c, i, rec) => !rec.data.restore,
>> + },
>> + ],
>> + },
>> {
>> text: gettext('Tapes'),
>> dataIndex: 'tapes',
>> diff --git a/www/tape/window/TapeRestore.js b/www/tape/window/TapeRestore.js
>> index 7e4f5cae..1844e5eb 100644
>> --- a/www/tape/window/TapeRestore.js
>> +++ b/www/tape/window/TapeRestore.js
>> @@ -10,14 +10,13 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>> showTaskViewer: true,
>> isCreate: true,
>>
>> + allowSnapshots: false,
>> +
>> cbindData: function(config) {
>> let me = this;
>> - me.isSingle = false;
>> - me.listText = "";
>> - if (me.list !== undefined) {
>> - me.isSingle = true;
>> - me.listText = me.list.join('<br>');
>> - me.title = gettext('Restore Snapshot');
>> + if (me.prefilter !== undefined) {
>> + me.allowSnapshots = true;
>> + me.title = gettext('Restore Snapshot(s)');
>> }
>> return {};
>> },
>> @@ -45,8 +44,8 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>> }
>> delete values.mapping;
>>
>> - if (me.up('window').list !== undefined) {
>> - values.snapshots = me.up('window').list;
>> + if (Ext.isString(values.snapshots)) {
>> + values.snapshots = values.snapshots.split(',');
>> }
>>
>> values.store = datastores.join(',');
>> @@ -71,15 +70,6 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>> value: '{uuid}',
>> },
>> },
>> - {
>> - xtype: 'displayfield',
>> - fieldLabel: gettext('Snapshot(s)'),
>> - submitValue: false,
>> - cbind: {
>> - hidden: '{!isSingle}',
>> - value: '{listText}',
>> - },
>> - },
>> {
>> xtype: 'pbsDriveSelector',
>> fieldLabel: gettext('Drive'),
>> @@ -138,6 +128,30 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>> defaultBindProperty: 'value',
>> hidden: true,
>> },
>> + {
>> + boxLabel: gettext('Snapshot Selection'),
>> + labelWidth: 200,
>> + xtype: 'proxmoxcheckbox',
>> + isFormField: false,
>> + cbind: {
>> + value: '{allowSnapshots}',
>> + },
>> + listeners: {
>> + change: function(cb, value) {
>> + let me = this;
>> + me.up('window').lookup('snapshotGrid').setDisabled(!value);
>> + },
>> + },
>> + },
>> + {
>> + xtype: 'pbsTapeSnapshotGrid',
>> + reference: 'snapshotGrid',
>> + name: 'snapshots',
>> + cbind: {
>> + disabled: '{!allowSnapshots}',
>> + prefilter: '{prefilter}',
>> + },
>> + },
>> ],
>> },
>> ],
>> @@ -171,29 +185,39 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>> let me = this;
>>
>> me.callParent();
>> - if (me.datastores) {
>> - me.setDataStores(me.datastores);
>> - } else {
>> - // use timeout so that the window is rendered already
>> - // for correct masking
>> - setTimeout(function() {
>> - Proxmox.Utils.API2Request({
>> - waitMsgTarget: me,
>> - url: `/tape/media/content?media-set=${me.uuid}`,
>> - success: function(response, opt) {
>> - let datastores = {};
>> - for (const content of response.result.data) {
>> - datastores[content.store] = true;
>> - }
>> - me.setDataStores(Object.keys(datastores));
>> - },
>> - failure: function() {
>> - // ignore failing api call, maybe catalog is missing
>> - me.setDataStores();
>> - },
>> - });
>> - }, 10);
>> - }
>> + // use timeout so that the window is rendered already
>> + // for correct masking
>> + setTimeout(function() {
>> + Proxmox.Utils.API2Request({
>> + waitMsgTarget: me,
>> + url: `/tape/media/content?media-set=${me.uuid}`,
>> + success: function(response, opt) {
>> + let datastores = {};
>> + for (const content of response.result.data) {
>> + datastores[content.store] = true;
>> + }
>> + me.setDataStores(Object.keys(datastores));
>> + let grid = me.lookup('snapshotGrid');
>> + let store = grid.getStore();
>> + store.setData(response.result.data);
>> + if (me.prefilter !== undefined) {
>> + store.each((rec) => {
>> + let restoreid = `${rec.data.store}:${rec.data.snapshot}`;
>> + if (restoreid.indexOf(me.prefilter) !== -1) {
>> + rec.set('include', true);
>> + }
>> + });
>> +
>> + grid.setFilter(me.prefilter);
>> + }
>> + store.sort('snapshot');
>> + },
>> + failure: function() {
>> + // ignore failing api call, maybe catalog is missing
>> + me.setDataStores();
>> + },
>> + });
>> + }, 10);
>> },
>> });
>>
>> @@ -202,6 +226,10 @@ Ext.define('PBS.TapeManagement.DataStoreMappingGrid', {
>> alias: 'widget.pbsDataStoreMappingField',
>> mixins: ['Ext.form.field.Field'],
>>
>> + maxHeight: 150,
>> + scrollable: true,
>> + margin: '0 0 10 0',
>> +
>> getValue: function() {
>> let me = this;
>> let datastores = [];
>> @@ -308,3 +336,106 @@ Ext.define('PBS.TapeManagement.DataStoreMappingGrid', {
>> },
>> ],
>> });
>> +
>> +Ext.define('PBS.TapeManagement.SnapshotGrid', {
>> + extend: 'Ext.grid.Panel',
>> + alias: 'widget.pbsTapeSnapshotGrid',
>> + mixins: ['Ext.form.field.Field'],
>> +
>> + getValue: function() {
>> + let me = this;
>> + let snapshots = [];
>> +
>> + me.getStore().each((rec) => {
>> + if (rec.data.include) {
>> + let store = rec.data.store;
>> + let snap = rec.data.snapshot;
>> + snapshots.push(`${store}:${snap}`);
>> + }
>> + });
>> +
>> + return snapshots;
>> + },
>> +
>> + setValue: function(value) {
>> + let me = this;
>> + // not implemented
>> + return me;
>> + },
>> +
>> + getErrors: function(value) {
>> + let me = this;
>> + let firstSelected = me.getStore().findBy((rec) => !!rec.data.include);
>> +
>> + if (firstSelected === -1) {
>> + me.addCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
>> + let errorMsg = gettext("Need at least one snapshot");
>> + me.getActionEl().dom.setAttribute('data-errorqtip', errorMsg);
>> +
>> + return [errorMsg];
>> + }
>> + me.removeCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
>> + me.getActionEl().dom.setAttribute('data-errorqtip', "");
>> + return [];
>> + },
>> +
>> + scrollable: true,
>> + height: 200,
>> +
>> + viewConfig: {
>> + emptyText: gettext('No Snapshots'),
>> + markDirty: false,
>> + },
>> +
>> + setFilter: function(value) {
>> + let me = this;
>> + me.down('textfield').setValue(value);
>> + },
>> +
>> + tbar: [
>> + {
>> + fieldLabel: gettext('Filter'),
>> + xtype: 'textfield',
>> + isFormField: false,
>> + listeners: {
>> + change: function(field, value) {
>> + let me = this;
>> + let grid = me.up('grid');
>> + let store = grid.getStore();
>> + store.clearFilter();
>> + store.filterBy((rec) => {
>> + let restoreid = `${rec.data.store}:${rec.data.snapshot}`;
>> + return restoreid.indexOf(value) !== -1;
>> + });
>> + grid.checkChange();
>> + },
>> + },
>> + },
>> + ],
>> +
>> + store: { data: [] },
>> +
>> + columns: [
>> + {
>> + xtype: 'checkcolumn',
>> + text: gettext('Include'),
>> + dataIndex: 'include',
>> + listeners: {
>> + checkchange: function(cb, value) {
>> + let grid = this.up('grid');
>> + grid.checkChange();
>> + },
>> + },
>> + },
>> + {
>> + text: gettext('Source Datastore'),
>> + dataIndex: 'store',
>> + flex: 1,
>> + },
>> + {
>> + text: gettext('Snapshot'),
>> + dataIndex: 'snapshot',
>> + flex: 4,
>> + },
>> + ],
>> +});
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v2] ui: tape/BackupOverview: unify and improve tape restore window
2021-05-14 6:29 ` Dominik Csapak
@ 2021-05-14 6:37 ` Thomas Lamprecht
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2021-05-14 6:37 UTC (permalink / raw)
To: Dominik Csapak, Proxmox Backup Server development discussion
On 14.05.21 08:29, Dominik Csapak wrote:
> On 5/12/21 21:34, Thomas Lamprecht wrote:
>> On 12.05.21 13:49, Dominik Csapak wrote:
>>> adds a snapshot list to the restore window where the user can
>>> select distinct snapshots to restore
>>>
>>> this also replaces the restore / restore single button by
>>> inline actions for the media-set, backup groups and snapshots
>>>
>>> when clicking the action for a group/snapshot, the snapshotselector
>>> gets activated and preselected/filtered so that it includes those
>>> snapshots (can be overridden from the user)
>>>
>>> this change means that we now load the media set everytime we open
>>> the window, since we want to have the list of snapshots.
>>> (we could build that from the tree again, or hold it there somewhere as
>>> list, but i do not think we gain much with that, and it simplifies the
>>> datastore selection code a bit)
>>>
>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>> ---
>>> changes from v1:
>>> * fixed snapshot selection checkbox (was in wrong context, looup needs
>>> to happen on the referenceHolder)
>>> * removed unnecessary 'node' variable in 'restore' function
>>> * fixed 'media-set' text in window (was the text of the node we clicked on)
>>>
>>
>> Recently it seem to me like we alternate between "place every new function and
>> it's use in 42 separate patches" and "dump^W squash three-zillion changes into
>> one" like a metronome, none of those is ideal IMO.
>>
>> IOW, this really needs to be two patches, one for the content overview changes
>> and the restore widow.
>
> ok will do
>
>>
>> Also, from my last reply to the v1 thread:
>>
>>> IMO slightly better than radio-group/checkbox as it removes a UI control
>>> element completely, so less choice/confusion, but without scarifying flexibility,
>>> so IMO a good idea.
>>
>> Now you added the extra checkbox... Why not just use the grid-header checkbox like
>> PVE backup-job edit-window and tick it by default when restoring a whole set?
>>
>> As said, reduction by one knob without losing any flexibility - seems like a win
>> win to me, or do I overlook something?
>>
>> With the list below the window's aspect ration feels wrong and the list a bit
>> squished, I'd add a few 100s px to the window height to improve that.
>>
>
> ok this was a misunderstanding then...
>
> yeah using the 'check all' checkbox makes sense, but then i am unsure
> how to handle filtering.
>
> now only visible entries get restored, so if a user would (with the check all) filter, it would seem like still all are selected and
> we generate wrong assumptions?
> we *should* be able to uncheck the check all box though to reduce
> confusion, and this seems to me the best way forward.
>
not sure this is a problem, in the search functionality for the PMG spam
quaratine the not visible entries just get deselected, if the filter is
cleared then nothing needs to be done, at all time the only things
actually selected must be visible (out ov view due to long list naturally
not counted)
> the alternative is that we restore *all* selected entries, filtered or not, but this can be similarly confusing since things get restored that
> weren't visible
>
I'm a bit confused here, I do not see the problem? Filtering deselects items
which got filtered out, and the rest then falls in place.
> in pve's 'mass *' windows we have a behaviour like the first suggestion,
> but i just noticed, depending on which column gets filtered, the
> 'check all' box sometimes stays checked and sometimes not...
>
> as for the height, if there are multiple datastores on the media-set
> (idk if you tested with this), there is another grid for the
> datastore mapping, with that the window is already quite tall
>
> maybe a multi-step 'wizard' would be better?
that or move one grid to a side.
>
> let the user first select the basic choices (drive,user,etc.)
> then a window with a full-size grid of the snapshots, and then
> depending on what he selected, either a target datastore
> or datastore mapping input?
>
> (or the snapshots first, and the rest of the info on the second panel?)
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-14 6:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-12 11:49 [pbs-devel] [PATCH proxmox-backup v2] ui: tape/BackupOverview: unify and improve tape restore window Dominik Csapak
2021-05-12 19:34 ` Thomas Lamprecht
2021-05-14 6:29 ` Dominik Csapak
2021-05-14 6:37 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox