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

[Dominik Csapak <d.csapak@proxmox.com>]

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?

>   	}
>       },
>   });