Re: [pdm-devel] [PATCH datacenter-manager v2 11/13] ui: add remote update view

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Nice work overall, some things that I noticed inline.

Am 17.10.25 um 14:10 schrieb Lukas Wagner:
> +fn render_remote_summary_counter(count: u32, task_class: RemoteSummaryIcon) -> Html {
> +    let (icon_class, icon_scheme, state_text) = match task_class {
> +        RemoteSummaryIcon::UpToDate => (
> +            "check",
> +            FontColor::Success,
> +            tr!("One node is up-to-date." | "{n} nodes are up-to-date." % count),
> +        ),
> +        RemoteSummaryIcon::Error => (
> +            "times-circle",
> +            FontColor::Error,
> +            tr!("Failed to retrieve update info for one node."
> +                | "Failed to retrieve update info for {n} nodes." % count),
> +        ),
> +        RemoteSummaryIcon::Updatable => (
> +            "refresh",
> +            FontColor::Primary,
> +            tr!("One node has updates available." | "{n} nodes have updates available." % count),
> +        ),
> +    };
> +
> +    let icon = Fa::new(icon_class).margin_end(3).class(icon_scheme);
> +
> +    Tooltip::new(
> +        Container::from_tag("span")
> +            .with_child(icon)
> +            .with_child(count)
> +            .margin_end(5),
> +    )
> +    .tip(state_text)

I found the standalone numbers a bit confusing, i.e. interpreted them as "X updates
available" at first. As we got the horizontal space, what about moving the state_text
out of a tooltip and make it the always visible text?

btw. could be nice to show single-node remotes (PBS, single-node PVEs and potentially
PMG in the future) directly at the top level, i.e. without a nesting level indirection.
That would save a bit vertical space and avoid clicks.

Some other things that might not belong to this reply but I noticed:
- do we have the last apt update time available? could be nice to show that as column,
  e.g. colored as warning if it's older than a day or so (but can be added anytime so
  definitively not a blocker now).

- Might be nicer to add the horizontal scrolling to the inner views, as with 1440x900
  there are already columns cut-off in the Update List view on the right, and that
  resolution is definitively one that should still be usable (but doesn't have to look
  great).

- Repo state would be really good to see here, as else one might get a false sense
  of security/safety if all is green checkmarks, but that then being the result of
  bad/no repos configured over the system being actually fully up-to-date.

Besides the unlabeled number these can all be follow ups (if at all), so I'm fine with
applying this as is, but you might have a better gut feeling if it's fine to do follow-ups
over a v3, so just tell me what you prefer.

> +    .into()
> +}

...

> +    fn render_update_list_panel(&self, ctx: &LoadableComponentContext<Self>) -> Panel {
> +        let title: Html = Row::new()
> +            .gap(2)
> +            .class(AlignItems::Baseline)
> +            .with_child(Fa::new("list"))
> +            .with_child(tr!("Update List"))
> +            .into();

Might be nice to see the selected nodename in the title, especially with many remotes/nodes
and the right list having been scrolled so that the selected one is out of view.




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