From: "Michael Köppl" <m.koeppl@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Alexander Abraham <a.abraham@proxmox.com>
Subject: Re: [pve-devel] [PATCH flutter_frontend 1/1] fix #4976: ui: Nodes offline
Date: Tue, 24 Jun 2025 14:53:46 +0200 [thread overview]
Message-ID: <03cea113-d66c-472c-ae4b-83829e7816b9@proxmox.com> (raw)
In-Reply-To: <3615656b-a9c5-430c-b0c2-57c72ac0854b@proxmox.com>
On 5/2/25 13:32, Michael Köppl wrote:
> On 4/29/25 15:16, Alexander Abraham wrote:
>> diff --git a/lib/bloc/pve_node_overview_bloc.dart b/lib/bloc/
>> pve_node_overview_bloc.dart
>> index 19d6563..98005fd 100644
>> --- a/lib/bloc/pve_node_overview_bloc.dart
>> +++ b/lib/bloc/pve_node_overview_bloc.dart
>> @@ -1,5 +1,5 @@
>> import 'dart:async';
>> -
>> +import 'package:flutter/material.dart';
>> import 'package:proxmox_dart_api_client/proxmox_dart_api_client.dart';
>> import 'package:pve_flutter_frontend/bloc/proxmox_base_bloc.dart';
>> import 'package:pve_flutter_frontend/states/
>> pve_node_overview_state.dart';
>> @@ -8,12 +8,13 @@ class PveNodeOverviewBloc
>> extends ProxmoxBaseBloc<PveNodeOverviewEvent,
>> PveNodeOverviewState> {
>> final ProxmoxApiClient apiClient;
>> final String nodeID;
>> + final BuildContext context;
>> final PveNodeOverviewState init;
>> @override
>> PveNodeOverviewState get initialState => init;
>> PveNodeOverviewBloc(
>> - {required this.apiClient, required this.nodeID, required
>> this.init});
>> + {required this.apiClient, required this.nodeID, required
>> this.init, required this.context});
>> Timer? sTimer;
>> @override
>> @@ -32,36 +33,56 @@ class PveNodeOverviewBloc
>> @override
>> Stream<PveNodeOverviewState> processEvents(
>> PveNodeOverviewEvent event) async* {
>> - if (event is UpdateNodeStatus) {
>> - final status = await apiClient.getNodeStatus(nodeID);
>> - yield latestState.rebuild((b) => b..status.replace(status));
>> - final rrdData =
>> - await apiClient.getNodeRRDdata(nodeID,
>> PveRRDTimeframeType.hour);
>> - yield latestState.rebuild((b) => b..rrdData.replace(rrdData));
>> - final services = await apiClient.getNodeServices(nodeID);
>> - yield latestState.rebuild((b) => b..services.replace(services));
>> - try {
>> - final updates = await apiClient.getNodeAptUpdate(nodeID);
>> - yield latestState.rebuild((b) => b..updates.replace(updates));
>> - yield latestState
>> - .rebuild((b) => b..updatesQueryPermissionFailure = false);
>> - } on ProxmoxApiException catch (e) {
>> - // only throw on non permission related errors
>> - if (e.statusCode != 403) {
>> - rethrow;
>> - } else {
>> + PveNodeStatusModel? status;
>> + try {
>> + status = await apiClient.getNodeStatus(nodeID);
>> + if (event is UpdateNodeStatus) {
>> + yield latestState.rebuild((b) => b..status.replace(status!));
>> + final rrdData =
>> + await apiClient.getNodeRRDdata(nodeID,
>> PveRRDTimeframeType.hour);
>> + yield latestState.rebuild((b) => b..rrdData.replace(rrdData));
>> + final services = await apiClient.getNodeServices(nodeID);
>> + yield latestState.rebuild((b) => b..services.replace(services));
>> + try {
>> + final updates = await apiClient.getNodeAptUpdate(nodeID);
>> + yield latestState.rebuild((b) => b..updates.replace(updates));
>> yield latestState
>> - .rebuild((b) => b..updatesQueryPermissionFailure = true);
>> + .rebuild((b) => b..updatesQueryPermissionFailure = false);
>> + }
>> + on ProxmoxApiException catch (e) {
>> + // only throw on non permission related errors
>> + if (e.statusCode != 403) {
>> + rethrow;
>> + }
>> + else {
>> + yield latestState
>> + .rebuild((b) => b..updatesQueryPermissionFailure =
>> true);
>> + }
>> + }
>> + final disks = await apiClient.getNodeDisksList(nodeID);
>> + yield latestState.rebuild((b) => b..disks.replace(disks));
>> + }
>> +
>> + if (event is PerformNodeAction) {
>> + await apiClient.doResourceAction(
>> + nodeID,
>> + '',
>> + 'node',
>> + event.action,
>> + parameters: <String, String>{});
>> + yield latestState;
>> }
>> }
>> - final disks = await apiClient.getNodeDisksList(nodeID);
>> - yield latestState.rebuild((b) => b..disks.replace(disks));
>> - }
>> - if (event is PerformNodeAction) {
>> - await apiClient.doResourceAction(nodeID, '', 'node', event.action,
>> - parameters: <String, String>{});
>> - yield latestState;
>> - }
>> + catch(e){
>> + yield latestState.rebuild(
>> + (b) => b
>> + ..updatesQueryPermissionFailure = true
>> + );
>> + if (context.mounted){
>> + Navigator.of(context).pop();
>
> Instead of navigating back immediately, I think it would make sense to
> instead display an AlertDialog, showing the error message to the user
> and letting them leave the screen through a "Close" button in the
> dialog. Otherwise, it feels a bit like the app is taking control away
> from the user. We use this approach in other places in the app (e.g. .
> What do you think?
Adding to this, I think you could use something similar to what the
PveGuestBackupWidget is using:
Future<bool?> _showConfirmDialog(
BuildContext context, String title, String body) async {
return await showDialog(
context: context,
builder: (context) => AlertDialog(
title: Text(title),
content: Text(body),
actions: [
TextButton(
onPressed: () => Navigator.of(context).pop(false),
child: const Text(
'Cancel',
style: TextStyle(fontWeight: FontWeight.bold),
),
),
TextButton(
onPressed: () => Navigator.of(context).pop(true),
child: const Text(
'Confirm',
style: TextStyle(fontWeight: FontWeight.bold),
),
)
],
),
);
}
You could then wrap the Navigator.of(context).pop() like this:
final confirm = (await _showConfirmDialog(
context,
"Node offline",
"Node is offline. Do you want to navigate back?",
))!;
if (confirm && context.mounted) {
Navigator.of(context).pop();
}
I think it would also make sense to have a utils method for opening an
AlertDialog for confirmation. But that could also be done separately in
another patch. I just noticed that we're using this in multiple widgets
already.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
prev parent reply other threads:[~2025-06-24 12:53 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-29 13:16 [pve-devel] [PATCH proxmox_dart_api_client/pve_flutter_frontend_null 0/2] fix #4976 Alexander Abraham
2025-04-29 13:16 ` [pve-devel] [PATCH dart_api_client 1/1] fix #4976: Request errors are Alexander Abraham
2025-05-02 10:19 ` Michael Köppl
2025-05-05 13:03 ` Michael Köppl
2025-04-29 13:16 ` [pve-devel] [PATCH flutter_frontend 1/1] fix #4976: ui: Nodes offline Alexander Abraham
2025-05-02 11:32 ` Michael Köppl
2025-06-24 12:53 ` Michael Köppl [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=03cea113-d66c-472c-ae4b-83829e7816b9@proxmox.com \
--to=m.koeppl@proxmox.com \
--cc=a.abraham@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal