From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 11D58694E6; Wed, 23 Mar 2022 12:04:40 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 07F9D26366; Wed, 23 Mar 2022 12:04:40 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id F3BC22635B; Wed, 23 Mar 2022 12:04:38 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6CC99418CA; Wed, 23 Mar 2022 12:04:38 +0100 (CET) Message-ID: <184a731e-139c-28af-5d68-89737d8521af@proxmox.com> Date: Wed, 23 Mar 2022 12:04:33 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:99.0) Gecko/20100101 Thunderbird/99.0 Content-Language: en-US To: Proxmox VE development discussion , Stefan Sterz , pbs-devel@lists.proxmox.com References: <20220304113202.4137916-1-s.sterz@proxmox.com> <20220304113202.4137916-5-s.sterz@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20220304113202.4137916-5-s.sterz@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.056 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH widget-toolkit v3 4/6] toolkit: add markdown based NotesView and NotesEdit X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Mar 2022 11:04:40 -0000 On 04.03.22 12:32, Stefan Sterz wrote: > move these from pve to the widget toolkit to be ablte to use them in > pbs > this is no 1:1 move but the commit messages fails to describe the changes made nor the rationale. > Signed-off-by: Stefan Sterz > --- > src/Makefile | 2 + > src/panel/NotesView.js | 155 ++++++++++++++++++++++++++++++++++++++++ > src/window/NotesEdit.js | 38 ++++++++++ > 3 files changed, 195 insertions(+) > create mode 100644 src/panel/NotesView.js > create mode 100644 src/window/NotesEdit.js > > diff --git a/src/Makefile b/src/Makefile > index de34531..ae20947 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -65,6 +65,7 @@ JSSRC= \ > panel/ACMEDomains.js \ > panel/StatusView.js \ > panel/TfaView.js \ > + panel/NotesView.js \ > window/Edit.js \ > window/PasswordEdit.js \ > window/SafeDestroy.js \ > @@ -86,6 +87,7 @@ JSSRC= \ > window/AddWebauthn.js \ > window/AddYubico.js \ > window/TfaEdit.js \ > + window/NotesEdit.js \ > node/APT.js \ > node/APTRepositories.js \ > node/NetworkEdit.js \ > diff --git a/src/panel/NotesView.js b/src/panel/NotesView.js > new file mode 100644 > index 0000000..df5e688 > --- /dev/null > +++ b/src/panel/NotesView.js > @@ -0,0 +1,155 @@ > +Ext.define('Proxmox.panel.NotesView', { > + extend: 'Ext.panel.Panel', > + xtype: 'pmxNotesView', > + mixins: ['Proxmox.Mixin.CBind'], > + > + title: gettext("Notes"), > + bodyPadding: 10, > + scrollable: true, > + animCollapse: false, > + maxLength: 64 * 1024, > + > + cbindData: function(initalConfig) { > + let me = this; > + > + if (me.node) { > + me.url = `/api2/extjs/nodes/${me.node}/config`; > + me.maxLength = 64 * 1022; why factor 1022 ?! And why override it uncoditionally, leaving no control for the caller? also half of initialization logic now gets handled by cbind and half by initComponent, why the split-approach? This could just be an `else` for the initComponent's if (!node) clause. > + } > + > + return {}; > + }, > + > + ^- extra whitespace > + run_editor: function() { > + let me = this; > + Ext.create('Proxmox.window.NotesEdit', { > + url: me.url, > + listeners: { > + destroy: () => me.load(), > + }, > + autoShow: true, > + }).setMaxLength(me.maxLength); > + }, > + > + setNotes: function(value) { > + let me = this; > + var data = value || ''; use let for new code, could also just drop intermediate variable and make either the caller fallback, or use `value || ''` for the parse and !data for the setCollapsed. > + > + 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) { > + me.update(gettext('Error') + " " + response.htmlStatus); > + me.setCollapsed(false); > + }, > + success: function(response, opts) { > + let text = response.result.data.description; useless intermediate variable > + me.setNotes(text); me.setNotes(response.result.data.description || ''); or alternatively: success: ({ result }) => me.setNotes(result.data.description || ''), > + }, > + }); > + }, > + > + 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(); > + }, > + }], > + > + tbar: { no hard feelings on moving tbar from top of file here, but it makes slightly diff'ing harder. > + itemId: 'tbar', > + hidden: true, > + items: [ > + { > + text: gettext('Edit'), > + handler: function() { > + this.up('panel').run_editor(); I'd keep the `let view = ` > + }, > + }, > + ], > + }, > + > + initComponent: function() { > + const me = this; > + let type = ''; > + > + if (!me.node) { > + if (me.pveSelNode.data.id === 'root') { > + me.url = '/api2/extjs/cluster/options'; > + type = me.pveSelNode.data.type; > + } else { > + const nodename = me.pveSelNode.data.node; > + type = me.pveSelNode.data.type; > + > + if (!nodename) { > + throw "no node name specified"; > + } > + > + if (!Ext.Array.contains(['node', 'qemu', 'lxc'], type)) { > + throw 'invalid type specified'; > + } > + > + const vmid = me.pveSelNode.data.vmid; > + > + if (!vmid && type !== 'node') { > + throw "no VM ID specified"; > + } > + > + me.url = `/api2/extjs/nodes/${nodename}/`; > + > + // add the type specific path if qemu/lxc and set the backend's maxLen > + if (type === 'qemu' || type === 'lxc') { > + me.url += `${type}/${vmid}/`; > + me.maxLength = 8 * 1024; > + } > + > + me.url += 'config'; > + } > + } > + > + me.callParent(); > + > + if (me.enableTbar === true || type === 'node' || type === '') { // '' is for datacenter s/enableTbar/enableTBar/ and please also set the default option value on top, as it's needlessly hard to make devs pull any possible config out from the code. > + me.down('#tbar').setVisible(true); > + } else if (me.pveSelNode.data.template !== 1) { use ?. for accessing properties that may not be defined, this can easily break if used in a context where the if evaluates false. > + me.setCollapsible(true); > + me.collapseDirection = 'right'; > + > + let sp = Ext.state.Manager.getProvider(); > + me.collapseMode = sp.get('guest-notes-collapse', 'never'); > + > + if (me.collapseMode === 'auto') { > + me.setCollapsed(true); > + } > + } > + > + > + me.load(); > + }, > +}); > diff --git a/src/window/NotesEdit.js b/src/window/NotesEdit.js > new file mode 100644 > index 0000000..ab5254d > --- /dev/null > +++ b/src/window/NotesEdit.js > @@ -0,0 +1,38 @@ > +Ext.define('Proxmox.window.NotesEdit', { > + extend: 'Proxmox.window.Edit', > + > + title: gettext('Notes'), > + onlineHelp: 'markdown_basics', it could be better to let the caller set the onlineHelp, so that the reference existence is correctly checked in pve-manage/proxmox-backup. > + > + width: 800, > + height: '600px', > + > + resizable: true, > + layout: 'fit', > + > + autoLoad: true, > + defaultButton: undefined, > + > + setMaxLength: function(maxLength) { > + let me = this; > + > + let area = me.down('textarea[name="description"]'); > + area.maxLength = maxLength; > + area.validate(); > + > + return me; > + }, > + > + items: { > + xtype: 'textarea', > + name: 'description', > + height: '100%', > + value: '', > + hideLabel: true, > + emptyText: gettext('You can use Markdown for rich text formatting.'), > + fieldStyle: { > + 'white-space': 'pre-wrap', > + 'font-family': 'monospace', > + }, > + }, > +});