[pdm-devel] [PATCH datacenter-manager v2] fix: show empty-state message inside the top entities panels

[Lukas Wagner]

Looking good mostly.

Regarding the commit message, usually we only use the 'fix' prefix only
if there is some concrete BZ entry that can be referenced. Also, usually
we use 'subsystem tags', for this commit this could be (slightly
reworded)

ui: top entities: show hint message if there are no entities

On Wed Dec 3, 2025 at 5:43 PM CET, Shan Shaji wrote:
> When the entities list was empty, the dashboard did not display any
> message inside the Top Entities panels. This change adds a empty-state
> message so the panels provide proper feedback when no entries are
> available.
>
> Signed-off-by: Shan Shaji <s.shaji at proxmox.com>
> ---
>  changes since v1: Thanks @Dominik
>  - Removed the extra property inside the TopEntities struct.
>  - Added optional_child to the Panel to show the empty error message. 
>  - There was an empty space caused by the `TopEntities` component when the
>    entities list was empty. Inorder to fix that now the TopEntities will
>    only be rendered if the entities list is not empty.
>
>  ui/src/dashboard/top_entities.rs | 68 ++++++++++++++++++++------------
>  1 file changed, 42 insertions(+), 26 deletions(-)
>
> diff --git a/ui/src/dashboard/top_entities.rs b/ui/src/dashboard/top_entities.rs
> index ab8c703..f79dab7 100644
> --- a/ui/src/dashboard/top_entities.rs
> +++ b/ui/src/dashboard/top_entities.rs
> @@ -1,6 +1,7 @@
>  use std::rc::Rc;
>  
>  use pwt::state::SharedState;
> +use pwt::widget::Fa;
>  use web_sys::HtmlElement;
>  use yew::virtual_dom::{VComp, VNode};
>  
> @@ -132,6 +133,7 @@ impl Component for TopEntitiesComp {
>              .style("gap", "var(--pwt-spacer-3)");
>          let mut tooltip = None;
>          let data = &props.entities;
> +

Unrelated newline change; please remove this one for v3 or at least
split it out to a separate patch, in case you think the improved code
formatting is worth it.

>          for entity in data.iter().rev() {
>              let resource = &entity.resource;
>              let rrd = &entity.rrd_data;
> @@ -199,6 +201,7 @@ impl Component for TopEntitiesComp {
>                      .with_optional_child(tooltip_anchor),
>              );
>          }
> +

Same here :)

>          Container::new()
>              .class(FlexFit)
>              .with_child(list)
> @@ -336,34 +339,47 @@ pub fn create_top_entities_panel(
>      leaderboard_type: LeaderboardType,
>  ) -> Panel {
>      let top_entities = top_entities.read();
> -    let (entities, icon, title, metrics_title, threshold) = match leaderboard_type {
> -        LeaderboardType::GuestCpu => (
> -            top_entities.data.as_ref().map(|e| e.guest_cpu.clone()),
> -            "desktop",
> -            tr!("Guests With the Highest CPU Usage"),
> -            tr!("CPU usage"),
> -            0.85,
> -        ),
> -        LeaderboardType::NodeCpu => (
> -            top_entities.data.as_ref().map(|e| e.node_cpu.clone()),
> -            "building",
> -            tr!("Nodes With the Highest CPU Usage"),
> -            tr!("CPU usage"),
> -            0.85,
> -        ),
> -        LeaderboardType::NodeMemory => (
> -            top_entities.data.as_ref().map(|e| e.node_memory.clone()),
> -            "building",
> -            tr!("Nodes With the Highest Memory Usage"),
> -            tr!("Memory usage"),
> -            0.95,
> -        ),
> -    };
> +    let (entities, icon, title, metrics_title, metrics_empty_message, threshold) =
> +        match leaderboard_type {
> +            LeaderboardType::GuestCpu => (
> +                top_entities.data.as_ref().map(|e| e.guest_cpu.clone()),
> +                "desktop",
> +                tr!("Guests With the Highest CPU Usage"),
> +                tr!("CPU usage"),
> +                tr!("No guests available"),
> +                0.85,
> +            ),
> +            LeaderboardType::NodeCpu => (
> +                top_entities.data.as_ref().map(|e| e.node_cpu.clone()),
> +                "building",
> +                tr!("Nodes With the Highest CPU Usage"),
> +                tr!("CPU usage"),
> +                tr!("No nodes available"),
> +                0.85,
> +            ),
> +            LeaderboardType::NodeMemory => (
> +                top_entities.data.as_ref().map(|e| e.node_memory.clone()),
> +                "building",
> +                tr!("Nodes With the Highest Memory Usage"),
> +                tr!("Memory usage"),
> +                tr!("No nodes available"),
> +                0.95,
> +            ),
> +        };
>      Panel::new()
>          .title(create_title_with_icon(icon, title))
> -        .with_optional_child(
> -            entities.map(|entities| TopEntities::new(entities, metrics_title, threshold)),
> -        )
> +        .with_optional_child(entities.as_ref().and_then(|entities| {
> +            entities.is_empty().then_some(
> +                Row::new()
> +                    .padding(4)
> +                    .gap(2)
> +                    .with_child(Fa::new("info-circle").fixed_width())
> +                    .with_child(&metrics_empty_message),
> +            )
> +        }))
> +        .with_optional_child(entities.and_then(|entities| {
> +            (!entities.is_empty()).then_some(TopEntities::new(entities, metrics_title, threshold))
> +        }))

I think it would a bit more readable if you used `if` to distinguish
both cases, since it's always one of the two (assuming that `entities`
is not None) cases.

e.g. like this:

.with_optional_child(entities.as_ref().and_then(|entities| {
    if entities.is_empty() {
        Row::new()
            .padding(4)
            .gap(2)
            .with_child(Fa::new("info-circle").fixed_width())
            .with_child(&metrics_empty_message)
    } else {
	TopEntities::new(entities, metrics_title, threshold)
    }
}))

What do you think?

>          .with_optional_child((!top_entities.has_data()).then_some(loading_column()))
>          .with_optional_child(
>              top_entities