[pbs-devel] [pve-devel] [PATCH widget-toolkit v3 07/14] notification: add UI for adding/updating webhook targets
Lukas Wagner
l.wagner at proxmox.com
Tue Nov 19 09:05:33 CET 2024
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!
More information about the pbs-devel
mailing list