Re: [pdm-devel] [PATCH yew-comp 1/2] utils/tfa add recover/token panel: add copy_text_to_clipboard function

From: Dominik Csapak <d.csapak@proxmox.com>
To: Proxmox Datacenter Manager development discussion
	<pdm-devel@lists.proxmox.com>,
	Shannon Sterz <s.sterz@proxmox.com>
Subject: Re: [pdm-devel] [PATCH yew-comp 1/2] utils/tfa add recover/token panel: add copy_text_to_clipboard function
Date: Fri, 10 Oct 2025 14:09:08 +0200	[thread overview]
Message-ID: <fb7784ad-929c-4b09-a377-fe10753a9306@proxmox.com> (raw)
In-Reply-To: <20251003142108.352525-5-s.sterz@proxmox.com>

looks good to me, consider

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>

On 10/3/25 4:20 PM, Shannon Sterz wrote:
> this also adapts all use sites of `copy_to_clipboard` to use this new
> function instead and marks the old function as deprecated.
> `copy_to_clipboard` is based on the `document.execCommand()` method
> that is deprecated and might not be supported in the future [1].
> 
> `copy_text_to_clipboard` is based on the new `Clipboard` API that is
> now in baseline and, thus, widely available. it should also be
> somewhat more ergonomic to use. users don't need to handle a `NodeRef`
> in multiple places, but can just pass text to be copied to the
> function directly.
> 
> this requires the web_sys features `Clipboard` and `Navigator`.
> 
> [1]:
> https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand
> [2]:
> https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/writeText
> 
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>   Cargo.toml                  |  2 ++
>   src/tfa/tfa_add_recovery.rs | 17 ++++++-----------
>   src/token_panel.rs          | 16 +++++++++-------
>   src/utils.rs                | 22 ++++++++++++++++++++++
>   4 files changed, 39 insertions(+), 18 deletions(-)
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index 9abb8d3..9fe242f 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -18,6 +18,7 @@ web-sys = { version = "0.3", features = [
>     "AbortController",
>     "AbortSignal",
>     "Attr",
> +  "Clipboard",
>     "Crypto",
>     "Document",
>     "DomParser",
> @@ -26,6 +27,7 @@ web-sys = { version = "0.3", features = [
>     "HtmlDocument",
>     "HtmlElement",
>     "NamedNodeMap",
> +  "Navigator",
>     "Node",
>     "Range",
>     "ReadableStreamDefaultReader",
> diff --git a/src/tfa/tfa_add_recovery.rs b/src/tfa/tfa_add_recovery.rs
> index b24e73c..7c0e9d6 100644
> --- a/src/tfa/tfa_add_recovery.rs
> +++ b/src/tfa/tfa_add_recovery.rs
> @@ -14,7 +14,7 @@ use crate::percent_encoding::percent_encode_component;
>   
>   use pwt_macros::builder;
>   
> -use crate::utils::copy_to_clipboard;
> +use crate::utils::copy_text_to_clipboard;
>   use crate::{AuthidSelector, EditWindow};
>   
>   #[derive(Debug, Deserialize)]
> @@ -81,7 +81,6 @@ pub enum Msg {
>   #[doc(hidden)]
>   pub struct ProxmoxTfaAddRecovery {
>       recovery_keys: Option<RecoveryKeyInfo>,
> -    container_ref: NodeRef,
>       print_counter: usize,
>       print_portal: Option<Html>,
>   }
> @@ -107,14 +106,15 @@ fn render_input_form(_form_ctx: FormContext) -> Html {
>   impl ProxmoxTfaAddRecovery {
>       fn recovery_keys_dialog(&self, ctx: &Context<Self>, data: &RecoveryKeyInfo) -> Html {
>           use std::fmt::Write;
> -        let text: String = data
> +        let text: AttrValue = data
>               .keys
>               .iter()
>               .enumerate()
>               .fold(String::new(), |mut acc, (i, key)| {
>                   let _ = writeln!(acc, "{i}: {key}\n");
>                   acc
> -            });
> +            })
> +            .into();
>   
>           Dialog::new(tr!("Recovery Keys for user '{}'", data.userid))
>               .on_close(ctx.props().on_close.clone())
> @@ -128,8 +128,7 @@ impl ProxmoxTfaAddRecovery {
>                                       .class("pwt-font-monospace")
>                                       .padding(2)
>                                       .border(true)
> -                                    .with_child(text)
> -                                    .into_html_with_ref(self.container_ref.clone()),
> +                                    .with_child(text.clone()),
>                               )
>                               .with_child(
>                                   Container::new()
> @@ -147,10 +146,7 @@ impl ProxmoxTfaAddRecovery {
>                                   Button::new(tr!("Copy Recovery Keys"))
>                                       .icon_class("fa fa-clipboard")
>                                       .class("pwt-scheme-primary")
> -                                    .onclick({
> -                                        let container_ref = self.container_ref.clone();
> -                                        move |_| copy_to_clipboard(&container_ref)
> -                                    }),
> +                                    .on_activate(move |_| copy_text_to_clipboard(&text)),
>                               )
>                               .with_child(
>                                   Button::new(tr!("Print Recovery Keys"))
> @@ -172,7 +168,6 @@ impl Component for ProxmoxTfaAddRecovery {
>       fn create(_ctx: &Context<Self>) -> Self {
>           Self {
>               recovery_keys: None,
> -            container_ref: NodeRef::default(),
>               print_portal: None,
>               print_counter: 0,
>           }
> diff --git a/src/token_panel.rs b/src/token_panel.rs
> index c70adb2..c027a32 100644
> --- a/src/token_panel.rs
> +++ b/src/token_panel.rs
> @@ -17,7 +17,9 @@ use pwt::widget::form::{Checkbox, DisplayField, Field, FormContext, InputType};
>   use pwt::widget::{Button, Column, Container, Dialog, InputPanel, Toolbar};
>   
>   use crate::percent_encoding::percent_encode_component;
> -use crate::utils::{copy_to_clipboard, epoch_to_input_value, render_boolean, render_epoch_short};
> +use crate::utils::{
> +    copy_text_to_clipboard, epoch_to_input_value, render_boolean, render_epoch_short,
> +};
>   use crate::{
>       AuthidSelector, ConfirmButton, EditWindow, LoadableComponent, LoadableComponentContext,
>       LoadableComponentLink, LoadableComponentMaster, PermissionPanel,
> @@ -121,7 +123,6 @@ enum Msg {
>   struct ProxmoxTokenView {
>       selection: Selection,
>       store: Store<ApiToken>,
> -    secret_node_ref: NodeRef,
>       columns: Rc<Vec<DataTableHeader<ApiToken>>>,
>   }
>   
> @@ -149,7 +150,6 @@ impl LoadableComponent for ProxmoxTokenView {
>           Self {
>               selection,
>               store,
> -            secret_node_ref: NodeRef::default(),
>               columns: columns(),
>           }
>       }
> @@ -351,8 +351,7 @@ impl ProxmoxTokenView {
>                               .style("opacity", "0")
>                               .with_child(AttrValue::from(
>                                   secret["value"].as_str().unwrap_or("").to_owned(),
> -                            ))
> -                            .into_html_with_ref(self.secret_node_ref.clone()),
> +                            )),
>                       )
>                       .with_child(
>                           Container::new()
> @@ -373,8 +372,11 @@ impl ProxmoxTokenView {
>                                       .icon_class("fa fa-clipboard")
>                                       .class("pwt-scheme-primary")
>                                       .on_activate({
> -                                        let copy_ref = self.secret_node_ref.clone();
> -                                        move |_| copy_to_clipboard(&copy_ref)
> +                                        move |_| {
> +                                            copy_text_to_clipboard(
> +                                                secret["value"].as_str().unwrap_or(""),
> +                                            )
> +                                        }
>                                       }),
>                               ),
>                       ),
> diff --git a/src/utils.rs b/src/utils.rs
> index 79b7ad7..23794b9 100644
> --- a/src/utils.rs
> +++ b/src/utils.rs
> @@ -2,6 +2,7 @@ use std::collections::HashMap;
>   use std::fmt::Display;
>   use std::sync::Mutex;
>   
> +use pwt::convert_js_error;
>   use serde_json::Value;
>   use wasm_bindgen::JsCast;
>   use yew::prelude::*;
> @@ -338,6 +339,9 @@ pub fn json_array_to_flat_string(list: &[Value]) -> String {
>       list.join(" ")
>   }
>   
> +#[deprecated(
> +    note = "This relies on the deprecated `execCommand` method. Please use `utils::copy_text_to_clipboard` instead."
> +)]
>   pub fn copy_to_clipboard(node_ref: &NodeRef) {
>       if let Some(el) = node_ref.cast::<web_sys::HtmlInputElement>() {
>           let window = gloo_utils::window();
> @@ -356,6 +360,24 @@ pub fn copy_to_clipboard(node_ref: &NodeRef) {
>       }
>   }
>   
> +pub fn copy_text_to_clipboard(text: &str) {
> +    let text = text.to_owned();
> +
> +    wasm_bindgen_futures::spawn_local(async move {
> +        let future: wasm_bindgen_futures::JsFuture = gloo_utils::window()
> +            .navigator()
> +            .clipboard()
> +            .write_text(&text)
> +            .into();
> +
> +        let res = future.await.map_err(convert_js_error);
> +
> +        if let Err(e) = res {
> +            log::error!("could not copy to clipboard: {e:#}");
> +        }
> +    });
> +}
> +
>   /// Set the browser window.location.href
>   pub fn set_location_href(href: &str) {
>       let window = gloo_utils::window();


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