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 EBEA2A03C1 for ; Wed, 8 Nov 2023 13:15:01 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CE0D8753A for ; Wed, 8 Nov 2023 13:15:01 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 8 Nov 2023 13:15:01 +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 05A45473A8; Wed, 8 Nov 2023 13:15:01 +0100 (CET) Message-ID: <07357be1-4b7e-4441-9323-9741b70cd483@proxmox.com> Date: Wed, 8 Nov 2023 13:14:59 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Content-Language: en-US To: Thomas Lamprecht , Proxmox VE development discussion References: <20231030125811.11759-1-d.csapak@proxmox.com> <394a78a0-2ab3-4953-b4bd-0534504daffd@proxmox.com> From: Dominik Csapak In-Reply-To: <394a78a0-2ab3-4953-b4bd-0534504daffd@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.017 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 T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [titlecaseconverter.com] Subject: Re: [pve-devel] [PATCH manager 1/2] ui: BulkActions: rework filters and include tags 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, 08 Nov 2023 12:15:02 -0000 On 11/6/23 17:01, Thomas Lamprecht wrote: > for the commit subject please: s/BulkActions/bulk actions/ > > Am 30/10/2023 um 13:58 schrieb Dominik Csapak: >> This moves the filters out of the grid header for the BulkActions and >> puts them into their own fieldset above the grid. With that, we can >> easily include a tags filter (one include and one exclude list). >> >> The filter fieldset is collapsible and shows the active filters in >> parenthesis. aside from that the filter should be the same as before. >> > > basic tests seem to work, and I did not really check the code closely, > so only a higher level review, and some stuff is even pre-existing (but > sticks a bit more out now): > > - the CT/VM ID filter is a bit odd, especially if tuned to match all, > not only parts of the VMID (which would not be *that* much better > either IMO), when I want to migrate/start/stop a single VM I can just > do so, no need for opening the bulk actions. counter argument: if i want to migrate/start/stop a specific list of vmid it may be faster if i go to the bulk window, search vmid -> select, search vmid -> select, etc. than go to vm -> click stop -> go to vm -> click stop but no hard feelings, if you want i'll remove it > > - The migration one should move target and jobs into two columns, as of > now there's just to much vertical space used. > > - Maybe we can also merge the "allow local disk" check box and the > warning for "ct will use restart mode" into columns (the former could > loose the box label, that note is not really that useful anyway) > yeah i'll try those two > - In the spirit of the last two points, the shutdown action might also > benefit from having force-stop and timeout on the same row in two > columns > make sense > - We have a mix of title case and sentence case for the fields, they > should all use title case (I find https://titlecaseconverter.com/ > nice). E.g., should be "HA Status" in the filters, and Parallel Jobs > for migration, and so on. sry for that, i'll fix it > > - tag space is very limited, maybe default to using circles or even > dense (I'm still wishing a bit for seeing the tag value on hover, like > a tool tip), or re-use the tree-style shape. > One alternative might be to add vertical scrolling here, but that is > probably rather odd (and not sure if that would even work easily > here). scrolling in two dimensions for different containers is always a bit weird imho, but yeah making the tags 'dense' 'circle' or reusing the treestyle makes sense i agree with the tooltip, but last time i looked at it that was not so easy because we already have a tooltip in the tree (and we reuse the rendering for that) but i could see if i could add a tooltip to the whole cell here that prints all tags in a nice way, what do you think? > > - disallowing multi-select for Type (maybe better labeled "Guest Type"?) > might improve UX, as if there are only two choices anyway a "All", > "VM" "CT" selection might be simpler – but no hard feelings here. ah yeah, i'm not really sure why i did it this way, dropdown with 3 distinct values makes much more sense here...