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

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

On 9/2/26 10:26 AM, Lukas Wagner wrote:
> Thanks for these patches!
> 

Thanks for looking at them!

> 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.

Yeah that make sense.

> 
> 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.
> 

True.

> 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.

probably the easiest to implement but IMO not the best option.

> 
> - 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

well that would convert the 'refresh configuration' into 'dashboard 
configuration' but only for the main dashboard. so we'd have
nearly identical settings for the dashboard and a individual view
without any hint from the outside.

I don't think the low discoverability is good ...

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

I would probably opt for this, since we already are missing
some settings that would be nice here (e.g. xterm.js/novnc settings
that we already have in 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.
> 

One of the biggest thing that makes me a bit hesitant to do this
is that we don't usually have per user backend (gui) settings at all

If we do want those here, I'd also like them on the PVE side
(e.g. tree/view settings)

> 
> What do you think?

see above, currently I'd prefer to implementing a 'my settings' window.

another option would be to have the global default set in the backend
but this clashes a bit with the permissions per view etc...

> 
> 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.

yes that sounds sensible (at least as long as we only save it in
the browser local storage. if we have a setting in the backend
we could prevent deletion if anybody is using that or reset
all users preferences)

> 
> 
>> 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.

Yes I'll look at doing that. This happens when one works too much with
JS 🤪

> 
>>   
>>   #[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}}
>>                       }
>