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 68B53A2AEB for ; Tue, 20 Jun 2023 12:18:43 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4998533473 for ; Tue, 20 Jun 2023 12:18:43 +0200 (CEST) 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 ; Tue, 20 Jun 2023 12:18:42 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 767DA41BEF for ; Tue, 20 Jun 2023 12:18:42 +0200 (CEST) Message-ID: <23bab32a-abc9-52a4-e4b9-3f35c9c5547f@proxmox.com> Date: Tue, 20 Jun 2023 12:18:41 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Content-Language: en-US To: Dominik Csapak , Proxmox VE development discussion References: <20230619141307.119430-1-d.csapak@proxmox.com> <20230619141307.119430-4-d.csapak@proxmox.com> From: Aaron Lauterer In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.034 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 NICE_REPLY_A -0.102 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] [RFC PATCH manager 4/4] ui: pci mapping: rework mapping panel for better user experience 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: Tue, 20 Jun 2023 10:18:43 -0000 On 6/20/23 11:57, Dominik Csapak wrote: > On 6/20/23 11:35, Aaron Lauterer wrote: >> I like the approach as it cleans up the overloaded tbar that has items that >> are only valid in certain contexts. >> >> Two small nits from a UX POV: >> >> - double clicking any PCI device should open the edit dialog for the node, >> similar to double clicking the node itself > > makes sense imo > >> - the Action Column should probably be further left and not on the far right >> side by default. I personally like it to be the second column from the left as >> all other columns are rather informal. > > mhmm can do that, but how i refactored that seems to be a bit hacky to inject an > actioncolumn at a > certain position, but technically not a problem But aren't we doing that already in the content view of PBS? AFAIK it is the 3rd column there. > >> >> >> I know it is kinda late, but would it be hard to add the "Device" column from >> the PCI device selection grid to the overview as well? This way one can easily >> verify that they got the right devices by name. >> But probably it is a bit harder to gather the info from the other nodes? >> > > we already query the pci list of each node, so we could extract that from there > but this only works if the user has Sys.Audit which may not be the case. > then the column would be empty > >> On 6/19/23 16:13, Dominik Csapak wrote: >>> by removing the confusing buttons in the toolbar and adding them as >>> actions in an actioncolumn. There a only relevant actions are visible >>> and get a more expressive tooltip >>> >>> with this, we now differentiate between 4 modes of the edit window: >>> * create a new mapping altogether >>>    - shows all fields >>> * edit existing mapping on top level >>>    - show only 'global' fields (comment+mdev), so no mappings >>> * add new host mapping >>>    - shows nodeselector, mapping and mdev, but mdev is disabled >>>      (informational only) >>> * edit existing host mapping >>>    - show selected node (displayfield) mdev and mappings, but only >>>      mappings are editable >>> >>> we have to split the nodeselector into two fields, since the disabling >>> cbind does not pass through to the editconfig (and thus makes the form >>> invalid if we try that) >>> >>> Signed-off-by: Dominik Csapak >>> --- >>> this is not intended to be applied as is, rather i'd like some feedback >>> on the approach (@thomas, @aaron ?) so that if we want to do it this way >>> i can also do it for the usb mappings >>> >>> the other approach mentioned off-list can still be done >>> (having a full grid with all mappings regardless of the node) >>> maybe only for usb devices (there it makes imho more sense) but then >>> we'd have two interfaces for the mappings instead of one >>> > >