From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 490081FF13B for ; Wed, 25 Mar 2026 09:58:26 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 394B1C53C; Wed, 25 Mar 2026 09:58:45 +0100 (CET) Message-ID: <17c45f2f-2e0a-4ded-9f9f-86d3c804b00c@proxmox.com> Date: Wed, 25 Mar 2026 09:58:39 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH pve-manager] fix: #4490 ui: backup job window: add filter/search for virtual guest grid To: David Riley , pve-devel@lists.proxmox.com References: <20260319124527.4343-1-d.riley@proxmox.com> Content-Language: en-US From: Thomas Lamprecht In-Reply-To: <20260319124527.4343-1-d.riley@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1774429072019 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.009 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 5H57FFPI2YB3GCR4SZKCBYU3N34LC4MA X-Message-ID-Hash: 5H57FFPI2YB3GCR4SZKCBYU3N34LC4MA X-MailFrom: t.lamprecht@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 19.03.26 um 13:50 schrieb David Riley: > Add a collapsible form for filtering by Name, Status, Pool, Type, > Tags, and HA State. > > Add corresponding columns (pool, type, tags, hastate) to the guest > grid so the visual output matches the available filters. > > Expand/collapse the filter UI depending on the selected Backup Mode > (All, Pool, Include/Exclude). I can spot some smaller code but more importantly UX issues, the biggest one would be that this could be confused for an implementation of very much looks like an implementation of #4186 "more flexible virtual guest selection mechanism for backup jobs, possibly including tags and multiple pools" I can e.g. still edit the filters if I change the base selection from selected-guests to all-guests, then e.g. select a some tag as filter and save the job thinking guests matching those tag will be backed up by this job, but rather all guests will be backed up, which e.g., can lead to full backup storage and/or long running jobs causing IO pressure. And even with selected-guests chosen as basic selection mode, users might still expect that the filter is applied on job run time, not just once on job edit or create. The filter also makes the window rather tall, still fine for nowadays rather common display sizes, but the low-res bare minimum we try to support such that it's at least possible to be able to use all UI controls, doesn't have to look good, might be >>From some years ago a user mentioned using 768p, but with OS task bar and the like only roughly 1300x680 is left for the PVE ui. Now, that has been some years and here it's also a bit borderline as the filters can be collapsed, making the window short enough to use it at that size. The grid is now pretty crammed and I cannot see much of the actual guest (host) names nor much else of the available columns. We can get us some space here by e.g. changing the renderer for type to just use the short VM and CT abbreviations (it's used in other places in the UI, and here the surrounding context makes it telling enough). The status column could be icon-only with the descriptive status text being shown as tooltip. The name column definitively needs to be wider. HA state might become a "HA configured or not" check mark, potentially with a third state for "ignored" special state (e.g., rendering to fa-check, fa-minus, fa-info icons). But while that can be improved, the main point is IMO the confusion potential for this being real filter, and IMO it'd be much better if we implement that when adding such complex (nice!) filters to the backup window, as that solves both cases. As stop-gap you could implement a very simple search text-field, e.g., something similar to what the Backup "Job Detail" window got. That's will already providee most of the convenience needed here, should be much simpler and avoids confusion potential for this being "live" filter saved to the job and is less intrusive UI wise. For the filters we could then see if making guest selection a dedicated tab might be worth it. > Signed-off-by: David Riley > --- > www/manager6/dc/Backup.js | 354 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 350 insertions(+), 4 deletions(-) > diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js > index 956a7cdf..28d63bbb 100644 > --- a/www/manager6/dc/Backup.js > +++ b/www/manager6/dc/Backup.js [...] > + prepareFilters: function () { > + let me = this; > + > + me.filterState = {}; > + me.activeFilter = new Ext.util.Filter({ > + id: 'filter', > + scope: me, > + filterFn: me.filterFn, > + }); > + > + let statusMap = {}; > + let poolMap = {}; > + let haMap = {}; > + let tagMap = {}; > + > + PVE.data.ResourceStore.each((rec) => { > + if (['qemu', 'lxc'].indexOf(rec.data.type) !== -1) { the .includes [0] for array is available in ES 2024 baseline (and much earlier already), so this can become: if (['qemu', 'lxc'].includes(rec.data.type)) [0]: https://developer.mozilla.org/de/docs/Web/JavaScript/Reference/Global_Objects/Array/includes > + statusMap[rec.data.status] = true; > + } > + if (rec.data.type === 'pool') { > + poolMap[rec.data.pool] = true; > + } > + if (rec.data.hastate !== '') { This also evaluates to true on null/undef though? Maybe go for rec.data.hastate?.length > + haMap[rec.data.hastate] = true; > + } > + if (rec.data.tags !== '') { > + rec.data.tags.split(/[,; ]/).forEach((tag) => { > + if (tag !== '') { > + tagMap[tag] = true; > + } > + }); > + } > + }); > + > + let statusList = Object.keys(statusMap).map((key) => [key, key]); > + statusList.unshift(['', gettext('All')]); > + let poolList = Object.keys(poolMap).map((key) => [key, key]); > + let haList = Object.keys(haMap).map((key) => [key, key]); > + let tagList = Object.keys(tagMap).map((key) => ({ value: key })); > + > + me.lookup('includetagfilter').getStore().setData(tagList); > + me.lookup('excludetagfilter').getStore().setData(tagList); > + me.lookup('statusfilter').getStore().setData(statusList); > + me.lookup('hastatefilter').getStore().setData(haList); > + me.lookup('poolfilter').getStore().setData(poolList); > + }, > + > init: function (view) { > let me = this; > + me.prepareFilters(); > > if (view.isCreate) { > me.lookup('modeSelector').setValue('include'); > @@ -330,6 +502,171 @@ Ext.define('PVE.dc.BackupEdit', { > 'data-qtip': gettext('Description of the job'), > }, > }, > + { > + xtype: 'fieldset', > + reference: 'filters', > + collapsible: true, > + title: gettext('Filters'), > + layout: 'hbox', > + margin: '0 2 10 0', > + items: [ > + { > + xtype: 'container', > + flex: 1, > + padding: 5, > + layout: { > + type: 'vbox', > + align: 'stretch', > + }, > + defaults: { > + listeners: { > + change: 'filterChange', > + }, > + isFormField: false, > + }, > + items: [ > + { > + fieldLabel: gettext('Name'), > + reference: 'namefilter', > + xtype: 'textfield', style nit: please order the xtype to the top of such configs. btw. as filters like these would be used in a few parts (e.g., bulk actions, which probably was your template I'd guess) we probably should factor out some common stuff. But might not be relevant if we go for the simple stop-gap for now and actual flexible selection that #smart tracks.