From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id AE9BC1FF17B
	for <inbox@lore.proxmox.com>; Tue, 19 Nov 2024 09:06:02 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 7E1F91CD96;
	Tue, 19 Nov 2024 09:06:06 +0100 (CET)
Mime-Version: 1.0
Date: Tue, 19 Nov 2024 09:05:33 +0100
Message-Id: <D5Q00EZ1KL78.1AH38NVRFVXZ@proxmox.com>
From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>,
 <pbs-devel@lists.proxmox.com>, <t.lamprecht@proxmox.com>,
 <d.csapak@proxmox.com>
X-Mailer: aerc 0.18.2
References: <20241108144124.273550-1-l.wagner@proxmox.com>
 <20241108144124.273550-8-l.wagner@proxmox.com>
 <f592fea7-e0a5-4858-af48-b0b2ed57bc50@proxmox.com>
In-Reply-To: <f592fea7-e0a5-4858-af48-b0b2ed57bc50@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.009 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH widget-toolkit v3 07/14] notification: add
 UI for adding/updating webhook targets
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.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