From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 094841FF0DF for ; Fri, 28 Aug 2026 12:57:54 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id A7C3A214D8; Fri, 28 Aug 2026 12:57:53 +0200 (CEST) Content-Type: text/plain; charset=UTF-8 Date: Fri, 28 Aug 2026 12:57:49 +0200 Message-Id: To: "Christoph Heiss" , Subject: Re: [PATCH datacenter-manager] ui: auto-installer: allow multi-select of installation entries From: "Lukas Wagner" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260827124033.871256-1-c.heiss@proxmox.com> In-Reply-To: <20260827124033.871256-1-c.heiss@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787914659874 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.567 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: MJ73YPBM5OASNXPNSH6HJU26KTZURU2P X-Message-ID-Hash: MJ73YPBM5OASNXPNSH6HJU26KTZURU2P X-MailFrom: l.wagner@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 Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu Aug 27, 2026 at 2:40 PM CEST, Christoph Heiss wrote: > Depending on the environment/usage, a lot of entries of past > installations can accumulate here over time. > > Having the possibility for bulk deletion makes it easier to deal with > this. > > Signed-off-by: Christoph Heiss Hi, thanks for the patch! I think the UX could be improved a bit. Right now, it's a bit weird that *any* click on the row automatically selects it. For instance, when going through entries one by one to show the "System Information", this is a bit annoying, since one cannot press the 'System Information' button if multiple entries are selected. I think this could be solved in two ways: - Require the user to explicitly enable 'bulk-select', e.g. by pressing a button or ticking a check-box in the header. Before doing so, the checkbox column could be hidden - Patch 'DataTable' so that only a click on the checkbox selects the entry, but not when clicking anywhere else. This probably needs to be configurable, not sure if we don't break other UIs if we do that. I think the first option could be the preferable one. > --- > .../auto_installer/installations_panel.rs | 38 +++++++++++-------- > 1 file changed, 23 insertions(+), 15 deletions(-) > > diff --git a/ui/src/remotes/auto_installer/installations_panel.rs b/ui/sr= c/remotes/auto_installer/installations_panel.rs > index 2bb52278..c40628e8 100644 > --- a/ui/src/remotes/auto_installer/installations_panel.rs > +++ b/ui/src/remotes/auto_installer/installations_panel.rs > @@ -22,7 +22,7 @@ use pwt::{ > tr, > widget::{ > Button, Toolbar, > - data_table::{DataTable, DataTableColumn, DataTableHeader}, > + data_table::{DataTable, DataTableColumn, DataTableHeader, MultiS= electMode}, > form::TextArea, > }, > }; > @@ -77,8 +77,9 @@ impl LoadableComponent for InstallationsPanelComponent = { > type ViewState =3D ViewState; > =20 > fn create(ctx: &LoadableComponentContext) -> Self { > - let selection =3D > - Selection::new().on_select(ctx.link().callback(|_| Message::= SelectionChange)); > + let selection =3D Selection::new() > + .multiselect(true) > + .on_select(ctx.link().callback(|_| Message::SelectionChange)= ); > =20 > let store =3D > Store::with_extract_key(|record: &Installation| Key::from(re= cord.uuid.to_string())); > @@ -112,15 +113,18 @@ impl LoadableComponent for InstallationsPanelCompon= ent { > } > Self::Message::SelectionChange =3D> true, > Self::Message::RemoveEntry =3D> { > - if let Some(key) =3D self.selection.selected_key() { > + self.spawn({ > let link =3D ctx.link().clone(); > - self.spawn(async move { > - if let Err(err) =3D delete_entry(key).await { > - link.show_error(tr!("Unable to delete entry"= ), err, true); > + let selection =3D self.selection.clone(); > + async move { > + for key in selection.selected_keys() { > + if let Err(err) =3D delete_entry(key).await = { > + link.show_error(tr!("Unable to delete en= try {key}"), err, true); > + } > } I guess in most cases a 'bulk-remove' API endpoint is preferable, mostly to at least *enable* atomicity at the API level. But since removing installations ends up removing individual files any way, which is inherently not atomic, I guess the current approach is fine as well. If we ever end up storing installations in something that supports atomic operations (say, a sqlite DB or a single JSON file), a bulk-delete endpoint can be added as well. > link.send_reload(); > - }) > - } > + } > + }); > false > } > }