[pve-devel] [PATCH manager v2 1/2] ui: dc: backup: improve UX for the different 'notification-mode's

[pve-devel] [PATCH manager v2 1/2] ui: dc: backup: improve UX for the different 'notification-mode's

[Lukas Wagner]

  - Switch order of 'mailto' and 'mailnotification' field
  - When mode is 'auto', disable 'mailtnotification' field
  - When mode is 'auto' and 'mailto' is empty, show
    hint that the notification system will be used

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

Notes:
    Changes since v1:
      - Set 'mailNotification' in viewModel to 'always'
        to  avoid the combobox being empty initially

 www/manager6/dc/Backup.js | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 70903bdc..70b5604e 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -207,13 +207,25 @@ Ext.define('PVE.dc.BackupEdit', {
 	data: {
 	    selMode: 'include',
 	    notificationMode: '__default__',
+	    mailto: '',
+	    mailNotification: 'always',
 	},
 
 	formulas: {
 	    poolMode: (get) => get('selMode') === 'pool',
-	    disableVMSelection: (get) => get('selMode') !== 'include' && get('selMode') !== 'exclude',
+	    disableVMSelection: (get) => get('selMode') !== 'include' &&
+		get('selMode') !== 'exclude',
 	    showMailtoFields: (get) =>
 		['auto', 'legacy-sendmail', '__default__'].includes(get('notificationMode')),
+	    enableMailnotificationField: (get) => {
+		let mode = get('notificationMode');
+		let mailto = get('mailto');
+
+		return (['auto', '__default__'].includes(mode) && mailto) ||
+		    mode === 'legacy-sendmail';
+	    },
+	    hintTextVisible: (get) =>
+		['auto', '__default__'].includes(get('notificationMode')) && !get('mailto'),
 	},
     },
 
@@ -325,6 +337,15 @@ Ext.define('PVE.dc.BackupEdit', {
 					value: '{notificationMode}',
 				    },
 				},
+				{
+				    xtype: 'textfield',
+				    fieldLabel: gettext('Send email to'),
+				    name: 'mailto',
+				    bind: {
+					hidden: '{!showMailtoFields}',
+					value: '{mailto}',
+				    },
+				},
 				{
 				    xtype: 'pveEmailNotificationSelector',
 				    fieldLabel: gettext('Send email'),
@@ -334,15 +355,18 @@ Ext.define('PVE.dc.BackupEdit', {
 					deleteEmpty: '{!isCreate}',
 				    },
 				    bind: {
-					disabled: '{!showMailtoFields}',
+					hidden: '{!showMailtoFields}',
+					disabled: '{!enableMailnotificationField}',
+					value: '{mailNotification}',
 				    },
 				},
 				{
-				    xtype: 'textfield',
-				    fieldLabel: gettext('Send email to'),
-				    name: 'mailto',
+				    xtype: 'displayfield',
+				    userCls: 'pmx-hint',
+				    hidden: true,
+				    value: gettext('No email configured, the notification system will be used'),
 				    bind: {
-					disabled: '{!showMailtoFields}',
+					hidden: '{!hintTextVisible}',
 				    },
 				},
 				{
-- 
2.39.2

[pve-devel] [PATCH manager v2 2/2] ui: one-off backup: show hint if notification-system is used

[Lukas Wagner]

When mode is 'auto' and 'mailto' is empty, show hint that the
notification system will be used.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 www/manager6/window/Backup.js | 37 +++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/www/manager6/window/Backup.js b/www/manager6/window/Backup.js
index 4418a9c7..ce679971 100644
--- a/www/manager6/window/Backup.js
+++ b/www/manager6/window/Backup.js
@@ -30,10 +30,27 @@ Ext.define('PVE.window.Backup', {
 	    name: 'mode',
 	});
 
+	let viewModel = new Ext.app.ViewModel({
+	    formulas: {
+		showMailtoField: (get) =>
+		    ['auto', 'legacy-sendmail'].includes(get('notificationMode')),
+		hintTextVisible: (get) => get('notificationMode') === 'auto' && !get('mailto'),
+	    },
+	    data: {
+		mailto: '',
+		notificationMode: 'auto',
+	    },
+	});
+
 	let mailtoField = Ext.create('Ext.form.field.Text', {
 	    fieldLabel: gettext('Send email to'),
 	    name: 'mailto',
 	    emptyText: Proxmox.Utils.noneText,
+	    viewModel,
+	    bind: {
+		value: '{mailto}',
+		hidden: '{!showMailtoField}',
+	    },
 	});
 
 	let notificationModeSelector = Ext.create({
@@ -46,10 +63,21 @@ Ext.define('PVE.window.Backup', {
 	    fieldLabel: gettext('Notification mode'),
 	    name: 'notification-mode',
 	    value: 'auto',
-	    listeners: {
-		change: function(field, value) {
-		    mailtoField.setDisabled(value === 'notification-system');
-		},
+	    viewModel,
+	    bind: {
+		value: '{notificationMode}',
+	    },
+	});
+
+	let notificationSystemHint = Ext.create({
+	    xtype: 'displayfield',
+	    padding: '0 0 0 5',
+	    userCls: 'pmx-hint',
+	    hidden: true,
+	    value: gettext('No email configured, the notification system will be used'),
+	    viewModel,
+	    bind: {
+		hidden: '{!hintTextVisible}',
 	    },
 	});
 
@@ -198,6 +226,7 @@ Ext.define('PVE.window.Backup', {
 		compressionSelector,
 		notificationModeSelector,
 		mailtoField,
+		notificationSystemHint,
 		removeCheckbox,
 	    ],
 	    columnB: [
-- 
2.39.2

[pve-devel] partially-applied: [PATCH manager v2 1/2] ui: dc: backup: improve UX for the different 'notification-mode's

[Thomas Lamprecht]

Am 15/04/2024 um 11:51 schrieb Lukas Wagner:
>   - Switch order of 'mailto' and 'mailnotification' field
>   - When mode is 'auto', disable 'mailtnotification' field
>   - When mode is 'auto' and 'mailto' is empty, show
>     hint that the notification system will be used

If one starts making lists about different things a commit does, it might
be good to evaluate if it should be split into multiple commits..

I dropped the hint part, this is IMO really not better than nothing in
its current state, as besides being quite flashy it doesn't really
explains what's going on.

Now with the advanced tab we could move the extra fields in there, add a
extended explanation potentially even providing a link to the local docs'
notification section (or wherever this is explained in detail in the
docs).

As compromise for now I'd accept that text without the pmx-hint class and
a link to the docs added in the text (could also be just a question mark
icon at the end, i.e. the help button without a text label).


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel