Re: [pdm-devel] [PATCH datacenter-manager 1/1] ui: enable the top navigation bar's documentation button

[Dominik Csapak <d.csapak@proxmox.com>]

two comments inline:


On 9/11/25 1:40 PM, Shannon Sterz wrote:
> and make it open the beta documentation in a new tab for now.
> 
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>   ui/Cargo.toml         |  2 +-
>   ui/src/top_nav_bar.rs | 20 +++++++++++++++++---
>   2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/ui/Cargo.toml b/ui/Cargo.toml
> index 43424db..c60426a 100644
> --- a/ui/Cargo.toml
> +++ b/ui/Cargo.toml
> @@ -23,7 +23,7 @@ serde_json = "1.0"
>   wasm-bindgen = "0.2.92"
>   wasm-bindgen-futures = "0.4"
>   wasm-logger = "0.2"
> -web-sys = { version = "0.3", features = ["Location"] }
> +web-sys = { version = "0.3", features = ["Location", "HtmlAnchorElement"] }
>   yew = { version = "0.21",  features = ["csr"] }
>   yew-router = { version = "0.18" }
>   
> diff --git a/ui/src/top_nav_bar.rs b/ui/src/top_nav_bar.rs
> index 9b1e37e..f348592 100644
> --- a/ui/src/top_nav_bar.rs
> +++ b/ui/src/top_nav_bar.rs
> @@ -3,6 +3,8 @@ use std::rc::Rc;
>   use anyhow::Error;
>   use pwt::css::ColorScheme;
>   use serde::Deserialize;
> +use wasm_bindgen::{JsCast, UnwrapThrowExt};
> +use web_sys::HtmlAnchorElement;
>   
>   use pwt::prelude::*;
>   use pwt::widget::menu::{Menu, MenuButton, MenuEntry, MenuEvent, MenuItem};
> @@ -181,10 +183,22 @@ impl Component for PdmTopNavBar {
>                   Tooltip::new(
>                       Button::new(tr!("Documentation"))
>                           .icon_class("fa fa-book")
> -                        .disabled(true)
> -                        .class(ColorScheme::Neutral),
> +                        .class(ColorScheme::Neutral)
> +                        .onclick(|_| {

for new code, it would be better to use 'on_activate'.
we try to be consistent here across our components
and use our own wrapper that handles mouse + keyboard correctly
(on the button it literally is just the onclick handler, but
using our wrapper here too promotes it for others that might
copy & paste these things)

> +                            let document = gloo_utils::document();
> +                            let _ = document.create_element("a").map(|a| {
> +                                let atag: HtmlAnchorElement = a
> +                                    .dyn_into()
> +                                    .expect_throw("an a tag should always be an anchor element");
> +                                atag.set_href(
> +                                    "https://pve.proxmox.com/wiki/Proxmox_Datacenter_Manager_Beta_Documentation",
> +                                );
> +                                atag.set_target("_blank");
> +                                atag.click();


is there a special reason why you create an 'a' tag and click on it?

e.g. for the pve ui links we always use:

`let _ = window().open_with_url(&url.href());`

wouldn't it here also be easier to use something like:

`let _ = window().open_with_url_and_target("https://pve...", "_blank");`

?

> +                            });
> +                        }),
>                   )
> -                .tip(tr!("Coming soon")),
> +                .tip(tr!("Open the Beta documentation in a new tab.")),
>               );
>   
>           if let Some(username) = &props.username {



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