Re: [PATCH datacenter-manager 3/3] ui: make the default dashboard page configurable

["Lukas Wagner" <l.wagner@proxmox.com>]

Thanks for these patches!

On Thu Aug 20, 2026 at 10:17 AM CEST, Dominik Csapak wrote:
> by allowing to set the user any view as their dashboard, saved in their
> local browser storage.
>
> It adds the selection combobox on the 'views' configuration page in the
> toolbar.
>

>From a user's perspective, I think having such a setting presented this
way is a bit confusing, since there is no hint about this being a
browser-local setting. If I didn't read the patch notes, my instinct
while exploring the UI on my own would be that this is a global setting
that affects all sessions and all users.

Adding a hint for this in the toolbar, either as a label or tooltip
seems a bit inpractical to me, mostly due to space constraints for
a label or discoverability for a tooltip.

Some alternatives could be:

- Replace the combobox in the toolbar with a button, e.g. "Configure
  Dashboard", pressing that button shows a dialog that allows selecting
  a view, while also explaining that this is a local setting.
  I would position this button on the right-hand side, to differentiate
  it from the 'entity-management' buttons add/modify/delete.

- The dashboard already has a couple local settings, such as the refresh
  interval, etc., these are in the 'gear' menu in the toolbar. Maybe
  on could select a view in there?
  Would in general also not hurt to add a note there that all of these
  settings are local

- Have a 'My Settings' dialog in the user menu similar to PVE

- Abandon the local setting altogether and make it a per-user setting
  that is stored in the backend. Might require bigger changes, since I
  don't think we have any good way to store per-user properties right
  now. Users that routinely use multiple browsers and/or devices might
  prefer this over a local setting.


What do you think?

One further thing that I have noticed: If a view that is used as a
dashboard is deleted, we should probably fall back to the default
dashboard again. Otherwise user might complain about 'broken' dashboards
without a clear indication about what is wrong.


> Most of the code is just extending the ViewSelector so it can work in
> other situations than the 'add view' dialog:
> * add on_change property
> * add dashboard_name property to show different text in different
>   contexts
> * passing through the 'default' property from the combobos
>
> It also refactors the '__dashboard__' string this does not have to be
> hardcoded in different places.
>

[snip]

> diff --git a/ui/src/widget/mod.rs b/ui/src/widget/mod.rs
> index 07c47e36..1eaf0ffd 100644
> --- a/ui/src/widget/mod.rs
> +++ b/ui/src/widget/mod.rs
> @@ -31,7 +31,7 @@ pub use remote_selector::RemoteSelector;
>  mod remote_endpoint_selector;
>  
>  mod view_selector;
> -pub use view_selector::ViewSelector;
> +pub use view_selector::{DASHBOARD_VALUE, ViewSelector};
>  
>  mod view_filter_selector;
>  pub use view_filter_selector::ViewFilterSelector;
> diff --git a/ui/src/widget/view_selector.rs b/ui/src/widget/view_selector.rs
> index b48ef4f7..8bc15b0a 100644
> --- a/ui/src/widget/view_selector.rs
> +++ b/ui/src/widget/view_selector.rs
> @@ -6,12 +6,29 @@ use pwt::widget::form::Combobox;
>  use pwt_macros::{builder, widget};
>  
>  use pdm_api_types::views::ViewConfig;
> +use yew::html::{IntoEventCallback, IntoPropValue};
> +
> +pub const DASHBOARD_VALUE: &str = "__dashboard__";


Do you think it would be possible to get this working without such a
magic value? So rather, in the code, instead of having a String for the
view ID, have a Option<String> where None is the default dashboard and
Some(id) is a specific view?

If that is impractical, we should probably disallow views with this
specific name from being created in the backend. I managed to get some
odd behavior after creating a view with this specific name.

Probably a bit unlikely for a user to create a view with this specific
name by accident, but if we can avoid having conflicts/issues altogether
with some changes in the code, that would of course be better.

>  
>  #[widget(comp=ViewSelectorComp, @input)]
>  #[derive(Clone, Properties, PartialEq)]
>  #[builder]
>  pub struct ViewSelector {
>      store: Store<ViewConfig>,
> +
> +    /// Change callback
> +    #[builder_cb(IntoEventCallback, into_event_callback, String)]
> +    #[prop_or_default]
> +    pub on_change: Option<Callback<String>>,
> +
> +    /// The default value
> +    #[builder(IntoPropValue, into_prop_value)]
> +    #[prop_or_default]
> +    pub default: Option<AttrValue>,
> +
> +    #[builder]
> +    #[prop_or(tr!("Dashboard"))]
> +    pub dashboard_name: String,
>  }
>  
>  impl ViewSelector {
> @@ -32,19 +49,23 @@ impl Component for ViewSelectorComp {
>      }
>  
>      fn view(&self, ctx: &Context<Self>) -> Html {
> -        let mut list = vec!["__dashboard__".into()];
> +        let props = ctx.props();
> +        let mut list = vec![DASHBOARD_VALUE.into()];
>          let store = &ctx.props().store;
>          for item in store.read().data().iter() {
>              list.push(item.id.clone().into());
>          }
>          Combobox::new()
> -            .items(Rc::new(list))
> +            .with_std_props(&ctx.props().std_props)
>              .with_input_props(&ctx.props().input_props)
> -            .on_change(|_| {})
> +            .items(Rc::new(list))
> +            .on_change(props.on_change.clone())
> +            .default(props.default.clone())
>              .render_value({
> +                let name = props.dashboard_name.clone();
>                  move |value: &AttrValue| {
> -                    if value == "__dashboard__" {
> -                        html! {{tr!("Dashboard")}}
> +                    if value == DASHBOARD_VALUE {
> +                        html! {{&name}}
>                      } else {
>                          html! {{value}}
>                      }