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


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to