[pbs-devel] [PATCH proxmox-backup v2 0/5] fix #3067: add notes functionality to webui

[pbs-devel] [PATCH proxmox-backup v2 0/5] fix #3067: add notes functionality to webui

[Stefan Sterz]

This series adds support for comments/notes to a PBS node similar to
PVE's datacenter or node notes. It's split in two parts, the first two
commits add support for a single line comment, similar to the comments
already available for datastores. The next three commits refactor this
into Markdown-based multi-line comments and add an additional tab to
the UI to provide more space. If prefered, I can squash the commits.

changes v2 (thanks @ Wolfgang Bumiller):

 * performance improvements when parsing/writing a node configuration
 * adjusted multi-line regex to remove superfluous "\s*"
 * better formatting of rust code

Stefan Sterz (5):
  fix #3067: api: add support for a comment field in node.cfg
  fix #3067: pbs ui: add support for a notes field in the dashboard
  fix #3067: api: add multi-line comments to node.cfg
  fix #3607: ui: make dashboard notes markdown capable
  fix #3607: ui: add a separate notes view for longer markdown notes

 pbs-api-types/src/lib.rs          |   9 ++
 src/api2/node/config.rs           |   4 +
 src/config/node.rs                |  14 ++-
 src/tools/config.rs               |  58 +++++++++++-
 www/Dashboard.js                  | 110 +++++++++++++---------
 www/Makefile                      |   4 +-
 www/NavigationTree.js             |   6 ++
 www/NodeNotes.js                  |  22 +++++
 www/datastore/Summary.js          |   6 +-
 www/panel/MarkdownNotes.js        | 151 ++++++++++++++++++++++++++++++
 www/{datastore => panel}/Notes.js |  19 +++-
 11 files changed, 348 insertions(+), 55 deletions(-)
 create mode 100644 www/NodeNotes.js
 create mode 100644 www/panel/MarkdownNotes.js
 rename www/{datastore => panel}/Notes.js (81%)

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 1/5] fix #3067: api: add support for a comment field in node.cfg

[Stefan Sterz]

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 src/api2/node/config.rs |  4 ++++
 src/config/node.rs      | 11 ++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs
index 0a119354..77fe69fe 100644
--- a/src/api2/node/config.rs
+++ b/src/api2/node/config.rs
@@ -64,6 +64,8 @@ pub enum DeletableProperty {
     ciphers_tls_1_2,
     /// Delete the default-lang property.
     default_lang,
+    /// Delete any comment
+    comment,
 }
 
 #[api(
@@ -124,6 +126,7 @@ pub fn update_node_config(
                 DeletableProperty::ciphers_tls_1_3 => { config.ciphers_tls_1_3 = None; },
                 DeletableProperty::ciphers_tls_1_2 => { config.ciphers_tls_1_2 = None; },
                 DeletableProperty::default_lang => { config.default_lang = None; },
+                DeletableProperty::comment => { config.comment = None; },
             }
         }
     }
@@ -139,6 +142,7 @@ pub fn update_node_config(
     if update.ciphers_tls_1_3.is_some() { config.ciphers_tls_1_3 = update.ciphers_tls_1_3; }
     if update.ciphers_tls_1_2.is_some() { config.ciphers_tls_1_2 = update.ciphers_tls_1_2; }
     if update.default_lang.is_some() { config.default_lang = update.default_lang; }
+    if update.comment.is_some() { config.comment = update.comment; }
 
     crate::config::node::save_config(&config)?;
 
diff --git a/src/config/node.rs b/src/config/node.rs
index 0ba87450..9ca44a52 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -8,7 +8,8 @@ use proxmox_schema::{api, ApiStringFormat, ApiType, Updater};
 
 use proxmox_http::ProxyConfig;
 
-use pbs_api_types::{EMAIL_SCHEMA, OPENSSL_CIPHERS_TLS_1_2_SCHEMA, OPENSSL_CIPHERS_TLS_1_3_SCHEMA};
+use pbs_api_types::{EMAIL_SCHEMA, OPENSSL_CIPHERS_TLS_1_2_SCHEMA, OPENSSL_CIPHERS_TLS_1_3_SCHEMA,
+    SINGLE_LINE_COMMENT_SCHEMA};
 use pbs_buildcfg::configdir;
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
@@ -167,6 +168,10 @@ pub enum Translation {
         "default-lang" : {
             schema: Translation::API_SCHEMA,
             optional: true,
+        },
+        "comment" : {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
         }
     },
 )]
@@ -210,6 +215,10 @@ pub struct NodeConfig {
     /// Default language used in the GUI
     #[serde(skip_serializing_if = "Option::is_none")]
     pub default_lang: Option<String>,
+
+    /// Dashboard comment
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
 }
 
 impl NodeConfig {
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard

[Stefan Sterz]

adds a panel to the dashboard displaying a comment similar to the
comments panel in a datastore summary

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 www/Dashboard.js                  | 110 ++++++++++++++++++------------
 www/Makefile                      |   2 +-
 www/datastore/Summary.js          |   6 +-
 www/{datastore => panel}/Notes.js |  19 +++++-
 4 files changed, 85 insertions(+), 52 deletions(-)
 rename www/{datastore => panel}/Notes.js (81%)

diff --git a/www/Dashboard.js b/www/Dashboard.js
index 70c2305b..a78ad375 100644
--- a/www/Dashboard.js
+++ b/www/Dashboard.js
@@ -122,7 +122,7 @@ Ext.define('PBS.Dashboard', {
 		if (key !== 'summarycolumns') {
 		    return;
 		}
-		Proxmox.Utils.updateColumns(view);
+		Proxmox.Utils.updateColumnWidth(view);
 	    });
 	},
     },
@@ -192,26 +192,14 @@ Ext.define('PBS.Dashboard', {
 
     listeners: {
 	resize: function(panel) {
-	    Proxmox.Utils.updateColumns(panel);
+	    panel.query('>').forEach(c => Proxmox.Utils.updateColumnWidth(c));
 	},
     },
 
     title: gettext('Dashboard'),
 
-    layout: {
-	type: 'column',
-    },
-
     bodyPadding: '20 0 0 20',
 
-    minWidth: 700,
-
-    defaults: {
-	columnWidth: 0.49,
-	xtype: 'panel',
-	margin: '0 20 20 0',
-    },
-
     tools: [
 	{
 	    type: 'gear',
@@ -224,42 +212,74 @@ Ext.define('PBS.Dashboard', {
 
     items: [
 	{
-	    xtype: 'pbsNodeInfoPanel',
-	    reference: 'nodeInfo',
-	    height: 280,
-	},
-	{
-	    xtype: 'pbsDatastoresStatistics',
+	    xtype: 'container',
+	    layout: {
+		type: 'hbox',
+		align: 'stretch',
+	    },
+	    margin: '0 0 20 0',
+	    padding: '0 20 0 0',
 	    height: 280,
-	},
-	{
-	    xtype: 'pbsLongestTasks',
-	    bind: {
-		title: gettext('Longest Tasks') + ' (' +
-		Ext.String.format(gettext('{0} days'), '{days}') + ')',
+	    defaults: {
+		flex: 1,
 	    },
-	    reference: 'longesttasks',
-	    height: 250,
-	},
-	{
-	    xtype: 'pbsRunningTasks',
-	    height: 250,
-	},
-	{
-	    bind: {
-		title: gettext('Task Summary') + ' (' +
-		Ext.String.format(gettext('{0} days'), '{days}') + ')',
+	    minWidth: 700,
+	    items: [{
+		xtype: 'pbsNodeInfoPanel',
+		reference: 'nodeInfo',
+		margin: '0 20 0 0',
 	    },
-	    xtype: 'pbsTaskSummary',
-	    height: 200,
-	    reference: 'tasksummary',
+	    {
+		xtype: 'pbsNotes',
+		reference: 'nodeNotes',
+		node: 'localhost',
+		loadOnInit: true,
+	    }],
 	},
 	{
-	    iconCls: 'fa fa-ticket',
-	    title: 'Subscription',
-	    height: 200,
-	    reference: 'subscription',
-	    xtype: 'pbsSubscriptionInfo',
+	    xtype: 'container',
+	    layout: {
+		type: 'column',
+	    },
+	    minWidth: 700,
+	    defaults: {
+		columnWidth: 0.5,
+		xtype: 'panel',
+		margin: '0 20 20 0',
+	    },
+	    items: [{
+		xtype: 'pbsDatastoresStatistics',
+		height: 250,
+	    },
+	    {
+		xtype: 'pbsLongestTasks',
+		bind: {
+		    title: gettext('Longest Tasks') + ' (' +
+		    Ext.String.format(gettext('{0} days'), '{days}') + ')',
+		},
+		reference: 'longesttasks',
+		height: 250,
+	    },
+	    {
+		xtype: 'pbsRunningTasks',
+		height: 250,
+	    },
+	    {
+		bind: {
+		    title: gettext('Task Summary') + ' (' +
+		    Ext.String.format(gettext('{0} days'), '{days}') + ')',
+		},
+		xtype: 'pbsTaskSummary',
+		height: 250,
+		reference: 'tasksummary',
+	    },
+	    {
+		iconCls: 'fa fa-ticket',
+		title: 'Subscription',
+		height: 200,
+		reference: 'subscription',
+		xtype: 'pbsSubscriptionInfo',
+	    }],
 	},
     ],
 });
diff --git a/www/Makefile b/www/Makefile
index 455fbeec..636d4a57 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -81,6 +81,7 @@ JSSRC=							\
 	panel/StorageAndDisks.js			\
 	panel/UsageChart.js				\
 	panel/NodeInfo.js				\
+	panel/Notes.js				    \
 	ZFSList.js					\
 	DirectoryList.js				\
 	LoginView.js					\
@@ -88,7 +89,6 @@ JSSRC=							\
 	SystemConfiguration.js				\
 	Subscription.js					\
 	datastore/Summary.js				\
-	datastore/Notes.js				\
 	datastore/PruneAndGC.js				\
 	datastore/Prune.js				\
 	datastore/Content.js				\
diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
index c3769257..d11646f0 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -206,7 +206,7 @@ Ext.define('PBS.DataStoreSummary', {
 		    },
 		},
 		{
-		    xtype: 'pbsDataStoreNotes',
+		    xtype: 'pbsNotes',
 		    flex: 1,
 		    cbind: {
 			datastore: '{datastore}',
@@ -278,14 +278,14 @@ Ext.define('PBS.DataStoreSummary', {
 	    success: function(response) {
 		let path = Ext.htmlEncode(response.result.data.path);
 		me.down('pbsDataStoreInfo').setTitle(`${me.datastore} (${path})`);
-		me.down('pbsDataStoreNotes').setNotes(response.result.data.comment);
+		me.down('pbsNotes').setNotes(response.result.data.comment);
 	    },
 	    failure: function(response) {
 		// fallback if e.g. we have no permissions to the config
 		let rec = Ext.getStore('pbs-datastore-list')
 		    .findRecord('store', me.datastore, 0, false, true, true);
 		if (rec) {
-		    me.down('pbsDataStoreNotes').setNotes(rec.data.comment || "");
+		    me.down('pbsNotes').setNotes(rec.data.comment || "");
 		}
 	    },
 	});
diff --git a/www/datastore/Notes.js b/www/panel/Notes.js
similarity index 81%
rename from www/datastore/Notes.js
rename to www/panel/Notes.js
index 2928b7ec..4128dd8b 100644
--- a/www/datastore/Notes.js
+++ b/www/panel/Notes.js
@@ -1,6 +1,6 @@
-Ext.define('PBS.DataStoreNotes', {
+Ext.define('PBS.panel.Notes', {
     extend: 'Ext.panel.Panel',
-    xtype: 'pbsDataStoreNotes',
+    xtype: 'pbsNotes',
     mixins: ['Proxmox.Mixin.CBind'],
 
     title: gettext("Comment"),
@@ -11,7 +11,16 @@ Ext.define('PBS.DataStoreNotes', {
 
     cbindData: function(initalConfig) {
 	let me = this;
-	me.url = `/api2/extjs/config/datastore/${me.datastore}`;
+
+	if (('node' in me && 'datastore' in me) ||
+	    (!('node' in me) && !('datastore' in me))) {
+	    throw 'either both a node and a datastore were given or neither. only provide one.';
+	} else if ('node' in me) {
+	    me.url = `/api2/extjs/nodes/${me.node}/config`;
+	} else {
+	    me.url = `/api2/extjs/config/datastore/${me.datastore}`;
+	}
+
 	return { };
     },
 
@@ -97,6 +106,10 @@ Ext.define('PBS.DataStoreNotes', {
 	let sp = Ext.state.Manager.getProvider();
 	me.collapseMode = sp.get('notes-collapse', 'never');
 
+	if (me.loadOnInit === true) {
+	    me.load();
+	}
+
 	if (me.collapseMode === 'auto') {
 	    me.setCollapsed(true);
 	}
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 3/5] fix #3067: api: add multi-line comments to node.cfg

[Stefan Sterz]

add support for multiline comments to node.cfg, similar to how pve
handles multi-line comments

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 pbs-api-types/src/lib.rs |  9 +++++++
 src/config/node.rs       |  9 ++++---
 src/tools/config.rs      | 58 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 754e7b22..3b1fad2c 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -137,6 +137,8 @@ const_regex! {
 
     pub SINGLE_LINE_COMMENT_REGEX = r"^[[:^cntrl:]]*$";
 
+    pub MULTI_LINE_COMMENT_REGEX = r"(?m)^([[:^cntrl:]]*)$";
+
     pub BACKUP_REPO_URL_REGEX = concat!(
         r"^^(?:(?:(",
         USER_ID_REGEX_STR!(), "|", APITOKEN_ID_REGEX_STR!(),
@@ -273,6 +275,13 @@ pub const SINGLE_LINE_COMMENT_SCHEMA: Schema = StringSchema::new("Comment (singl
     .format(&SINGLE_LINE_COMMENT_FORMAT)
     .schema();
 
+pub const MULTI_LINE_COMMENT_FORMAT: ApiStringFormat =
+    ApiStringFormat::Pattern(&MULTI_LINE_COMMENT_REGEX);
+
+pub const MULTI_LINE_COMMENT_SCHEMA: Schema = StringSchema::new("Comment (multiple lines).")
+    .format(&MULTI_LINE_COMMENT_FORMAT)
+    .schema();
+
 pub const SUBSCRIPTION_KEY_SCHEMA: Schema = StringSchema::new("Proxmox Backup Server subscription key.")
     .format(&SUBSCRIPTION_KEY_FORMAT)
     .min_length(15)
diff --git a/src/config/node.rs b/src/config/node.rs
index 9ca44a52..a2f759a8 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -8,8 +8,11 @@ use proxmox_schema::{api, ApiStringFormat, ApiType, Updater};
 
 use proxmox_http::ProxyConfig;
 
-use pbs_api_types::{EMAIL_SCHEMA, OPENSSL_CIPHERS_TLS_1_2_SCHEMA, OPENSSL_CIPHERS_TLS_1_3_SCHEMA,
-    SINGLE_LINE_COMMENT_SCHEMA};
+use pbs_api_types::{
+    EMAIL_SCHEMA, MULTI_LINE_COMMENT_SCHEMA, OPENSSL_CIPHERS_TLS_1_2_SCHEMA,
+    OPENSSL_CIPHERS_TLS_1_3_SCHEMA,
+};
+
 use pbs_buildcfg::configdir;
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
@@ -171,7 +174,7 @@ pub enum Translation {
         },
         "comment" : {
             optional: true,
-            schema: SINGLE_LINE_COMMENT_SCHEMA,
+            schema: MULTI_LINE_COMMENT_SCHEMA,
         }
     },
 )]
diff --git a/src/tools/config.rs b/src/tools/config.rs
index f666a8ab..5bdc18ef 100644
--- a/src/tools/config.rs
+++ b/src/tools/config.rs
@@ -31,8 +31,19 @@ pub fn value_from_str(input: &str, schema: &'static Schema) -> Result<Value, Err
     let schema = object_schema(schema)?;
 
     let mut config = Object::new();
+    let mut lines = input.lines().enumerate().peekable();
+    let mut comments = String::new();
 
-    for (lineno, line) in input.lines().enumerate() {
+    while let Some((_, line)) = lines.next_if(|(_, line)| line.starts_with('#')) {
+        comments.push_str(&line[1..]);
+        comments.push('\n');
+    }
+
+    if !comments.is_empty() {
+        config.insert("comment".to_string(), Value::String(comments));
+    }
+
+    for (lineno, line) in lines {
         let line = line.trim();
         if line.starts_with('#') || line.is_empty() {
             continue;
@@ -133,9 +144,17 @@ pub fn value_to_bytes(value: &Value, schema: &'static Schema) -> Result<Vec<u8>,
 
 /// Note: the object must have already been verified at this point.
 fn object_to_writer(output: &mut dyn Write, object: &Object) -> Result<(), Error> {
+    // special key `comment` for multi-line notes, must be written before everything else
+    if let Some(Value::String(comment)) = object.get("comment") {
+        for lines in comment.lines() {
+            writeln!(output, "#{}", lines)?;
+        }
+    }
+
     for (key, value) in object.iter() {
         match value {
-            Value::Null => continue, // delete this entry
+            _ if key == "comment" => continue, // skip comment as we handle it above
+            Value::Null => continue,           // delete this entry
             Value::Bool(v) => writeln!(output, "{}: {}", key, v)?,
             Value::String(v) => {
                 if v.as_bytes().contains(&b'\n') {
@@ -172,3 +191,38 @@ fn test() {
 
     assert_eq!(config, NODE_CONFIG.as_bytes());
 }
+
+#[test]
+fn test_with_comment() {
+    use proxmox_schema::ApiType;
+
+    // let's just reuse some schema we actually have available:
+    use crate::config::node::NodeConfig;
+
+    const NODE_INPUT: &str = "\
+        #this should\n\
+        #be included\n\
+        acme: account=pebble\n\
+        # this should not\n\
+        acmedomain0: test1.invalid.local,plugin=power\n\
+        acmedomain1: test2.invalid.local\n\
+    ";
+
+    const NODE_OUTPUT: &str = "\
+        #this should\n\
+        #be included\n\
+        acme: account=pebble\n\
+        acmedomain0: test1.invalid.local,plugin=power\n\
+        acmedomain1: test2.invalid.local\n\
+    ";
+
+    let data: NodeConfig = from_str(NODE_INPUT, &NodeConfig::API_SCHEMA)
+        .expect("failed to parse multi-line notes node config");
+
+    println!("{}", data.comment.as_ref().expect("no comment was parsed"));
+
+    let config = to_bytes(&data, &NodeConfig::API_SCHEMA)
+        .expect("failed to serialize multi-line notes node config");
+
+    assert_eq!(config, NODE_OUTPUT.as_bytes());
+}
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable

[Stefan Sterz]

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 www/Dashboard.js           |   2 +-
 www/Makefile               |   3 +-
 www/panel/MarkdownNotes.js | 134 +++++++++++++++++++++++++++++++++++++
 3 files changed, 137 insertions(+), 2 deletions(-)
 create mode 100644 www/panel/MarkdownNotes.js

diff --git a/www/Dashboard.js b/www/Dashboard.js
index a78ad375..a76660ff 100644
--- a/www/Dashboard.js
+++ b/www/Dashboard.js
@@ -230,7 +230,7 @@ Ext.define('PBS.Dashboard', {
 		margin: '0 20 0 0',
 	    },
 	    {
-		xtype: 'pbsNotes',
+		xtype: 'pbsMarkdownNotes',
 		reference: 'nodeNotes',
 		node: 'localhost',
 		loadOnInit: true,
diff --git a/www/Makefile b/www/Makefile
index 636d4a57..2d55d39d 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -81,7 +81,8 @@ JSSRC=							\
 	panel/StorageAndDisks.js			\
 	panel/UsageChart.js				\
 	panel/NodeInfo.js				\
-	panel/Notes.js				    \
+	panel/Notes.js				        \
+	panel/MarkdownNotes.js			        \
 	ZFSList.js					\
 	DirectoryList.js				\
 	LoginView.js					\
diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
new file mode 100644
index 00000000..83119d36
--- /dev/null
+++ b/www/panel/MarkdownNotes.js
@@ -0,0 +1,134 @@
+Ext.define('PBS.panel.MarkdownNotes', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pbsMarkdownNotes',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    title: gettext("Notes"),
+    bodyPadding: 10,
+    scrollable: true,
+    animCollapse: false,
+    maxLength: 64*1022,
+
+    cbindData: function(initalConfig) {
+	let me = this;
+
+	if (('node' in me && 'datastore' in me) ||
+	    (!('node' in me) && !('datastore' in me))) {
+	    throw 'either both a node and a datastore were given or neither. please provide one.';
+	} else if ('node' in me) {
+	    me.url = `/api2/extjs/nodes/${me.node}/config`;
+	} else {
+	    me.url = `/api2/extjs/config/datastore/${me.datastore}`;
+	}
+
+	return {};
+    },
+
+    run_editor: function() {
+	let me = this;
+	let win = Ext.create('Proxmox.window.Edit', {
+	    title: gettext('Notes'),
+	    width: 800,
+	    height: 600,
+	    resizable: true,
+	    layout: 'fit',
+	    defaultButton: undefined,
+	    setMaxLength: function(maxLength) {
+		let area = win.down('textarea[name="comment"]');
+		area.maxLength = maxLength;
+		area.validate();
+
+		return me;
+	    },
+	    items: {
+		xtype: 'textarea',
+		name: 'comment',
+		height: '100%',
+		value: '',
+		hideLabel: true,
+		emptyText: gettext('You can use Markdown for rich text formatting.'),
+		fieldStyle: {
+		    'white-space': 'pre-wrap',
+		    'font-family': 'monospace',
+		},
+	    },
+	    url: me.url,
+	    listeners: {
+		destroy: function() {
+		    me.load();
+		},
+	    },
+	}).show();
+	win.setMaxLength(me.maxLength);
+	win.load();
+    },
+
+    setNotes: function(value) {
+	let me = this;
+	var data = value || '';
+
+	let mdHtml = Proxmox.Markdown.parse(data);
+	me.update(mdHtml);
+
+	if (me.collapsible && me.collapseMode === 'auto') {
+	    me.setCollapsed(data === '');
+	}
+    },
+
+    load: function() {
+	var me = this;
+
+	Proxmox.Utils.API2Request({
+	    url: me.url,
+	    waitMsgTarget: me,
+	    failure: function(response, opts) {
+		Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+		me.setCollapsed(false);
+	    },
+	    success: function(response, opts) {
+		me.setNotes(response.result.data.comment);
+	    },
+	});
+    },
+
+    listeners: {
+	render: function(c) {
+	    var me = this;
+	    me.getEl().on('dblclick', me.run_editor, me);
+	},
+	afterlayout: function() {
+	    let me = this;
+	    if (me.collapsible && !me.getCollapsed() && me.collapseMode === 'always') {
+		me.setCollapsed(true);
+		me.collapseMode = ''; // only once, on initial load!
+	    }
+	},
+    },
+
+    tools: [{
+	type: 'gear',
+	handler: function() {
+	    this.up('panel').run_editor();
+	},
+    }],
+
+    collapsible: true,
+    collapseDirection: 'right',
+
+    initComponent: function() {
+	var me = this;
+
+	me.callParent();
+
+	let sp = Ext.state.Manager.getProvider();
+	me.collapseMode = sp.get('notes-collapse', 'never');
+
+	if (me.loadOnInit === true) {
+	    me.load();
+	}
+
+	if (me.collapseMode === 'auto') {
+	    me.setCollapsed(true);
+	}
+    },
+});
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 5/5] fix #3607: ui: add a separate notes view for longer markdown notes

[Stefan Sterz]

since markdown notes might be rather long having only the small panel
in the dashboard might not be sufficient. this commit adds a tab
similar to pve's datacenter or node notes.

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 www/Makefile               |  1 +
 www/NavigationTree.js      |  6 ++++++
 www/NodeNotes.js           | 22 ++++++++++++++++++++++
 www/panel/MarkdownNotes.js | 33 +++++++++++++++++++++++++--------
 4 files changed, 54 insertions(+), 8 deletions(-)
 create mode 100644 www/NodeNotes.js

diff --git a/www/Makefile b/www/Makefile
index 2d55d39d..f1c0f8bb 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -99,6 +99,7 @@ JSSRC=							\
 	datastore/DataStoreList.js			\
 	ServerStatus.js					\
 	ServerAdministration.js				\
+	NodeNotes.js				        \
 	Dashboard.js					\
 	${TAPE_UI_FILES}				\
 	NavigationTree.js				\
diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index 576d05ab..916582ef 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -32,6 +32,12 @@ Ext.define('PBS.store.NavigationStore', {
 		path: 'pbsDashboard',
 		leaf: true,
 	    },
+	    {
+		text: gettext('Notes'),
+		iconCls: 'fa fa-sticky-note-o',
+		path: 'pbsNodeNotes',
+		leaf: true,
+	    },
 	    {
 		text: gettext('Configuration'),
 		iconCls: 'fa fa-gears',
diff --git a/www/NodeNotes.js b/www/NodeNotes.js
new file mode 100644
index 00000000..9a0fa00c
--- /dev/null
+++ b/www/NodeNotes.js
@@ -0,0 +1,22 @@
+Ext.define('PBS.NodeNotes', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pbsNodeNotes',
+
+    scrollable: true,
+    layout: 'fit',
+
+    items: [
+	{
+	    xtype: 'container',
+	    layout: 'fit',
+	    items: [{
+		xtype: 'pbsMarkdownNotes',
+		tools: false,
+		border: false,
+		node: 'localhost',
+		loadOnInit: true,
+		enableTbar: true,
+	    }],
+	},
+    ],
+});
diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
index 83119d36..f522cdfd 100644
--- a/www/panel/MarkdownNotes.js
+++ b/www/panel/MarkdownNotes.js
@@ -112,23 +112,40 @@ Ext.define('PBS.panel.MarkdownNotes', {
 	},
     }],
 
-    collapsible: true,
-    collapseDirection: 'right',
+    tbar: {
+	itemId: 'tbar',
+	hidden: true,
+	items: [
+	    {
+		text: gettext('Edit'),
+		handler: function() {
+		    this.up('panel').run_editor();
+		},
+	    },
+	],
+    },
 
     initComponent: function() {
 	var me = this;
 
 	me.callParent();
 
-	let sp = Ext.state.Manager.getProvider();
-	me.collapseMode = sp.get('notes-collapse', 'never');
+	if (me.enableTbar === true) {
+	    me.down('#tbar').setVisible(true);
+	} else {
+	    me.setCollapsible(true);
+	    me.collapseDirection = 'right';
 
-	if (me.loadOnInit === true) {
-	    me.load();
+	    let sp = Ext.state.Manager.getProvider();
+	    me.collapseMode = sp.get('notes-collapse', 'never');
+
+	    if (me.collapseMode === 'auto') {
+		me.setCollapsed(true);
+	    }
 	}
 
-	if (me.collapseMode === 'auto') {
-	    me.setCollapsed(true);
+	if (me.loadOnInit === true) {
+	    me.load();
 	}
     },
 });
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable

[Dominik Csapak]

high level comment:

seems mostly copy&pasted, so would it not be better to try
to refactor the panels/windows from pve into widget toolkit?

otherwise comments inline

On 2/24/22 15:18, Stefan Sterz wrote:
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>   www/Dashboard.js           |   2 +-
>   www/Makefile               |   3 +-
>   www/panel/MarkdownNotes.js | 134 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 137 insertions(+), 2 deletions(-)
>   create mode 100644 www/panel/MarkdownNotes.js
> 
> diff --git a/www/Dashboard.js b/www/Dashboard.js
> index a78ad375..a76660ff 100644
> --- a/www/Dashboard.js
> +++ b/www/Dashboard.js
> @@ -230,7 +230,7 @@ Ext.define('PBS.Dashboard', {
>   		margin: '0 20 0 0',
>   	    },
>   	    {
> -		xtype: 'pbsNotes',
> +		xtype: 'pbsMarkdownNotes',

does that not make the changes to the 'pbsNotes' from 2/5 irrelevant ?

>   		reference: 'nodeNotes',
>   		node: 'localhost',
>   		loadOnInit: true,
> diff --git a/www/Makefile b/www/Makefile
> index 636d4a57..2d55d39d 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -81,7 +81,8 @@ JSSRC=							\
>   	panel/StorageAndDisks.js			\
>   	panel/UsageChart.js				\
>   	panel/NodeInfo.js				\
> -	panel/Notes.js				    \
> +	panel/Notes.js				        \
> +	panel/MarkdownNotes.js			        \
>   	ZFSList.js					\
>   	DirectoryList.js				\
>   	LoginView.js					\
> diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
> new file mode 100644
> index 00000000..83119d36
> --- /dev/null
> +++ b/www/panel/MarkdownNotes.js
> @@ -0,0 +1,134 @@
> +Ext.define('PBS.panel.MarkdownNotes', {
> +    extend: 'Ext.panel.Panel',
> +    xtype: 'pbsMarkdownNotes',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    title: gettext("Notes"),
> +    bodyPadding: 10,
> +    scrollable: true,
> +    animCollapse: false,
> +    maxLength: 64*1022,
> +
> +    cbindData: function(initalConfig) {
> +	let me = this;
> +
> +	if (('node' in me && 'datastore' in me) ||
> +	    (!('node' in me) && !('datastore' in me))) {
> +	    throw 'either both a node and a datastore were given or neither. please provide one.';
> +	} else if ('node' in me) {
> +	    me.url = `/api2/extjs/nodes/${me.node}/config`;
> +	} else {
> +	    me.url = `/api2/extjs/config/datastore/${me.datastore}`;
> +	}

same hints as in 2/5

> +
> +	return {};
> +    },
> +
> +    run_editor: function() {
> +	let me = this;
> +	let win = Ext.create('Proxmox.window.Edit', {
> +	    title: gettext('Notes'),
> +	    width: 800,
> +	    height: 600,
> +	    resizable: true,
> +	    layout: 'fit',
> +	    defaultButton: undefined,
> +	    setMaxLength: function(maxLength) {
> +		let area = win.down('textarea[name="comment"]');
> +		area.maxLength = maxLength;
> +		area.validate();
> +
> +		return me;

i don't understand that?
why not simply setting maxLength of the area down below?

> +	    },
> +	    items: {
> +		xtype: 'textarea',
> +		name: 'comment',
> +		height: '100%',
> +		value: '',
> +		hideLabel: true,
> +		emptyText: gettext('You can use Markdown for rich text formatting.'),
> +		fieldStyle: {
> +		    'white-space': 'pre-wrap',
> +		    'font-family': 'monospace',
> +		},
> +	    },
> +	    url: me.url,
> +	    listeners: {
> +		destroy: function() {
> +		    me.load();
> +		},
> +	    },
> +	}).show();
> +	win.setMaxLength(me.maxLength);
> +	win.load();

with the remark above and 'autoLoad' you can omit the whole 'win' variable

> +    },
> +
> +    setNotes: function(value) {
> +	let me = this;
> +	var data = value || '';
> +
> +	let mdHtml = Proxmox.Markdown.parse(data);
> +	me.update(mdHtml);
> +
> +	if (me.collapsible && me.collapseMode === 'auto') {
> +	    me.setCollapsed(data === '');
> +	}
> +    },
> +
> +    load: function() {
> +	var me = this;
> +
> +	Proxmox.Utils.API2Request({
> +	    url: me.url,
> +	    waitMsgTarget: me,
> +	    failure: function(response, opts) {
> +		Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> +		me.setCollapsed(false);
> +	    },
> +	    success: function(response, opts) {
> +		me.setNotes(response.result.data.comment);
> +	    },
> +	});
> +    },
> +
> +    listeners: {
> +	render: function(c) {
> +	    var me = this;
> +	    me.getEl().on('dblclick', me.run_editor, me);
> +	},
> +	afterlayout: function() {
> +	    let me = this;
> +	    if (me.collapsible && !me.getCollapsed() && me.collapseMode === 'always') {
> +		me.setCollapsed(true);
> +		me.collapseMode = ''; // only once, on initial load!
> +	    }
> +	},
> +    },
> +
> +    tools: [{
> +	type: 'gear',
> +	handler: function() {
> +	    this.up('panel').run_editor();
> +	},
> +    }],
> +
> +    collapsible: true,
> +    collapseDirection: 'right',
> +
> +    initComponent: function() {
> +	var me = this;
> +
> +	me.callParent();
> +
> +	let sp = Ext.state.Manager.getProvider();
> +	me.collapseMode = sp.get('notes-collapse', 'never');
> +
> +	if (me.loadOnInit === true) {
> +	    me.load();
> +	}
> +
> +	if (me.collapseMode === 'auto') {
> +	    me.setCollapsed(true);
> +	}
> +    },
> +});

Re: [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard

[Dominik Csapak]

high level comment:

i get what you tried to achieve here with the
container for info and notes, but i'd probably
simply add it as a regular panel.

that way, the user is not confused when the column selection
does not work the way he expects (e.g. setting it to 3 columns
still always has only the info/note as the first line)

so either a regular panel (without the collapsing feature, maybe
a browser storage option to hide it? like the days) or let
it behave like on the datastore summary (info+notes = 1 panel)

some comments inline

On 2/24/22 15:18, Stefan Sterz wrote:
> adds a panel to the dashboard displaying a comment similar to the
> comments panel in a datastore summary
> 
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>   www/Dashboard.js                  | 110 ++++++++++++++++++------------
>   www/Makefile                      |   2 +-
>   www/datastore/Summary.js          |   6 +-
>   www/{datastore => panel}/Notes.js |  19 +++++-
>   4 files changed, 85 insertions(+), 52 deletions(-)
>   rename www/{datastore => panel}/Notes.js (81%)
> 
> diff --git a/www/Dashboard.js b/www/Dashboard.js
> index 70c2305b..a78ad375 100644
> --- a/www/Dashboard.js
> +++ b/www/Dashboard.js
> @@ -122,7 +122,7 @@ Ext.define('PBS.Dashboard', {
>   		if (key !== 'summarycolumns') {
>   		    return;
>   		}
> -		Proxmox.Utils.updateColumns(view);
> +		Proxmox.Utils.updateColumnWidth(view);

this does not work like that? you'd have to
use the 'view.query...' expression like below else the
child panels do not get updated

>   	    });
>   	},
>       },
> @@ -192,26 +192,14 @@ Ext.define('PBS.Dashboard', {
>   
>       listeners: {
>   	resize: function(panel) {
> -	    Proxmox.Utils.updateColumns(panel);
> +	    panel.query('>').forEach(c => Proxmox.Utils.updateColumnWidth(c));
>   	},
>       },
>   
>       title: gettext('Dashboard'),
>   
> -    layout: {
> -	type: 'column',
> -    },
> -
>       bodyPadding: '20 0 0 20',
>   
> -    minWidth: 700,
> -
> -    defaults: {
> -	columnWidth: 0.49,
> -	xtype: 'panel',
> -	margin: '0 20 20 0',
> -    },
> -
>       tools: [
>   	{
>   	    type: 'gear',
> @@ -224,42 +212,74 @@ Ext.define('PBS.Dashboard', {
>   
>       items: [
>   	{
> -	    xtype: 'pbsNodeInfoPanel',
> -	    reference: 'nodeInfo',
> -	    height: 280,
> -	},
> -	{
> -	    xtype: 'pbsDatastoresStatistics',
> +	    xtype: 'container',
> +	    layout: {
> +		type: 'hbox',
> +		align: 'stretch',
> +	    },
> +	    margin: '0 0 20 0',
> +	    padding: '0 20 0 0',
>   	    height: 280,
> -	},
> -	{
> -	    xtype: 'pbsLongestTasks',
> -	    bind: {
> -		title: gettext('Longest Tasks') + ' (' +
> -		Ext.String.format(gettext('{0} days'), '{days}') + ')',
> +	    defaults: {
> +		flex: 1,
>   	    },
> -	    reference: 'longesttasks',
> -	    height: 250,
> -	},
> -	{
> -	    xtype: 'pbsRunningTasks',
> -	    height: 250,
> -	},
> -	{
> -	    bind: {
> -		title: gettext('Task Summary') + ' (' +
> -		Ext.String.format(gettext('{0} days'), '{days}') + ')',
> +	    minWidth: 700,
> +	    items: [{
> +		xtype: 'pbsNodeInfoPanel',
> +		reference: 'nodeInfo',
> +		margin: '0 20 0 0',
>   	    },
> -	    xtype: 'pbsTaskSummary',
> -	    height: 200,
> -	    reference: 'tasksummary',
> +	    {
> +		xtype: 'pbsNotes',
> +		reference: 'nodeNotes',
> +		node: 'localhost',
> +		loadOnInit: true,
> +	    }],
>   	},
>   	{
> -	    iconCls: 'fa fa-ticket',
> -	    title: 'Subscription',
> -	    height: 200,
> -	    reference: 'subscription',
> -	    xtype: 'pbsSubscriptionInfo',
> +	    xtype: 'container',
> +	    layout: {
> +		type: 'column',
> +	    },
> +	    minWidth: 700,
> +	    defaults: {
> +		columnWidth: 0.5,
> +		xtype: 'panel',
> +		margin: '0 20 20 0',
> +	    },
> +	    items: [{
> +		xtype: 'pbsDatastoresStatistics',
> +		height: 250,
> +	    },
> +	    {
> +		xtype: 'pbsLongestTasks',
> +		bind: {
> +		    title: gettext('Longest Tasks') + ' (' +
> +		    Ext.String.format(gettext('{0} days'), '{days}') + ')',
> +		},
> +		reference: 'longesttasks',
> +		height: 250,
> +	    },
> +	    {
> +		xtype: 'pbsRunningTasks',
> +		height: 250,
> +	    },
> +	    {
> +		bind: {
> +		    title: gettext('Task Summary') + ' (' +
> +		    Ext.String.format(gettext('{0} days'), '{days}') + ')',
> +		},
> +		xtype: 'pbsTaskSummary',
> +		height: 250,
> +		reference: 'tasksummary',
> +	    },
> +	    {
> +		iconCls: 'fa fa-ticket',
> +		title: 'Subscription',
> +		height: 200,
> +		reference: 'subscription',
> +		xtype: 'pbsSubscriptionInfo',
> +	    }],
>   	},
>       ],
>   });
> diff --git a/www/Makefile b/www/Makefile
> index 455fbeec..636d4a57 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -81,6 +81,7 @@ JSSRC=							\
>   	panel/StorageAndDisks.js			\
>   	panel/UsageChart.js				\
>   	panel/NodeInfo.js				\
> +	panel/Notes.js				    \
>   	ZFSList.js					\
>   	DirectoryList.js				\
>   	LoginView.js					\
> @@ -88,7 +89,6 @@ JSSRC=							\
>   	SystemConfiguration.js				\
>   	Subscription.js					\
>   	datastore/Summary.js				\
> -	datastore/Notes.js				\
>   	datastore/PruneAndGC.js				\
>   	datastore/Prune.js				\
>   	datastore/Content.js				\
> diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
> index c3769257..d11646f0 100644
> --- a/www/datastore/Summary.js
> +++ b/www/datastore/Summary.js
> @@ -206,7 +206,7 @@ Ext.define('PBS.DataStoreSummary', {
>   		    },
>   		},
>   		{
> -		    xtype: 'pbsDataStoreNotes',
> +		    xtype: 'pbsNotes',
>   		    flex: 1,
>   		    cbind: {
>   			datastore: '{datastore}',
> @@ -278,14 +278,14 @@ Ext.define('PBS.DataStoreSummary', {
>   	    success: function(response) {
>   		let path = Ext.htmlEncode(response.result.data.path);
>   		me.down('pbsDataStoreInfo').setTitle(`${me.datastore} (${path})`);
> -		me.down('pbsDataStoreNotes').setNotes(response.result.data.comment);
> +		me.down('pbsNotes').setNotes(response.result.data.comment);
>   	    },
>   	    failure: function(response) {
>   		// fallback if e.g. we have no permissions to the config
>   		let rec = Ext.getStore('pbs-datastore-list')
>   		    .findRecord('store', me.datastore, 0, false, true, true);
>   		if (rec) {
> -		    me.down('pbsDataStoreNotes').setNotes(rec.data.comment || "");
> +		    me.down('pbsNotes').setNotes(rec.data.comment || "");
>   		}
>   	    },
>   	});
> diff --git a/www/datastore/Notes.js b/www/panel/Notes.js
> similarity index 81%
> rename from www/datastore/Notes.js
> rename to www/panel/Notes.js
> index 2928b7ec..4128dd8b 100644
> --- a/www/datastore/Notes.js
> +++ b/www/panel/Notes.js
> @@ -1,6 +1,6 @@
> -Ext.define('PBS.DataStoreNotes', {
> +Ext.define('PBS.panel.Notes', {

imo, the changes below would warrant its own patch
(makes it easier to review)

>       extend: 'Ext.panel.Panel',
> -    xtype: 'pbsDataStoreNotes',
> +    xtype: 'pbsNotes',
>       mixins: ['Proxmox.Mixin.CBind'],
>   
>       title: gettext("Comment"),
> @@ -11,7 +11,16 @@ Ext.define('PBS.DataStoreNotes', {
>   
>       cbindData: function(initalConfig) {
>   	let me = this;
> -	me.url = `/api2/extjs/config/datastore/${me.datastore}`;
> +
> +	if (('node' in me && 'datastore' in me) ||
> +	    (!('node' in me) && !('datastore' in me))) {
> +	    throw 'either both a node and a datastore were given or neither. only provide one.';

i'd do that check differently for multiple reasons

"'foo' in bar" is not syntax we really use
having a single error for multiple error cases seems complicated

something like this is more readable:
---
if (!me.node && !me.datastore) {
     throw "no ...";
}
if (me.node && me.datastore) {
     throw "both ....";
}
--

alternatively, you could prefer the 'node' (or datastore, doesn't matter)
instead and only throw an error if none is given

> +	} else if ('node' in me) {
> +	    me.url = `/api2/extjs/nodes/${me.node}/config`;
> +	} else {
> +	    me.url = `/api2/extjs/config/datastore/${me.datastore}`;
> +	}
> +
>   	return { };
>       },
>   
> @@ -97,6 +106,10 @@ Ext.define('PBS.DataStoreNotes', {
>   	let sp = Ext.state.Manager.getProvider();
>   	me.collapseMode = sp.get('notes-collapse', 'never');
>   
> +	if (me.loadOnInit === true) {
> +	    me.load();
> +	}
> +
>   	if (me.collapseMode === 'auto') {
>   	    me.setCollapsed(true);
>   	}

Re: [pbs-devel] [PATCH proxmox-backup v2 5/5] fix #3607: ui: add a separate notes view for longer markdown notes

[Dominik Csapak]

comment inline

On 2/24/22 15:18, Stefan Sterz wrote:
> since markdown notes might be rather long having only the small panel
> in the dashboard might not be sufficient. this commit adds a tab
> similar to pve's datacenter or node notes.
> 
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>   www/Makefile               |  1 +
>   www/NavigationTree.js      |  6 ++++++
>   www/NodeNotes.js           | 22 ++++++++++++++++++++++
>   www/panel/MarkdownNotes.js | 33 +++++++++++++++++++++++++--------
>   4 files changed, 54 insertions(+), 8 deletions(-)
>   create mode 100644 www/NodeNotes.js
> 
> diff --git a/www/Makefile b/www/Makefile
> index 2d55d39d..f1c0f8bb 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -99,6 +99,7 @@ JSSRC=							\
>   	datastore/DataStoreList.js			\
>   	ServerStatus.js					\
>   	ServerAdministration.js				\
> +	NodeNotes.js				        \
>   	Dashboard.js					\
>   	${TAPE_UI_FILES}				\
>   	NavigationTree.js				\
> diff --git a/www/NavigationTree.js b/www/NavigationTree.js
> index 576d05ab..916582ef 100644
> --- a/www/NavigationTree.js
> +++ b/www/NavigationTree.js
> @@ -32,6 +32,12 @@ Ext.define('PBS.store.NavigationStore', {
>   		path: 'pbsDashboard',
>   		leaf: true,
>   	    },
> +	    {
> +		text: gettext('Notes'),
> +		iconCls: 'fa fa-sticky-note-o',
> +		path: 'pbsNodeNotes',
> +		leaf: true,
> +	    },
>   	    {
>   		text: gettext('Configuration'),
>   		iconCls: 'fa fa-gears',
> diff --git a/www/NodeNotes.js b/www/NodeNotes.js
> new file mode 100644
> index 00000000..9a0fa00c
> --- /dev/null
> +++ b/www/NodeNotes.js
> @@ -0,0 +1,22 @@
> +Ext.define('PBS.NodeNotes', {
> +    extend: 'Ext.panel.Panel',
> +    xtype: 'pbsNodeNotes',

i don't think this warrants it's own class. AFAICS this
is only used once and could be simply be wrapped there

> +
> +    scrollable: true,
> +    layout: 'fit',
> +
> +    items: [
> +	{
> +	    xtype: 'container',
> +	    layout: 'fit',
> +	    items: [{
> +		xtype: 'pbsMarkdownNotes',
> +		tools: false,
> +		border: false,
> +		node: 'localhost',
> +		loadOnInit: true,
> +		enableTbar: true,
> +	    }],
> +	},
> +    ],
> +});
> diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
> index 83119d36..f522cdfd 100644
> --- a/www/panel/MarkdownNotes.js
> +++ b/www/panel/MarkdownNotes.js
> @@ -112,23 +112,40 @@ Ext.define('PBS.panel.MarkdownNotes', {
>   	},
>       }],
>   
> -    collapsible: true,
> -    collapseDirection: 'right',
> +    tbar: {
> +	itemId: 'tbar',
> +	hidden: true,
> +	items: [
> +	    {
> +		text: gettext('Edit'),
> +		handler: function() {
> +		    this.up('panel').run_editor();
> +		},
> +	    },
> +	],
> +    },
>   
>       initComponent: function() {
>   	var me = this;
>   
>   	me.callParent();
>   
> -	let sp = Ext.state.Manager.getProvider();
> -	me.collapseMode = sp.get('notes-collapse', 'never');
> +	if (me.enableTbar === true) {
> +	    me.down('#tbar').setVisible(true);
> +	} else {
> +	    me.setCollapsible(true);
> +	    me.collapseDirection = 'right';
>   
> -	if (me.loadOnInit === true) {
> -	    me.load();
> +	    let sp = Ext.state.Manager.getProvider();
> +	    me.collapseMode = sp.get('notes-collapse', 'never');
> +
> +	    if (me.collapseMode === 'auto') {
> +		me.setCollapsed(true);
> +	    }
>   	}
>   
> -	if (me.collapseMode === 'auto') {
> -	    me.setCollapsed(true);
> +	if (me.loadOnInit === true) {
> +	    me.load();

wouldn't these hunks better fit into the previous commit
where you add the MarkDown panel?

>   	}
>       },
>   });

Re: [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable

[Dominik Csapak]

i forgot one thing in my mail before (comment inline)

On 2/24/22 15:18, Stefan Sterz wrote:
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>   www/Dashboard.js           |   2 +-
>   www/Makefile               |   3 +-
>   www/panel/MarkdownNotes.js | 134 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 137 insertions(+), 2 deletions(-)
>   create mode 100644 www/panel/MarkdownNotes.js
> 
> diff --git a/www/Dashboard.js b/www/Dashboard.js
> index a78ad375..a76660ff 100644
> --- a/www/Dashboard.js
> +++ b/www/Dashboard.js
> @@ -230,7 +230,7 @@ Ext.define('PBS.Dashboard', {
>   		margin: '0 20 0 0',
>   	    },
>   	    {
> -		xtype: 'pbsNotes',
> +		xtype: 'pbsMarkdownNotes',
>   		reference: 'nodeNotes',
>   		node: 'localhost',
>   		loadOnInit: true,
> diff --git a/www/Makefile b/www/Makefile
> index 636d4a57..2d55d39d 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -81,7 +81,8 @@ JSSRC=							\
>   	panel/StorageAndDisks.js			\
>   	panel/UsageChart.js				\
>   	panel/NodeInfo.js				\
> -	panel/Notes.js				    \
> +	panel/Notes.js				        \
> +	panel/MarkdownNotes.js			        \
>   	ZFSList.js					\
>   	DirectoryList.js				\
>   	LoginView.js					\
> diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
> new file mode 100644
> index 00000000..83119d36
> --- /dev/null
> +++ b/www/panel/MarkdownNotes.js
> @@ -0,0 +1,134 @@
> +Ext.define('PBS.panel.MarkdownNotes', {
> +    extend: 'Ext.panel.Panel',
> +    xtype: 'pbsMarkdownNotes',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    title: gettext("Notes"),
> +    bodyPadding: 10,
> +    scrollable: true,
> +    animCollapse: false,
> +    maxLength: 64*1022,
> +
> +    cbindData: function(initalConfig) {
> +	let me = this;
> +
> +	if (('node' in me && 'datastore' in me) ||
> +	    (!('node' in me) && !('datastore' in me))) {
> +	    throw 'either both a node and a datastore were given or neither. please provide one.';
> +	} else if ('node' in me) {
> +	    me.url = `/api2/extjs/nodes/${me.node}/config`;
> +	} else {
> +	    me.url = `/api2/extjs/config/datastore/${me.datastore}`;
> +	}
> +
> +	return {};
> +    },
> +
> +    run_editor: function() {
> +	let me = this;
> +	let win = Ext.create('Proxmox.window.Edit', {
> +	    title: gettext('Notes'),
> +	    width: 800,
> +	    height: 600,
> +	    resizable: true,
> +	    layout: 'fit',
> +	    defaultButton: undefined,
> +	    setMaxLength: function(maxLength) {
> +		let area = win.down('textarea[name="comment"]');
> +		area.maxLength = maxLength;
> +		area.validate();
> +
> +		return me;
> +	    },
> +	    items: {
> +		xtype: 'textarea',
> +		name: 'comment',
> +		height: '100%',
> +		value: '',
> +		hideLabel: true,
> +		emptyText: gettext('You can use Markdown for rich text formatting.'),
> +		fieldStyle: {
> +		    'white-space': 'pre-wrap',
> +		    'font-family': 'monospace',
> +		},
> +	    },
> +	    url: me.url,
> +	    listeners: {
> +		destroy: function() {
> +		    me.load();
> +		},
> +	    },
> +	}).show();
> +	win.setMaxLength(me.maxLength);
> +	win.load();
> +    },
> +
> +    setNotes: function(value) {
> +	let me = this;
> +	var data = value || '';
> +
> +	let mdHtml = Proxmox.Markdown.parse(data);
> +	me.update(mdHtml);
> +
> +	if (me.collapsible && me.collapseMode === 'auto') {
> +	    me.setCollapsed(data === '');
> +	}
> +    },
> +
> +    load: function() {
> +	var me = this;
> +
> +	Proxmox.Utils.API2Request({
> +	    url: me.url,
> +	    waitMsgTarget: me,
> +	    failure: function(response, opts) {
> +		Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> +		me.setCollapsed(false);
> +	    },
> +	    success: function(response, opts) {
> +		me.setNotes(response.result.data.comment);
> +	    },
> +	});
> +    },
> +
> +    listeners: {
> +	render: function(c) {
> +	    var me = this;
> +	    me.getEl().on('dblclick', me.run_editor, me);
> +	},
> +	afterlayout: function() {
> +	    let me = this;
> +	    if (me.collapsible && !me.getCollapsed() && me.collapseMode === 'always') {
> +		me.setCollapsed(true);
> +		me.collapseMode = ''; // only once, on initial load!
> +	    }
> +	},
> +    },
> +
> +    tools: [{
> +	type: 'gear',
> +	handler: function() {
> +	    this.up('panel').run_editor();
> +	},
> +    }],
> +
> +    collapsible: true,
> +    collapseDirection: 'right',
> +
> +    initComponent: function() {
> +	var me = this;
> +
> +	me.callParent();
> +
> +	let sp = Ext.state.Manager.getProvider();
> +	me.collapseMode = sp.get('notes-collapse', 'never');

this is never set in pbs, so it has no use here
(though it would probably a nice addition for pbs)

> +
> +	if (me.loadOnInit === true) {
> +	    me.load();
> +	}
> +
> +	if (me.collapseMode === 'auto') {
> +	    me.setCollapsed(true);
> +	}
> +    },
> +});

Re: [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard

[Thomas Lamprecht]

On 01.03.22 11:41, Dominik Csapak wrote:
> high level comment:
> 
> i get what you tried to achieve here with the
> container for info and notes, but i'd probably
> simply add it as a regular panel.
> 
> that way, the user is not confused when the column selection
> does not work the way he expects (e.g. setting it to 3 columns
> still always has only the info/note as the first line)
> 
> so either a regular panel (without the collapsing feature, maybe
> a browser storage option to hide it? like the days) or let
> it behave like on the datastore summary (info+notes = 1 panel)

I rather would do a separate Notes entry in the navigation tree, i.e., the
same as PVE does at the node level.