Re: [pve-devel] [RFC PATCH manager 4/4] ui: pci mapping: rework mapping panel for better user experience

[Aaron Lauterer <a.lauterer@proxmox.com>]

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