all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Christoph Heiss" <c.heiss@proxmox.com>
To: "Lukas Wagner" <l.wagner@proxmox.com>
Cc: pdm-devel@lists.proxmox.com
Subject: Re: [PATCH datacenter-manager] ui: auto-installer: allow multi-select of installation entries
Date: Tue, 08 Sep 2026 12:56:47 +0200	[thread overview]
Message-ID: <DL9VPZM0RMTO.EZ81OYMYE5UE@proxmox.com> (raw)
In-Reply-To: <DL0IUS37HRNP.22DT771NF9FZQ@proxmox.com>

Thanks for the review!

On Fri Aug 28, 2026 at 12:57 PM CEST, Lukas Wagner wrote:
[..]
> 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.

Makes sense. I'll see and will try out what works and fits better.

I also guess we don't have that pattern of UI somewhere else yet, which
could be taken as reference?

[..]
>>              Self::Message::RemoveEntry => {
>> -                if let Some(key) = self.selection.selected_key() {
>> +                self.spawn({
>>                      let link = ctx.link().clone();
>> -                    self.spawn(async move {
>> -                        if let Err(err) = delete_entry(key).await {
>> -                            link.show_error(tr!("Unable to delete entry"), err, true);
>> +                    let selection = self.selection.clone();
>> +                    async move {
>> +                        for key in selection.selected_keys() {
>> +                            if let Err(err) = delete_entry(key).await {
>> +                                link.show_error(tr!("Unable to delete entry {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.

Yeah, thought about that too - but kept it simple here on purpose (as
you described) for now.

>
> 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.
>




      reply	other threads:[~2026-09-08 10:56 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27 12:40 [PATCH datacenter-manager] ui: auto-installer: allow multi-select of installation entries Christoph Heiss
2026-08-28 10:57 ` Lukas Wagner
2026-09-08 10:56   ` Christoph Heiss [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DL9VPZM0RMTO.EZ81OYMYE5UE@proxmox.com \
    --to=c.heiss@proxmox.com \
    --cc=l.wagner@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal