From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <a.lauterer@proxmox.com>
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 F3DCBA2B24
 for <pve-devel@lists.proxmox.com>; Tue, 20 Jun 2023 11:35:13 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id DD65E32D19
 for <pve-devel@lists.proxmox.com>; Tue, 20 Jun 2023 11:35:13 +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 <pve-devel@lists.proxmox.com>; Tue, 20 Jun 2023 11:35:13 +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 187A940938
 for <pve-devel@lists.proxmox.com>; Tue, 20 Jun 2023 11:35:13 +0200 (CEST)
Message-ID: <bad242d3-7a01-d063-6f08-9bc27c55cb7b@proxmox.com>
Date: Tue, 20 Jun 2023 11:35:12 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Dominik Csapak <d.csapak@proxmox.com>
References: <20230619141307.119430-1-d.csapak@proxmox.com>
 <20230619141307.119430-4-d.csapak@proxmox.com>
Content-Language: en-US
From: Aaron Lauterer <a.lauterer@proxmox.com>
In-Reply-To: <20230619141307.119430-4-d.csapak@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
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 <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 20 Jun 2023 09:35:14 -0000

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


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?

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 <d.csapak@proxmox.com>
> ---
> 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
>