From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id D2A861FF183 for ; Wed, 22 Oct 2025 12:22:27 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9EC5B145DA; Wed, 22 Oct 2025 12:22:53 +0200 (CEST) Mime-Version: 1.0 Date: Wed, 22 Oct 2025 12:22:19 +0200 Message-Id: From: "Lukas Wagner" To: "Thomas Lamprecht" , "Proxmox Datacenter Manager development discussion" , "Lukas Wagner" X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20251017121009.212499-1-l.wagner@proxmox.com> <20251017121009.212499-12-l.wagner@proxmox.com> <4381330b-7db7-427e-b82a-653d10dd4aa5@proxmox.com> In-Reply-To: <4381330b-7db7-427e-b82a-653d10dd4aa5@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761128531991 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.124 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH datacenter-manager v2 11/13] ui: add remote update view X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" 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) -> 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