Re: [pve-devel] [PATCH widget-toolkit v3 07/14] notification: add UI for adding/updating webhook targets

["Lukas Wagner" <l.wagner@proxmox.com>]

On Mon Nov 11, 2024 at 11:09 PM CET, Thomas Lamprecht wrote:
> Am 08.11.24 um 15:41 schrieb Lukas Wagner:
> > The widgets for editing the headers/secrets were adapted from
> > the 'Tag Edit' dialog from PVE's datacenter options.
> > 
> > Apart from that, the new dialog is rather standard. I've decided
> > to put the http method and url in a single row, mostly to
> > save space and also to make it analogous to how an actual http request
> > is structured (VERB URL, followed by headers, followed by the body).
> > 
> > The secrets are a mechanism to store tokens/passwords in the
> > protected notification config. Secrets are accessible via
> > templating in the URL, headers and body via {{ secrets.NAME }}.
> > Secrets can only be set/updated, but not retrieved/displayed.
> > 
>
> I re-checked this now with a bit more time and while it works fine I
> think there can be still some UX enhancements:
>
> - Move the "Add" buttons below the grids, like we do for most other
>   such UIs (like e.g. in the "Match Rules" tab of the Matcher Add/Edit
>   window). You probably need to change the layout a bit to keep the
>   label on top; might want to ask Dominik for how to best achieve that.
>   Maybe include what is added in the button text, like "Add Header" or
>   "Add Secret"
>
> - always show an emptyText for the key/value fields to better convey
>   what is what, alternatively show colum headers, but that takes more
>   space.
>
> - improve validity checking for header/secret fields, without clicking
>   in them no invalid/required border is shown, but the overall form
>   state is invalid, thus disabling the window's Add button. This can
>   be confusing if no field is marked as invalid.
>

Thanks for the feedback, much appreciated.

Seems like these issues were already taken care of while I was on sick
leave, thanks Dominik!



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