Re: [pve-devel] [PATCH pve_flutter_frontend v2] feat: ui: add edit button in guests options page

["Michael Köppl" <m.koeppl@proxmox.com>]

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