public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH pve_flutter_frontend v2] feat: ui: add edit button in guests options page
@ 2025-08-29 15:18 Shan Shaji
  2025-09-02 11:09 ` Michael Köppl
  0 siblings, 1 reply; 3+ messages in thread
From: Shan Shaji @ 2025-08-29 15:18 UTC (permalink / raw)
  To: pve-devel; +Cc: Thomas Lamprecht

On the options page for VMs and CTs it was easy to change the
configs by mistake. To avoid that, added an edit button on the top
of the screen. The toggle buttons will only be enabled if the edit
button is clicked.

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---

 changes since v1:
 - Rebase with master

 lib/bloc/pve_lxc_overview_bloc.dart          | 11 +++++
 lib/bloc/pve_qemu_overview_bloc.dart         | 11 +++++
 lib/states/pve_lxc_overview_state.dart       | 21 ++++++----
 lib/states/pve_qemu_overview_state.dart      | 21 ++++++----
 lib/widgets/pve_config_switch_list_tile.dart |  8 +++-
 lib/widgets/pve_icon_button_widget.dart      | 43 ++++++++++++++++++++
 lib/widgets/pve_lxc_options_widget.dart      | 16 +++++++-
 lib/widgets/pve_lxc_overview.dart            | 22 ++++++----
 lib/widgets/pve_qemu_options_widget.dart     | 17 ++++++++
 lib/widgets/pve_qemu_overview.dart           |  2 +-
 10 files changed, 143 insertions(+), 29 deletions(-)
 create mode 100644 lib/widgets/pve_icon_button_widget.dart

diff --git a/lib/bloc/pve_lxc_overview_bloc.dart b/lib/bloc/pve_lxc_overview_bloc.dart
index e287f97..b856006 100644
--- a/lib/bloc/pve_lxc_overview_bloc.dart
+++ b/lib/bloc/pve_lxc_overview_bloc.dart
@@ -89,6 +89,11 @@ class PveLxcOverviewBloc
         yield latestState.rebuild((b) => b..errorMessage = '');
       }
     }
+
+    if (event is LockLxcOptions) {
+      yield latestState
+          .rebuild((b) => b..isOptionsLocked = event.isLockOptions);
+    }
   }
 
   Future<List<PveGuestRRDdataModel>> _preProcessRRDdata() async {
@@ -131,3 +136,9 @@ class RevertPendingLxcConfig extends PveLxcOverviewEvent {
 
   RevertPendingLxcConfig(this.cField);
 }
+
+class LockLxcOptions extends PveLxcOverviewEvent {
+  final bool isLockOptions;
+
+  LockLxcOptions(this.isLockOptions);
+}
diff --git a/lib/bloc/pve_qemu_overview_bloc.dart b/lib/bloc/pve_qemu_overview_bloc.dart
index 3d0fd0e..98b1261 100644
--- a/lib/bloc/pve_qemu_overview_bloc.dart
+++ b/lib/bloc/pve_qemu_overview_bloc.dart
@@ -94,6 +94,11 @@ class PveQemuOverviewBloc
         yield latestState.rebuild((b) => b..errorMessage = '');
       }
     }
+
+    if (event is LockQemuOptions) {
+      yield latestState
+          .rebuild((b) => b..isOptionsLocked = event.isLockOptions);
+    }
   }
 
   Future<List<PveGuestRRDdataModel>> _preProcessRRDdata() async {
@@ -136,3 +141,9 @@ class RevertPendingQemuConfig extends PveQemuOverviewEvent {
 
   RevertPendingQemuConfig(this.cField);
 }
+
+class LockQemuOptions extends PveQemuOverviewEvent {
+  final bool isLockOptions;
+
+  LockQemuOptions(this.isLockOptions);
+}
\ No newline at end of file
diff --git a/lib/states/pve_lxc_overview_state.dart b/lib/states/pve_lxc_overview_state.dart
index c10c2e7..a162121 100644
--- a/lib/states/pve_lxc_overview_state.dart
+++ b/lib/states/pve_lxc_overview_state.dart
@@ -10,6 +10,7 @@ abstract class PveLxcOverviewState
     implements Built<PveLxcOverviewState, PveLxcOverviewStateBuilder> {
   // Fields
   String get nodeID;
+  bool get isOptionsLocked;
   PveNodesLxcStatusModel? get currentStatus;
   BuiltList<PveGuestRRDdataModel>? get rrdData;
   PveNodesLxcConfigModel? get config;
@@ -20,13 +21,15 @@ abstract class PveLxcOverviewState
           [void Function(PveLxcOverviewStateBuilder) updates]) =
       _$PveLxcOverviewState;
 
-  factory PveLxcOverviewState.init(String nodeID) =>
-      PveLxcOverviewState((b) => b
-        //base
-        ..errorMessage = ''
-        ..isBlank = true
-        ..isLoading = false
-        ..isSuccess = false
-        //class
-        ..nodeID = nodeID);
+  factory PveLxcOverviewState.init(String nodeID) => PveLxcOverviewState(
+        (b) => b
+          //base
+          ..errorMessage = ''
+          ..isBlank = true
+          ..isLoading = false
+          ..isSuccess = false
+          //class
+          ..nodeID = nodeID
+          ..isOptionsLocked = true,
+      );
 }
diff --git a/lib/states/pve_qemu_overview_state.dart b/lib/states/pve_qemu_overview_state.dart
index 43201bc..8d8dd96 100644
--- a/lib/states/pve_qemu_overview_state.dart
+++ b/lib/states/pve_qemu_overview_state.dart
@@ -8,6 +8,7 @@ abstract class PveQemuOverviewState
     with PveBaseState
     implements Built<PveQemuOverviewState, PveQemuOverviewStateBuilder> {
   String get nodeID;
+  bool get isOptionsLocked;
   PveQemuStatusModel? get currentStatus;
   BuiltList<PveGuestRRDdataModel>? get rrdData;
   PveNodesQemuConfigModel? get config;
@@ -18,13 +19,15 @@ abstract class PveQemuOverviewState
           [void Function(PveQemuOverviewStateBuilder) updates]) =
       _$PveQemuOverviewState;
 
-  factory PveQemuOverviewState.init(String nodeID) =>
-      PveQemuOverviewState((b) => b
-        //base
-        ..errorMessage = ''
-        ..isBlank = true
-        ..isLoading = false
-        ..isSuccess = false
-        //class
-        ..nodeID = nodeID);
+  factory PveQemuOverviewState.init(String nodeID) => PveQemuOverviewState(
+        (b) => b
+          //base
+          ..errorMessage = ''
+          ..isBlank = true
+          ..isLoading = false
+          ..isSuccess = false
+          //class
+          ..nodeID = nodeID
+          ..isOptionsLocked = true,
+      );
 }
diff --git a/lib/widgets/pve_config_switch_list_tile.dart b/lib/widgets/pve_config_switch_list_tile.dart
index c209fbe..1f2e6c0 100644
--- a/lib/widgets/pve_config_switch_list_tile.dart
+++ b/lib/widgets/pve_config_switch_list_tile.dart
@@ -7,6 +7,7 @@ class PveConfigSwitchListTile extends StatelessWidget {
   final Widget? title;
   final ValueChanged<bool>? onChanged;
   final VoidCallback? onDeleted;
+  final bool disable;
 
   const PveConfigSwitchListTile({
     super.key,
@@ -16,6 +17,7 @@ class PveConfigSwitchListTile extends StatelessWidget {
     this.title,
     this.onChanged,
     this.onDeleted,
+    this.disable = false,
   });
   @override
   Widget build(BuildContext context) {
@@ -26,7 +28,11 @@ class PveConfigSwitchListTile extends StatelessWidget {
     return SwitchListTile(
       title: _getTitle(),
       value: pBool ?? value ?? defaultValue!,
-      onChanged: pending != null ? null : onChanged,
+      onChanged: disable
+          ? null
+          : pending != null
+              ? null
+              : onChanged,
     );
   }
 
diff --git a/lib/widgets/pve_icon_button_widget.dart b/lib/widgets/pve_icon_button_widget.dart
new file mode 100644
index 0000000..0d30ce8
--- /dev/null
+++ b/lib/widgets/pve_icon_button_widget.dart
@@ -0,0 +1,43 @@
+import 'package:flutter/material.dart';
+
+class PveIconButton extends StatelessWidget {
+  final IconData icon;
+  final String label;
+  final Color? color;
+  final VoidCallback? onPressed;
+
+  const PveIconButton({
+    super.key,
+    required this.icon,
+    required this.label,
+    this.color,
+    this.onPressed,
+  });
+
+  const PveIconButton.edit({
+    super.key,
+    this.color,
+    this.onPressed,
+  })  : icon = Icons.edit,
+        label = 'Edit';
+
+  @override
+  Widget build(BuildContext context) {
+    return TextButton(
+      onPressed: onPressed,
+      child: Row(
+        spacing: 2,
+        children: [
+          Icon(
+            icon,
+            color: color,
+          ),
+          Text(
+            label,
+            style: TextStyle(color: color),
+          ),
+        ],
+      ),
+    );
+  }
+}
diff --git a/lib/widgets/pve_lxc_options_widget.dart b/lib/widgets/pve_lxc_options_widget.dart
index 7ad0224..6adc6d9 100644
--- a/lib/widgets/pve_lxc_options_widget.dart
+++ b/lib/widgets/pve_lxc_options_widget.dart
@@ -3,6 +3,7 @@ import 'package:pve_flutter_frontend/bloc/pve_lxc_overview_bloc.dart';
 import 'package:pve_flutter_frontend/states/pve_lxc_overview_state.dart';
 import 'package:pve_flutter_frontend/widgets/proxmox_stream_builder_widget.dart';
 import 'package:pve_flutter_frontend/widgets/pve_config_switch_list_tile.dart';
+import 'package:pve_flutter_frontend/widgets/pve_icon_button_widget.dart';
 
 class PveLxcOptions extends StatelessWidget {
   final PveLxcOverviewBloc? lxcBloc;
@@ -16,7 +17,17 @@ class PveLxcOptions extends StatelessWidget {
           final config = state.config;
           if (config != null) {
             return Scaffold(
-                appBar: AppBar(),
+                appBar: AppBar(
+                  actions: [
+                    if (state.isOptionsLocked)
+                      PveIconButton.edit(
+                        color: Theme.of(context).colorScheme.onPrimary,
+                        onPressed: () => lxcBloc!.events.add(
+                          LockLxcOptions(false),
+                        ),
+                      )
+                  ],
+                ),
                 body: SingleChildScrollView(
                   child: Column(
                     children: <Widget>[
@@ -25,6 +36,7 @@ class PveLxcOptions extends StatelessWidget {
                         subtitle: Text(config.hostname ?? 'undefined'),
                       ),
                       PveConfigSwitchListTile(
+                        disable: state.isOptionsLocked,
                         title: const Text("Start on boot"),
                         value: config.onboot,
                         defaultValue: false,
@@ -47,6 +59,7 @@ class PveLxcOptions extends StatelessWidget {
                         subtitle: Text("${config.arch}"),
                       ),
                       PveConfigSwitchListTile(
+                        disable: state.isOptionsLocked,
                         title: const Text("/dev/console"),
                         value: config.console,
                         defaultValue: true,
@@ -65,6 +78,7 @@ class PveLxcOptions extends StatelessWidget {
                         subtitle: Text(config.cmode?.name ?? 'tty'),
                       ),
                       PveConfigSwitchListTile(
+                        disable: state.isOptionsLocked,
                         title: const Text("Protection"),
                         value: config.protection,
                         defaultValue: false,
diff --git a/lib/widgets/pve_lxc_overview.dart b/lib/widgets/pve_lxc_overview.dart
index fe43a26..3d66207 100644
--- a/lib/widgets/pve_lxc_overview.dart
+++ b/lib/widgets/pve_lxc_overview.dart
@@ -151,14 +151,20 @@ class PveLxcOverview extends StatelessWidget {
                                         state.nodeID,
                                         'lxc')),
                               createActionCard(
-                                  'Options',
-                                  Icons.settings,
-                                  () => Navigator.of(context)
-                                      .push(MaterialPageRoute(
-                                          builder: (context) => PveLxcOptions(
-                                                lxcBloc: lxcBloc,
-                                              ),
-                                          fullscreenDialog: true))),
+                                'Options',
+                                Icons.settings,
+                                () {
+                                  return Navigator.of(context).push(
+                                    MaterialPageRoute(
+                                      builder: (context) => PveLxcOptions(
+                                        lxcBloc: lxcBloc
+                                          ..events.add(LockLxcOptions(true)),
+                                      ),
+                                      fullscreenDialog: true,
+                                    ),
+                                  );
+                                },
+                              ),
                               if (!resourceBloc.latestState.isStandalone)
                                 createActionCard(
                                     'Migrate',
diff --git a/lib/widgets/pve_qemu_options_widget.dart b/lib/widgets/pve_qemu_options_widget.dart
index 7ed0a3e..2c3f05f 100644
--- a/lib/widgets/pve_qemu_options_widget.dart
+++ b/lib/widgets/pve_qemu_options_widget.dart
@@ -4,6 +4,7 @@ import 'package:pve_flutter_frontend/bloc/pve_qemu_overview_bloc.dart';
 import 'package:pve_flutter_frontend/states/pve_qemu_overview_state.dart';
 import 'package:pve_flutter_frontend/widgets/proxmox_stream_builder_widget.dart';
 import 'package:pve_flutter_frontend/widgets/pve_config_switch_list_tile.dart';
+import 'package:pve_flutter_frontend/widgets/pve_icon_button_widget.dart';
 
 class PveQemuOptions extends StatelessWidget {
   final String guestID;
@@ -24,6 +25,15 @@ class PveQemuOptions extends StatelessWidget {
                     icon: const Icon(Icons.close),
                     onPressed: () => Navigator.of(context).pop(),
                   ),
+                  actions: [
+                    if (state.isOptionsLocked)
+                      PveIconButton.edit(
+                        onPressed: () => bloc.events.add(
+                          LockQemuOptions(false),
+                        ),
+                        color: Theme.of(context).colorScheme.onPrimary,
+                      )
+                  ],
                 ),
                 body: SingleChildScrollView(
                   child: Form(
@@ -36,6 +46,7 @@ class PveQemuOptions extends StatelessWidget {
                           subtitle: Text(config.name ?? 'VM$guestID'),
                         ),
                         PveConfigSwitchListTile(
+                          disable: state.isOptionsLocked,
                           title: const Text("Start on boot"),
                           value: config.onboot,
                           defaultValue: false,
@@ -61,6 +72,7 @@ class PveQemuOptions extends StatelessWidget {
                           subtitle: Text(config.boot ?? 'Disk, Network, USB'),
                         ),
                         PveConfigSwitchListTile(
+                          disable: state.isOptionsLocked,
                           title: const Text("Use tablet for pointer"),
                           value: config.tablet,
                           defaultValue: true,
@@ -75,6 +87,7 @@ class PveQemuOptions extends StatelessWidget {
                           subtitle: Text(config.hotplug ?? 'disk,network,usb'),
                         ),
                         PveConfigSwitchListTile(
+                          disable: state.isOptionsLocked,
                           title: const Text("ACPI support"),
                           value: config.acpi,
                           defaultValue: true,
@@ -85,6 +98,7 @@ class PveQemuOptions extends StatelessWidget {
                               bloc.events.add(RevertPendingQemuConfig('acpi')),
                         ),
                         PveConfigSwitchListTile(
+                          disable: state.isOptionsLocked,
                           title: const Text("KVM hardware virtualization"),
                           value: config.kvm,
                           defaultValue: true,
@@ -95,6 +109,7 @@ class PveQemuOptions extends StatelessWidget {
                               bloc.events.add(RevertPendingQemuConfig('kvm')),
                         ),
                         PveConfigSwitchListTile(
+                          disable: state.isOptionsLocked,
                           title: const Text("Freeze CPU on startup"),
                           value: config.freeze,
                           defaultValue: false,
@@ -105,6 +120,7 @@ class PveQemuOptions extends StatelessWidget {
                               .add(RevertPendingQemuConfig('freeze')),
                         ),
                         PveConfigSwitchListTile(
+                          disable: state.isOptionsLocked,
                           title: const Text("Use local time for RTC"),
                           value: config.localtime,
                           defaultValue: false,
@@ -128,6 +144,7 @@ class PveQemuOptions extends StatelessWidget {
                           subtitle: Text(config.agent ?? 'Default (disabled)'),
                         ),
                         PveConfigSwitchListTile(
+                          disable: state.isOptionsLocked,
                           title: const Text("Protection"),
                           value: config.protection,
                           defaultValue: false,
diff --git a/lib/widgets/pve_qemu_overview.dart b/lib/widgets/pve_qemu_overview.dart
index 50f7382..473731c 100644
--- a/lib/widgets/pve_qemu_overview.dart
+++ b/lib/widgets/pve_qemu_overview.dart
@@ -257,7 +257,7 @@ class PveQemuOverview extends StatelessWidget {
   Route _createOptionsRoute(PveQemuOverviewBloc bloc) {
     return PageRouteBuilder(
       pageBuilder: (context, animation, secondaryAnimation) => Provider.value(
-        value: bloc,
+        value: bloc..events.add(LockQemuOptions(true)),
         child: PveQemuOptions(
           guestID: guestID,
         ),
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH pve_flutter_frontend v2] feat: ui: add edit button in guests options page
  2025-08-29 15:18 [pve-devel] [PATCH pve_flutter_frontend v2] feat: ui: add edit button in guests options page Shan Shaji
@ 2025-09-02 11:09 ` Michael Köppl
  2025-09-03  8:42   ` Shan Shaji
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Köppl @ 2025-09-02 11:09 UTC (permalink / raw)
  To: Proxmox VE development discussion, Shan Shaji; +Cc: pve-devel, Thomas Lamprecht

Gave this a spin in my Android emulator. Works as expected. I also
checked that there's no state change, etc. when rotating to landscape
and back and that options are locked again upon re-opening the VM/LXC
options view. Did not notice anything off.

Consider this
Tested-by: Michael Köppl <m.koeppl@proxmox.com>

I also had a look at the implementation and left 4 comments inline.

On Fri Aug 29, 2025 at 5:18 PM CEST, Shan Shaji wrote:
> On the options page for VMs and CTs it was easy to change the
> configs by mistake. To avoid that, added an edit button on the top
> of the screen. The toggle buttons will only be enabled if the edit
> button is clicked.
>
> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>
>  changes since v1:
>  - Rebase with master
>
>  lib/bloc/pve_lxc_overview_bloc.dart          | 11 +++++
>  lib/bloc/pve_qemu_overview_bloc.dart         | 11 +++++
>  lib/states/pve_lxc_overview_state.dart       | 21 ++++++----
>  lib/states/pve_qemu_overview_state.dart      | 21 ++++++----
>  lib/widgets/pve_config_switch_list_tile.dart |  8 +++-
>  lib/widgets/pve_icon_button_widget.dart      | 43 ++++++++++++++++++++
>  lib/widgets/pve_lxc_options_widget.dart      | 16 +++++++-
>  lib/widgets/pve_lxc_overview.dart            | 22 ++++++----
>  lib/widgets/pve_qemu_options_widget.dart     | 17 ++++++++
>  lib/widgets/pve_qemu_overview.dart           |  2 +-
>  10 files changed, 143 insertions(+), 29 deletions(-)
>  create mode 100644 lib/widgets/pve_icon_button_widget.dart
>
> diff --git a/lib/bloc/pve_lxc_overview_bloc.dart b/lib/bloc/pve_lxc_overview_bloc.dart
> index e287f97..b856006 100644
> --- a/lib/bloc/pve_lxc_overview_bloc.dart
> +++ b/lib/bloc/pve_lxc_overview_bloc.dart
> @@ -89,6 +89,11 @@ class PveLxcOverviewBloc
>          yield latestState.rebuild((b) => b..errorMessage = '');
>        }
>      }
> +
> +    if (event is LockLxcOptions) {
> +      yield latestState
> +          .rebuild((b) => b..isOptionsLocked = event.isLockOptions);
> +    }
>    }
>  
>    Future<List<PveGuestRRDdataModel>> _preProcessRRDdata() async {
> @@ -131,3 +136,9 @@ class RevertPendingLxcConfig extends PveLxcOverviewEvent {
>  
>    RevertPendingLxcConfig(this.cField);
>  }
> +
> +class LockLxcOptions extends PveLxcOverviewEvent {
> +  final bool isLockOptions;

nit: I think isOptionsLocked would work better here as well.

> +
> +  LockLxcOptions(this.isLockOptions);
> +}
> diff --git a/lib/bloc/pve_qemu_overview_bloc.dart b/lib/bloc/pve_qemu_overview_bloc.dart
> index 3d0fd0e..98b1261 100644
> --- a/lib/bloc/pve_qemu_overview_bloc.dart
> +++ b/lib/bloc/pve_qemu_overview_bloc.dart
> @@ -94,6 +94,11 @@ class PveQemuOverviewBloc
>          yield latestState.rebuild((b) => b..errorMessage = '');
>        }
>      }
> +
> +    if (event is LockQemuOptions) {
> +      yield latestState
> +          .rebuild((b) => b..isOptionsLocked = event.isLockOptions);
> +    }
>    }
>  
>    Future<List<PveGuestRRDdataModel>> _preProcessRRDdata() async {
> @@ -136,3 +141,9 @@ class RevertPendingQemuConfig extends PveQemuOverviewEvent {
>  
>    RevertPendingQemuConfig(this.cField);
>  }
> +
> +class LockQemuOptions extends PveQemuOverviewEvent {
> +  final bool isLockOptions;
> +
> +  LockQemuOptions(this.isLockOptions);
> +}
> \ No newline at end of file
> diff --git a/lib/states/pve_lxc_overview_state.dart b/lib/states/pve_lxc_overview_state.dart
> index c10c2e7..a162121 100644
> --- a/lib/states/pve_lxc_overview_state.dart
> +++ b/lib/states/pve_lxc_overview_state.dart
> @@ -10,6 +10,7 @@ abstract class PveLxcOverviewState
>      implements Built<PveLxcOverviewState, PveLxcOverviewStateBuilder> {
>    // Fields
>    String get nodeID;
> +  bool get isOptionsLocked;
>    PveNodesLxcStatusModel? get currentStatus;
>    BuiltList<PveGuestRRDdataModel>? get rrdData;
>    PveNodesLxcConfigModel? get config;
> @@ -20,13 +21,15 @@ abstract class PveLxcOverviewState
>            [void Function(PveLxcOverviewStateBuilder) updates]) =
>        _$PveLxcOverviewState;
>  
> -  factory PveLxcOverviewState.init(String nodeID) =>
> -      PveLxcOverviewState((b) => b
> -        //base
> -        ..errorMessage = ''
> -        ..isBlank = true
> -        ..isLoading = false
> -        ..isSuccess = false
> -        //class
> -        ..nodeID = nodeID);
> +  factory PveLxcOverviewState.init(String nodeID) => PveLxcOverviewState(
> +        (b) => b
> +          //base
> +          ..errorMessage = ''
> +          ..isBlank = true
> +          ..isLoading = false
> +          ..isSuccess = false
> +          //class
> +          ..nodeID = nodeID
> +          ..isOptionsLocked = true,
> +      );
>  }
> diff --git a/lib/states/pve_qemu_overview_state.dart b/lib/states/pve_qemu_overview_state.dart
> index 43201bc..8d8dd96 100644
> --- a/lib/states/pve_qemu_overview_state.dart
> +++ b/lib/states/pve_qemu_overview_state.dart
> @@ -8,6 +8,7 @@ abstract class PveQemuOverviewState
>      with PveBaseState
>      implements Built<PveQemuOverviewState, PveQemuOverviewStateBuilder> {
>    String get nodeID;
> +  bool get isOptionsLocked;
>    PveQemuStatusModel? get currentStatus;
>    BuiltList<PveGuestRRDdataModel>? get rrdData;
>    PveNodesQemuConfigModel? get config;
> @@ -18,13 +19,15 @@ abstract class PveQemuOverviewState
>            [void Function(PveQemuOverviewStateBuilder) updates]) =
>        _$PveQemuOverviewState;
>  
> -  factory PveQemuOverviewState.init(String nodeID) =>
> -      PveQemuOverviewState((b) => b
> -        //base
> -        ..errorMessage = ''
> -        ..isBlank = true
> -        ..isLoading = false
> -        ..isSuccess = false
> -        //class
> -        ..nodeID = nodeID);
> +  factory PveQemuOverviewState.init(String nodeID) => PveQemuOverviewState(
> +        (b) => b
> +          //base
> +          ..errorMessage = ''
> +          ..isBlank = true
> +          ..isLoading = false
> +          ..isSuccess = false
> +          //class
> +          ..nodeID = nodeID
> +          ..isOptionsLocked = true,
> +      );
>  }
> diff --git a/lib/widgets/pve_config_switch_list_tile.dart b/lib/widgets/pve_config_switch_list_tile.dart
> index c209fbe..1f2e6c0 100644
> --- a/lib/widgets/pve_config_switch_list_tile.dart
> +++ b/lib/widgets/pve_config_switch_list_tile.dart
> @@ -7,6 +7,7 @@ class PveConfigSwitchListTile extends StatelessWidget {
>    final Widget? title;
>    final ValueChanged<bool>? onChanged;
>    final VoidCallback? onDeleted;
> +  final bool disable;
>  
>    const PveConfigSwitchListTile({
>      super.key,
> @@ -16,6 +17,7 @@ class PveConfigSwitchListTile extends StatelessWidget {
>      this.title,
>      this.onChanged,
>      this.onDeleted,
> +    this.disable = false,
>    });
>    @override
>    Widget build(BuildContext context) {
> @@ -26,7 +28,11 @@ class PveConfigSwitchListTile extends StatelessWidget {
>      return SwitchListTile(
>        title: _getTitle(),
>        value: pBool ?? value ?? defaultValue!,
> -      onChanged: pending != null ? null : onChanged,
> +      onChanged: disable
> +          ? null
> +          : pending != null
> +              ? null
> +              : onChanged,

Wouldn't

    disable || pending != null ? null : onChanged

work here as well?

>      );
>    }
>  
> diff --git a/lib/widgets/pve_icon_button_widget.dart b/lib/widgets/pve_icon_button_widget.dart
> new file mode 100644
> index 0000000..0d30ce8
> --- /dev/null
> +++ b/lib/widgets/pve_icon_button_widget.dart
> @@ -0,0 +1,43 @@
> +import 'package:flutter/material.dart';
> +
> +class PveIconButton extends StatelessWidget {
> +  final IconData icon;
> +  final String label;
> +  final Color? color;
> +  final VoidCallback? onPressed;
> +
> +  const PveIconButton({
> +    super.key,
> +    required this.icon,
> +    required this.label,
> +    this.color,
> +    this.onPressed,
> +  });
> +
> +  const PveIconButton.edit({
> +    super.key,
> +    this.color,
> +    this.onPressed,
> +  })  : icon = Icons.edit,
> +        label = 'Edit';

I don't know what @Thomas initially suggested, but I think what would
work well here is a "lock", so either a button or a toggle to unlock and
lock the settings, so you could also revert back to a read-only view
without closing the view and re-opening it. It's just a suggestion from
my side though. I think a button would have the advantage that it could
also either show an unlocked or a locked icon depending on the state,
which would illustrate to the user what the button does without
additional text (though a tooltip thingy when long-pressing on it could
be added). It would work with a toggle as well, but I think icons and
toggles are not usually used together (at least in Android).

> +
> +  @override
> +  Widget build(BuildContext context) {
> +    return TextButton(
> +      onPressed: onPressed,
> +      child: Row(
> +        spacing: 2,
> +        children: [
> +          Icon(
> +            icon,
> +            color: color,
> +          ),
> +          Text(
> +            label,
> +            style: TextStyle(color: color),
> +          ),
> +        ],
> +      ),
> +    );
> +  }
> +}
> diff --git a/lib/widgets/pve_lxc_options_widget.dart b/lib/widgets/pve_lxc_options_widget.dart
> index 7ad0224..6adc6d9 100644
> --- a/lib/widgets/pve_lxc_options_widget.dart
> +++ b/lib/widgets/pve_lxc_options_widget.dart
> @@ -3,6 +3,7 @@ import 'package:pve_flutter_frontend/bloc/pve_lxc_overview_bloc.dart';
>  import 'package:pve_flutter_frontend/states/pve_lxc_overview_state.dart';
>  import 'package:pve_flutter_frontend/widgets/proxmox_stream_builder_widget.dart';
>  import 'package:pve_flutter_frontend/widgets/pve_config_switch_list_tile.dart';
> +import 'package:pve_flutter_frontend/widgets/pve_icon_button_widget.dart';
>  
>  class PveLxcOptions extends StatelessWidget {
>    final PveLxcOverviewBloc? lxcBloc;
> @@ -16,7 +17,17 @@ class PveLxcOptions extends StatelessWidget {
>            final config = state.config;
>            if (config != null) {
>              return Scaffold(
> -                appBar: AppBar(),
> +                appBar: AppBar(
> +                  actions: [
> +                    if (state.isOptionsLocked)
> +                      PveIconButton.edit(
> +                        color: Theme.of(context).colorScheme.onPrimary,
> +                        onPressed: () => lxcBloc!.events.add(
> +                          LockLxcOptions(false),
> +                        ),
> +                      )
> +                  ],
> +                ),
>                  body: SingleChildScrollView(
>                    child: Column(
>                      children: <Widget>[
> @@ -25,6 +36,7 @@ class PveLxcOptions extends StatelessWidget {
>                          subtitle: Text(config.hostname ?? 'undefined'),
>                        ),
>                        PveConfigSwitchListTile(
> +                        disable: state.isOptionsLocked,
>                          title: const Text("Start on boot"),
>                          value: config.onboot,
>                          defaultValue: false,
> @@ -47,6 +59,7 @@ class PveLxcOptions extends StatelessWidget {
>                          subtitle: Text("${config.arch}"),
>                        ),
>                        PveConfigSwitchListTile(
> +                        disable: state.isOptionsLocked,
>                          title: const Text("/dev/console"),
>                          value: config.console,
>                          defaultValue: true,
> @@ -65,6 +78,7 @@ class PveLxcOptions extends StatelessWidget {
>                          subtitle: Text(config.cmode?.name ?? 'tty'),
>                        ),
>                        PveConfigSwitchListTile(
> +                        disable: state.isOptionsLocked,
>                          title: const Text("Protection"),
>                          value: config.protection,
>                          defaultValue: false,
> diff --git a/lib/widgets/pve_lxc_overview.dart b/lib/widgets/pve_lxc_overview.dart
> index fe43a26..3d66207 100644
> --- a/lib/widgets/pve_lxc_overview.dart
> +++ b/lib/widgets/pve_lxc_overview.dart
> @@ -151,14 +151,20 @@ class PveLxcOverview extends StatelessWidget {
>                                          state.nodeID,
>                                          'lxc')),
>                                createActionCard(
> -                                  'Options',
> -                                  Icons.settings,
> -                                  () => Navigator.of(context)
> -                                      .push(MaterialPageRoute(
> -                                          builder: (context) => PveLxcOptions(
> -                                                lxcBloc: lxcBloc,
> -                                              ),
> -                                          fullscreenDialog: true))),
> +                                'Options',
> +                                Icons.settings,
> +                                () {
> +                                  return Navigator.of(context).push(
> +                                    MaterialPageRoute(
> +                                      builder: (context) => PveLxcOptions(
> +                                        lxcBloc: lxcBloc
> +                                          ..events.add(LockLxcOptions(true)),
> +                                      ),
> +                                      fullscreenDialog: true,
> +                                    ),
> +                                  );
> +                                },
> +                              ),

nit: was this changed by the formatter? I see that you added the
..events.add, but the formatting change from

    () => ...

to

    () {
        return ...
    }

seems kind of unrelated.



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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH pve_flutter_frontend v2] feat: ui: add edit button in guests options page
  2025-09-02 11:09 ` Michael Köppl
@ 2025-09-03  8:42   ` Shan Shaji
  0 siblings, 0 replies; 3+ messages in thread
From: Shan Shaji @ 2025-09-03  8:42 UTC (permalink / raw)
  To: Michael Köppl, Proxmox VE development discussion
  Cc: pve-devel, Thomas Lamprecht

Thanks for the review. Will send an updated patch.  

On Tue Sep 2, 2025 at 1:09 PM CEST, Michael Köppl wrote:
> Gave this a spin in my Android emulator. Works as expected. I also
> checked that there's no state change, etc. when rotating to landscape
> and back and that options are locked again upon re-opening the VM/LXC
> options view. Did not notice anything off.
>
> Consider this
> Tested-by: Michael Köppl <m.koeppl@proxmox.com>
>
>> +class LockLxcOptions extends PveLxcOverviewEvent {
>> +  final bool isLockOptions;
>
> nit: I think isOptionsLocked would work better here as well.

Yeah, makes sense. will update it accordingly. 

>>    @override
>>    Widget build(BuildContext context) {
>> @@ -26,7 +28,11 @@ class PveConfigSwitchListTile extends StatelessWidget {
>>      return SwitchListTile(
>>        title: _getTitle(),
>>        value: pBool ?? value ?? defaultValue!,
>> -      onChanged: pending != null ? null : onChanged,
>> +      onChanged: disable
>> +          ? null
>> +          : pending != null
>> +              ? null
>> +              : onChanged,
>
> Wouldn't
>
>     disable || pending != null ? null : onChanged
>
> work here as well?
>
>>      );
>>    }

Yes, Thank you. Will update it. 
>>  
>> diff --git a/lib/widgets/pve_icon_button_widget.dart b/lib/widgets/pve_icon_button_widget.dart
>> new file mode 100644
>> index 0000000..0d30ce8
>> --- /dev/null
>> +++ b/lib/widgets/pve_icon_button_widget.dart
>> @@ -0,0 +1,43 @@
>> +import 'package:flutter/material.dart';
>> +
>> +class PveIconButton extends StatelessWidget {
>> +  final IconData icon;
>> +  final String label;
>> +  final Color? color;
>> +  final VoidCallback? onPressed;
>> +
>> +  const PveIconButton({
>> +    super.key,
>> +    required this.icon,
>> +    required this.label,
>> +    this.color,
>> +    this.onPressed,
>> +  });
>> +
>> +  const PveIconButton.edit({
>> +    super.key,
>> +    this.color,
>> +    this.onPressed,
>> +  })  : icon = Icons.edit,
>> +        label = 'Edit';
>
> I don't know what @Thomas initially suggested,  but I think what would
> work well here is a "lock", so either a button or a toggle to unlock and
> lock the settings, so you could also revert back to a read-only view
> without closing the view and re-opening it. It's just a suggestion from
> my side though. I think a button would have the advantage that it could
> also either show an unlocked or a locked icon depending on the state,
> which would illustrate to the user what the button does without
> additional text (though a tooltip thingy when long-pressing on it could
> be added). It would work with a toggle as well, but I think icons and
> toggles are not usually used together (at least in Android).

IMHO, I think toggle might not be nice,  i liked your idea on using a lock and
unlock icon with the text. I think it's more user friendly than a toggle
with just icon. It will be like "<icon> (lock | unlock)". I will create 
another patch including that change. 

>> +
>> +  @override
>> +  Widget build(BuildContext context) {
>> +    return TextButton(
>> +      onPressed: onPressed,
>> +      child: Row(
>> +        spacing: 2,
>> +        children: [
>> +          Icon(
>> +            icon,
>> +            color: color,
>> +          ),
>> +          Text(
>> +            label,
>> +            style: TextStyle(color: color),
>> +          ),
>> +        ],
>> +      ),
>> +    );
>> +  }
>> +}
>
> nit: was this changed by the formatter? I see that you added the
> ..events.add, but the formatting change from

This was not an intented change. Thanks for pointing out.

>     () => ...
>
> to
>
>     () {
>         return ...
>     }
>
> seems kind of unrelated.



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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-09-03  8:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-29 15:18 [pve-devel] [PATCH pve_flutter_frontend v2] feat: ui: add edit button in guests options page Shan Shaji
2025-09-02 11:09 ` Michael Köppl
2025-09-03  8:42   ` Shan Shaji

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal