public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: Gabriel Goller <g.goller@proxmox.com>
Cc: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [RFC backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
Date: Tue, 21 May 2024 10:35:08 +0200	[thread overview]
Message-ID: <28b9c877-ee65-43a1-8676-c2993bf1bf46@proxmox.com> (raw)
In-Reply-To: <20240521082240.il4iqsvnogv3yzwt@luna.proxmox.com>

On 5/21/24 10:22, Gabriel Goller wrote:
> On 17.05.2024 09:15, Dominik Csapak wrote:
>> thanks for tackling this
>>
>> first, while this approach seems ok to me, there are probably still some
>> things to figure out yet in the bug report (see the bug report)
>> probably the biggest thing i'd change is the need for an extra api call.
>>
>> since we already have to serve the index file,
>> we could use a similar mechanism to register the consent text
>> there (i.e. save it in memory and reread it when the mtime of the file changes)
>>
>> and render the text of the consent directly into the index template.
>> that way we can avoid an extra round trip and directly show it
>> without an api call
> 
> Oooh, that's an interesting idea...
> Added these few lines to the index.hbs file. I think the only way is to
> bind the function to a window.* variable isn't there?
> 
>      {{#if consentText}}
>          <script type='text/javascript' > window.getConsentText = function () { return 
> `{{ consentText }}`; } </script>
>      {{else}}
>          <script type='text/javascript'> window.getConsentText = function () { return ""; } </script>
>      {{/if}}
> 

i don't think it has to be as complicated.

e.g. i'd just add a new property for the 'Proxmox' object in the template:

---
Proxmox = {
  ...
  consentText: "{{ consentText }}",
};
---

and then checking in the ui if it's the non empty string ?

(maybe we could find a way to add it in a more general way instead of
adding each option we want individually
if that's possible, we wouldn't have to add a new option to the template
every time

(e.g. some kind of 'additionalData' object with keys+values ?
not sure what the templating systems of pbs and pve support)

> 
> Another question is if we'd like to have customizable buttons. On
> one hand 'I agree' and 'I decline' probably cover 99% of the use-cases,
> (especially as we can translate it) but we could still add something like
> "<agree-text>|<decline-text>" to the end of the consent.txt file. For
> example:
> 
>      YES|NO
> 
> or
>      Agree|Decline
> 
> or (this won't show the disagree button)
> 
>      Agree|
> 
> 
> 


according to the bug report this should not be necessary, a simple box with
'OK' as only option should even be enough

that would be generic for now that others could even repurpose it for some kind of system 
announcements (like outages, etc.?)


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

  reply	other threads:[~2024-05-21  8:34 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-16 16:24 Gabriel Goller
2024-05-16 16:24 ` [pbs-devel] [PATCH proxmox-backup 1/4] api: add consent api handler and config Gabriel Goller
2024-05-16 16:24 ` [pbs-devel] [PATCH proxmox-backup 2/4] ui: show consent banner before login Gabriel Goller
2024-05-16 16:25 ` [pbs-devel] [PATCH proxmox-backup 3/4] docs: add short section about consent banner Gabriel Goller
2024-05-16 16:25 ` [pbs-devel] [PATCH backup 4/4] window: add consent modal Gabriel Goller
2024-05-17  7:15 ` [pbs-devel] [RFC backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Dominik Csapak
2024-05-21  8:22   ` Gabriel Goller
2024-05-21  8:35     ` Dominik Csapak [this message]
2024-05-21 10:13       ` Gabriel Goller
2024-05-22 13:28 ` Gabriel Goller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=28b9c877-ee65-43a1-8676-c2993bf1bf46@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=g.goller@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal