public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] ui: tape/BackupOverview: add 'restore partial' button
@ 2021-05-11 12:42 Dominik Csapak
  2021-05-11 16:17 ` Thomas Lamprecht
  0 siblings, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2021-05-11 12:42 UTC (permalink / raw)
  To: pbs-devel

this opens the restore window, but with a snapshot selector, so that
the user can select the snapshots to restore

includes a textfield for basic filtering

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/tape/BackupOverview.js     |  31 ++++++++
 www/tape/window/TapeRestore.js | 125 +++++++++++++++++++++++++++++++++
 2 files changed, 156 insertions(+)

diff --git a/www/tape/BackupOverview.js b/www/tape/BackupOverview.js
index c028d58d..e039595d 100644
--- a/www/tape/BackupOverview.js
+++ b/www/tape/BackupOverview.js
@@ -48,6 +48,29 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
 	    }).show();
 	},
 
+	restoreList: 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'];
+	    Ext.create('PBS.TapeManagement.TapeRestoreWindow', {
+		mediaset,
+		uuid,
+		list_snapshots: true,
+		listeners: {
+		    destroy: function() {
+			me.reload();
+		    },
+		},
+	    }).show();
+	},
+
 	restore: function(button, record) {
 	    let me = this;
 	    let view = me.getView();
@@ -295,6 +318,14 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
 	    parentXType: 'treepanel',
 	    enableFn: (rec) => !!rec.data['media-set-uuid'],
 	},
+	{
+	    xtype: 'proxmoxButton',
+	    disabled: true,
+	    text: gettext('Restore partial Media Set'),
+	    handler: 'restoreList',
+	    parentXType: 'treepanel',
+	    enableFn: (rec) => !!rec.data['media-set-uuid'],
+	},
 	{
 	    xtype: 'proxmoxButton',
 	    disabled: true,
diff --git a/www/tape/window/TapeRestore.js b/www/tape/window/TapeRestore.js
index 7e4f5cae..560d4812 100644
--- a/www/tape/window/TapeRestore.js
+++ b/www/tape/window/TapeRestore.js
@@ -49,6 +49,10 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
 		    values.snapshots = me.up('window').list;
 		}
 
+		if (Ext.isString(values.snapshots)) {
+		    values.snapshots = values.snapshots.split(',');
+		}
+
 		values.store = datastores.join(',');
 
 		return values;
@@ -138,6 +142,23 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
 		    defaultBindProperty: 'value',
 		    hidden: true,
 		},
+		{
+		    fieldLabel: gettext('Snapshot Selection'),
+		    labelWidth: 200,
+		    cbind: {
+			hidden: '{!list_snapshots}',
+		    },
+		    reference: 'snapshotLabel',
+		    xtype: 'displayfield',
+		},
+		{
+		    xtype: 'pbsTapeSnapshotGrid',
+		    reference: 'snapshotGrid',
+		    name: 'snapshots',
+		    cbind: {
+			hidden: '{!list_snapshots}',
+		    },
+		},
 	    ],
 	},
     ],
@@ -186,6 +207,9 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
 			    datastores[content.store] = true;
 			}
 			me.setDataStores(Object.keys(datastores));
+			let store = me.lookup('snapshotGrid').getStore();
+			store.setData(response.result.data);
+			store.sort('snapshot');
 		    },
 		    failure: function() {
 			// ignore failing api call, maybe catalog is missing
@@ -308,3 +332,104 @@ 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,
+    },
+
+    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.filter({
+			property: 'snapshot',
+			anyMatch: true,
+			caseSensitive: true,
+			exactMatch: false,
+			value,
+		    });
+		    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] ui: tape/BackupOverview: add 'restore partial' button
  2021-05-11 12:42 [pbs-devel] [PATCH proxmox-backup] ui: tape/BackupOverview: add 'restore partial' button Dominik Csapak
@ 2021-05-11 16:17 ` Thomas Lamprecht
  2021-05-12  6:22   ` Dominik Csapak
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2021-05-11 16:17 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

On 11.05.21 14:42, Dominik Csapak wrote:
> this opens the restore window, but with a snapshot selector, so that
> the user can select the snapshots to restore
> 
> includes a textfield for basic filtering


why isn't that handled directly in the restore window, without extra buttons?
Could possibly be a radio group there.

I really want to avoid many buttons in places like top-bars, makes it crowded
and confusing..

Also, why doesn't this uses action buttons for those things like the content
tree? IMO we should try to be consistent with UX, cross-product this may be
hard and lots of work, but I do not see any excuse for intra-product consistency
(especially on rather simple UIs like PBS, compared to PVE).

> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/tape/BackupOverview.js     |  31 ++++++++
>  www/tape/window/TapeRestore.js | 125 +++++++++++++++++++++++++++++++++
>  2 files changed, 156 insertions(+)
> 
> diff --git a/www/tape/BackupOverview.js b/www/tape/BackupOverview.js
> index c028d58d..e039595d 100644
> --- a/www/tape/BackupOverview.js
> +++ b/www/tape/BackupOverview.js
> @@ -48,6 +48,29 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>  	    }).show();
>  	},
>  
> +	restoreList: 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'];
> +	    Ext.create('PBS.TapeManagement.TapeRestoreWindow', {
> +		mediaset,
> +		uuid,
> +		list_snapshots: true,
> +		listeners: {
> +		    destroy: function() {
> +			me.reload();
> +		    },
> +		},
> +	    }).show();
> +	},
> +
>  	restore: function(button, record) {
>  	    let me = this;
>  	    let view = me.getView();
> @@ -295,6 +318,14 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>  	    parentXType: 'treepanel',
>  	    enableFn: (rec) => !!rec.data['media-set-uuid'],
>  	},
> +	{
> +	    xtype: 'proxmoxButton',
> +	    disabled: true,
> +	    text: gettext('Restore partial Media Set'),

really long button text's should be avoided...

> +	    handler: 'restoreList',
> +	    parentXType: 'treepanel',
> +	    enableFn: (rec) => !!rec.data['media-set-uuid'],
> +	},
>  	{
>  	    xtype: 'proxmoxButton',
>  	    disabled: true,
> diff --git a/www/tape/window/TapeRestore.js b/www/tape/window/TapeRestore.js
> index 7e4f5cae..560d4812 100644
> --- a/www/tape/window/TapeRestore.js
> +++ b/www/tape/window/TapeRestore.js
> @@ -49,6 +49,10 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>  		    values.snapshots = me.up('window').list;
>  		}
>  
> +		if (Ext.isString(values.snapshots)) {
> +		    values.snapshots = values.snapshots.split(',');
> +		}
> +
>  		values.store = datastores.join(',');
>  
>  		return values;
> @@ -138,6 +142,23 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>  		    defaultBindProperty: 'value',
>  		    hidden: true,
>  		},
> +		{
> +		    fieldLabel: gettext('Snapshot Selection'),
> +		    labelWidth: 200,
> +		    cbind: {
> +			hidden: '{!list_snapshots}',
> +		    },
> +		    reference: 'snapshotLabel',
> +		    xtype: 'displayfield',
> +		},
> +		{
> +		    xtype: 'pbsTapeSnapshotGrid',
> +		    reference: 'snapshotGrid',
> +		    name: 'snapshots',
> +		    cbind: {
> +			hidden: '{!list_snapshots}',
> +		    },
> +		},
>  	    ],
>  	},
>      ],
> @@ -186,6 +207,9 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>  			    datastores[content.store] = true;
>  			}
>  			me.setDataStores(Object.keys(datastores));
> +			let store = me.lookup('snapshotGrid').getStore();
> +			store.setData(response.result.data);
> +			store.sort('snapshot');
>  		    },
>  		    failure: function() {
>  			// ignore failing api call, maybe catalog is missing
> @@ -308,3 +332,104 @@ 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,
> +    },
> +
> +    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.filter({
> +			property: 'snapshot',
> +			anyMatch: true,
> +			caseSensitive: true,
> +			exactMatch: false,
> +			value,
> +		    });
> +		    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] ui: tape/BackupOverview: add 'restore partial' button
  2021-05-11 16:17 ` Thomas Lamprecht
@ 2021-05-12  6:22   ` Dominik Csapak
  2021-05-12  6:44     ` Thomas Lamprecht
  0 siblings, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2021-05-12  6:22 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox Backup Server development discussion

On 5/11/21 18:17, Thomas Lamprecht wrote:
> On 11.05.21 14:42, Dominik Csapak wrote:
>> this opens the restore window, but with a snapshot selector, so that
>> the user can select the snapshots to restore
>>
>> includes a textfield for basic filtering
> 
> 
> why isn't that handled directly in the restore window, without extra buttons?
> Could possibly be a radio group there.

yes you're right, that'll much better, though i would not do a radio 
button, but maybe only a checkbox '[] Select snapshots' which
enables the selection grid?

we could then open that window with a preselected(and filtered?) list 
when the user clicks on the 'restore single snapshot' button?

does that make sense?

> 
> I really want to avoid many buttons in places like top-bars, makes it crowded
> and confusing..
> 
> Also, why doesn't this uses action buttons for those things like the content
> tree? IMO we should try to be consistent with UX, cross-product this may be
> hard and lots of work, but I do not see any excuse for intra-product consistency
> (especially on rather simple UIs like PBS, compared to PVE).

mhmm sounds good, i'd do an action column where each media set and
snapshot gets a restore button, and for single snapshots i preselect
like i mentioned above.. does that make sense?

(we could probably do a restore button on every level in the tree,
and preselect accordingly, though i am unsure if that does not
get too confusing and if its even helpful...)

> 
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   www/tape/BackupOverview.js     |  31 ++++++++
>>   www/tape/window/TapeRestore.js | 125 +++++++++++++++++++++++++++++++++
>>   2 files changed, 156 insertions(+)
>>
>> diff --git a/www/tape/BackupOverview.js b/www/tape/BackupOverview.js
>> index c028d58d..e039595d 100644
>> --- a/www/tape/BackupOverview.js
>> +++ b/www/tape/BackupOverview.js
>> @@ -48,6 +48,29 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>>   	    }).show();
>>   	},
>>   
>> +	restoreList: 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'];
>> +	    Ext.create('PBS.TapeManagement.TapeRestoreWindow', {
>> +		mediaset,
>> +		uuid,
>> +		list_snapshots: true,
>> +		listeners: {
>> +		    destroy: function() {
>> +			me.reload();
>> +		    },
>> +		},
>> +	    }).show();
>> +	},
>> +
>>   	restore: function(button, record) {
>>   	    let me = this;
>>   	    let view = me.getView();
>> @@ -295,6 +318,14 @@ Ext.define('PBS.TapeManagement.BackupOverview', {
>>   	    parentXType: 'treepanel',
>>   	    enableFn: (rec) => !!rec.data['media-set-uuid'],
>>   	},
>> +	{
>> +	    xtype: 'proxmoxButton',
>> +	    disabled: true,
>> +	    text: gettext('Restore partial Media Set'),
> 
> really long button text's should be avoided...
> 
>> +	    handler: 'restoreList',
>> +	    parentXType: 'treepanel',
>> +	    enableFn: (rec) => !!rec.data['media-set-uuid'],
>> +	},
>>   	{
>>   	    xtype: 'proxmoxButton',
>>   	    disabled: true,
>> diff --git a/www/tape/window/TapeRestore.js b/www/tape/window/TapeRestore.js
>> index 7e4f5cae..560d4812 100644
>> --- a/www/tape/window/TapeRestore.js
>> +++ b/www/tape/window/TapeRestore.js
>> @@ -49,6 +49,10 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>>   		    values.snapshots = me.up('window').list;
>>   		}
>>   
>> +		if (Ext.isString(values.snapshots)) {
>> +		    values.snapshots = values.snapshots.split(',');
>> +		}
>> +
>>   		values.store = datastores.join(',');
>>   
>>   		return values;
>> @@ -138,6 +142,23 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>>   		    defaultBindProperty: 'value',
>>   		    hidden: true,
>>   		},
>> +		{
>> +		    fieldLabel: gettext('Snapshot Selection'),
>> +		    labelWidth: 200,
>> +		    cbind: {
>> +			hidden: '{!list_snapshots}',
>> +		    },
>> +		    reference: 'snapshotLabel',
>> +		    xtype: 'displayfield',
>> +		},
>> +		{
>> +		    xtype: 'pbsTapeSnapshotGrid',
>> +		    reference: 'snapshotGrid',
>> +		    name: 'snapshots',
>> +		    cbind: {
>> +			hidden: '{!list_snapshots}',
>> +		    },
>> +		},
>>   	    ],
>>   	},
>>       ],
>> @@ -186,6 +207,9 @@ Ext.define('PBS.TapeManagement.TapeRestoreWindow', {
>>   			    datastores[content.store] = true;
>>   			}
>>   			me.setDataStores(Object.keys(datastores));
>> +			let store = me.lookup('snapshotGrid').getStore();
>> +			store.setData(response.result.data);
>> +			store.sort('snapshot');
>>   		    },
>>   		    failure: function() {
>>   			// ignore failing api call, maybe catalog is missing
>> @@ -308,3 +332,104 @@ 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,
>> +    },
>> +
>> +    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.filter({
>> +			property: 'snapshot',
>> +			anyMatch: true,
>> +			caseSensitive: true,
>> +			exactMatch: false,
>> +			value,
>> +		    });
>> +		    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] ui: tape/BackupOverview: add 'restore partial' button
  2021-05-12  6:22   ` Dominik Csapak
@ 2021-05-12  6:44     ` Thomas Lamprecht
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2021-05-12  6:44 UTC (permalink / raw)
  To: Dominik Csapak, Proxmox Backup Server development discussion

On 12.05.21 08:22, Dominik Csapak wrote:
> On 5/11/21 18:17, Thomas Lamprecht wrote:
>> On 11.05.21 14:42, Dominik Csapak wrote:
>>> this opens the restore window, but with a snapshot selector, so that
>>> the user can select the snapshots to restore
>>>
>>> includes a textfield for basic filtering
>>
>>
>> why isn't that handled directly in the restore window, without extra buttons?
>> Could possibly be a radio group there.
> 
> yes you're right, that'll much better, though i would not do a radio button, but maybe only a checkbox '[] Select snapshots' which
> enables the selection grid?
> 
> we could then open that window with a preselected(and filtered?) list when the user clicks on the 'restore single snapshot' button?
> 
> does that make sense?

yes, can make sense - would be related to the backup-job edit window in PVE.

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.


>>
>> I really want to avoid many buttons in places like top-bars, makes it crowded
>> and confusing..
>>
>> Also, why doesn't this uses action buttons for those things like the content
>> tree? IMO we should try to be consistent with UX, cross-product this may be
>> hard and lots of work, but I do not see any excuse for intra-product consistency
>> (especially on rather simple UIs like PBS, compared to PVE).
> 
> mhmm sounds good, i'd do an action column where each media set and
> snapshot gets a restore button, and for single snapshots i preselect
> like i mentioned above.. does that make sense?
> 

yeah, if the others are still shown for the single snapshot I'd try to scroll
it in view or order it first though, so its immediately visible. We could even
order selected generally first by default, just an idea though...

> (we could probably do a restore button on every level in the tree,
> and preselect accordingly, though i am unsure if that does not
> get too confusing and if its even helpful...)
> 






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

end of thread, other threads:[~2021-05-12  6:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-11 12:42 [pbs-devel] [PATCH proxmox-backup] ui: tape/BackupOverview: add 'restore partial' button Dominik Csapak
2021-05-11 16:17 ` Thomas Lamprecht
2021-05-12  6:22   ` Dominik Csapak
2021-05-12  6:44     ` 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