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

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

On Tue Oct 21, 2025 at 9:18 PM CEST, Thomas Lamprecht wrote:
> 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?
>

Fair point, I can see how that could be confusing.

The main issue is that a remote as a whole can have multiple states at
the same time, one for each node. For example, a remote with four nodes
could have all of these at the same time:

  - One node is up-to-date
  - Two nodes have updates available
  - One node not reachable
  - (Repo configuration issues - once implemented)

For a collapsed tree item that represents the entire remote, it's pretty
hard to 'summarise' everything into a single line in text form and still
fit the view, and that is why I went with the status icons plus the
number of nodes that have the given status.

After evaluating your suggestion, I discussed the issue with
Maximiliano. One idea that came up was to remove the counter to avoid
any confusion and to just show the icon itself. After all, at a glance,
for the summary it does not matter that much *how many* nodes have
updates, but that *some* nodes have updates available, or in general,
need attention. To get the full picture for a remote, one can always
just expand the item in the tree.

We also discussed whether it would make sense to not show the 'green
checkmark' at all in the summary (when the remote is collapsed) and just
use icons where it makes sense to draw attention from the user (updates
available, some error, warnings about the repo configuration).
This could make it easier to spot actionable remotes quickly.
When the item is expanded, we would still show the green checkmark plus
text for up-to-date nodes.

TL;DR: My next approach would be to drop the counter and also skip the
green checkmark in case everything is up-to-date (for the collapsed
remote item in the tree, the node 'leaves' would stay the same)

What do you think?

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

Yeah, I had a similar idea already, and generally I believe this could
be a good addition to improve UX. That being said, I think with the idea
described above, where we show an icon-only summary for expandable items
in the tree, it could look a bit odd if we show it directly at the top
level, since then you would have icon-only and 'icon + text' tree items
side-by-side, at the same level. I'll have to actually try this out and
see how 'bad' this is, but I'll definitely keep it in mind.

I'd say this would definitely be material for a follow-up patch.

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

Right now, we save the timestamp of when we successfully polled a list
of updates from a node, but that's purely done on the PDM side - we
don't really know how fresh the apt database on the node is and there is
at the moment no API for this. For automatic refreshes of the cached
update state, we don't actually invoke 'apt update' on the node, but
rely on the daily update task on the node itself.

A nice addition that I already had in mind would be to augment the 
GET /nodes/.../apt/update endpoint in PVE/PBS to also return the timestamp
of the last 'apt update' (or add a new endpoint, if adding it to the
existing one turns out to be hard due to the structure of the response).

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

For me horizontal scrolling already works for the update list on the
right, but maybe I misunderstand you?

Some idea that I already had was to hide the 'Description' column by
default for the 'half-width' update lists like here and then find some
other way to display the description on demand (e.g. mouse-over, or
change the 'Changelog' button to a 'Details' button, which then shows
the description *and* the changelog in a dialog)
>
> - 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.

Noted and already planned for the future, the API and the UI were
already designed with this in mind.

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

I'll post a v3 with the icon-only status column so that you can try this
out and give your opinion. v3 also fixes some small bug that I found
while toying around with different ideas for the icon-only status line.

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

Will include this as well in a v3.


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