[pve-devel] [PATCH manager 1/2] ui: BulkActions: rework filters and include tags

[Dominik Csapak]

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