[pve-devel] [PATCH manager] fix 1928: ct snapshot: changed rollback message to warning

Tim Marx t.marx at proxmox.com
Fri Nov 16 10:26:22 CET 2018



> Thomas Lamprecht <t.lamprecht at proxmox.com> hat am 16. November 2018 um 08:30 geschrieben:
> 
> 
> On 11/13/18 1:24 PM, Tim Marx wrote:
> > Signed-off-by: Tim Marx <t.marx at proxmox.com>
> 
> I find the result quite nice! Some higher level notes inline.
> 
> > ---
> >  www/manager6/lxc/SnapshotTree.js | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/www/manager6/lxc/SnapshotTree.js b/www/manager6/lxc/SnapshotTree.js
> > index 4996e2e3..8d76f13c 100644
> > --- a/www/manager6/lxc/SnapshotTree.js
> > +++ b/www/manager6/lxc/SnapshotTree.js
> > @@ -156,11 +156,18 @@ Ext.define('PVE.lxc.SnapshotTree', {
> >  	var rollbackBtn = new Proxmox.button.Button({
> >  	    text: gettext('Rollback'),
> >  	    disabled: true,
> > +	    dangerous: true,
> >  	    selModel: sm,
> >  	    enableFn: valid_snapshot_rollback,
> >  	    confirmMsg: function(rec) {
> > -		return Proxmox.Utils.format_task_description('vzrollback', me.vmid) +
> > -		    " '" +  rec.data.name + "'";
> > +		var taskdescription = Proxmox.Utils.format_task_description('vzrollback', me.vmid);
> > +		var snaptime = Ext.Date.format(rec.data.snaptime,'Y-m-d H:i:s');
> > +		var snapname = rec.data.name;
> > +		var msg = Ext.String.format(gettext('{0} to {1} - {2}'+
> 
> I'd mabye transform this into '{0} to {1} ({2})' (0 the same, 1 is snapname, 2 is date)
> and use it's own gettext for this. But no strong feelings on the format, yours is OK too.
> 

I will send a v2 anyway, because of your other comment.
Will swap it, but actually no strong feelings either.

> > +		'<p>Note: Data between now and latest snapshot will be lost,
'+
> > +		'running CT will be suspended for rollback!</p>'),
> 
> Maybe a question like:
> 
> 'Suspend CT for rollback?' would be enough here.. less to translate and a question may
> bring a bit more sense to the yes/no choices. What do you think?
> 

Wouldn't that make it a little bit like: 
"If you press no we will rollback the container without suspending it?" 
Would be nonsense and anyway results in no harm, but I'm just curios if someone not that experienced would be confused.
Maybe something like:
"Suspend needed, suspend CT now to perform rollback?"

Don't know if we should really omit the data loss part, compared to the inexperienced example before that would be much more relevant in my opinion.

But yeah, if no one else stand up for it, I will ditch it in favor for shortness.

Thanks for looking at this!


> > +		taskdescription, snaptime, snapname);
> > +		return msg;
> >  	    },
> >  	    handler: function(btn, event) {
> >  		var rec = sm.getSelection()[0];
> > 
>




More information about the pve-devel mailing list