public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH pve_flutter_frontend v4] ui: enable noVNC console on iOS
@ 2025-08-14  9:32 Shan Shaji
  2025-08-14 11:39 ` Dominik Csapak
  0 siblings, 1 reply; 4+ messages in thread
From: Shan Shaji @ 2025-08-14  9:32 UTC (permalink / raw)
  To: pve-devel

The noVNC console was disabled in iOS and was only available in android

To fix the issue enabled the noVNC console view on iOS devices. The
changes also includes a refactor of the function responsible for
displaying the webview.

Additionally, the `PveAppBar` has been added to the console view
to allow users to easily close the view. To avoid the body of the
scaffold to resize when the keyboard pops up set
`resizeToAvoidBottomInset` [0] to false.

- [0] https://api.flutter.dev/flutter/material/Scaffold/resizeToAvoidBottomInset.html

Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---

 changes since v3:
 - Remove dart formatting changes. 
 - Updated commit message. 

 changes since v2:
 - Set `resizeToAvoidBottomInset` to false to avoid the scaffold body 
   to resize automatically when the keyboard pops up. 
 - Update commit message. 

 changes since v1:
 - Rebased with master. 
 - Updated commit message. 

 lib/widgets/pve_console_menu_widget.dart | 68 +++++++++++-------------
 1 file changed, 30 insertions(+), 38 deletions(-)

diff --git a/lib/widgets/pve_console_menu_widget.dart b/lib/widgets/pve_console_menu_widget.dart
index 473595d..ab98ce4 100644
--- a/lib/widgets/pve_console_menu_widget.dart
+++ b/lib/widgets/pve_console_menu_widget.dart
@@ -9,6 +9,7 @@ import 'package:proxmox_dart_api_client/proxmox_dart_api_client.dart';
 import 'package:proxmox_login_manager/proxmox_general_settings_model.dart';
 import 'package:flutter_inappwebview/flutter_inappwebview.dart';
 import 'package:crypto/crypto.dart';
+import 'package:pve_flutter_frontend/widgets/pve_app_bar.dart';
 
 class PveConsoleMenu extends StatelessWidget {
   static const platform =
@@ -97,45 +98,18 @@ class PveConsoleMenu extends StatelessWidget {
                   }
                 },
               ),
-            if (Platform.isAndroid) // web_view is only available for mobile :(
+            // web_view is only available for mobile :(
+            // xterm.js doesn't work that well on mobile
+            if (Platform.isAndroid || Platform.isIOS)
               ListTile(
                 title: const Text(
-                  //type == "qemu" ? "noVNC Console" : "xterm.js Console",
-                  "noVNC Console", // xterm.js doesn't work that well on mobile
+                  "noVNC Console",
                   style: TextStyle(fontWeight: FontWeight.bold),
                 ),
                 subtitle: const Text("Open console view"),
-                onTap: () async {
-                  if (Platform.isAndroid) {
-                    if (['qemu', 'lxc'].contains(type)) {
-                      SystemChrome.setEnabledSystemUIMode(
-                          SystemUiMode.immersive);
-                      Navigator.of(context)
-                          .push(_createHTMLConsoleRoute())
-                          .then((completion) {
-                        SystemChrome.setEnabledSystemUIMode(
-                            SystemUiMode.edgeToEdge,
-                            overlays: [
-                              SystemUiOverlay.top,
-                              SystemUiOverlay.bottom
-                            ]);
-                      });
-                    } else if (type == 'node') {
-                      SystemChrome.setEnabledSystemUIMode(
-                          SystemUiMode.immersive);
-                      Navigator.of(context)
-                          .push(_createHTMLConsoleRoute())
-                          .then((completion) {
-                        SystemChrome.setEnabledSystemUIMode(
-                            SystemUiMode.edgeToEdge,
-                            overlays: [
-                              SystemUiOverlay.top,
-                              SystemUiOverlay.bottom
-                            ]);
-                      });
-                    }
-                  } else {
-                    print('not implemented for current platform');
+                onTap: () {
+                  if (['qemu', 'lxc'].contains(type) || type == 'node') {
+                    _openNoVncConsole(context);
                   }
                 },
               ),
@@ -145,6 +119,18 @@ class PveConsoleMenu extends StatelessWidget {
     );
   }
 
+  Future<void> _openNoVncConsole(BuildContext context) async {
+    SystemChrome.setEnabledSystemUIMode(SystemUiMode.immersive);
+    await Navigator.push(context, _createHTMLConsoleRoute());
+    SystemChrome.setEnabledSystemUIMode(
+      SystemUiMode.edgeToEdge,
+      overlays: [
+        SystemUiOverlay.top,
+        SystemUiOverlay.bottom,
+      ],
+    );
+  }
+
   void showTextDialog(BuildContext context, String title, String content) {
     showDialog(
       context: context,
@@ -232,8 +218,10 @@ class PVEWebConsoleState extends State<PVEWebConsole> {
           value: ticket,
         ),
         builder: (context, snapshot) {
-          return SafeArea(
-            child: InAppWebView(
+          return Scaffold(
+            resizeToAvoidBottomInset: false,
+            appBar: PveAppBar(),
+            body: InAppWebView(
               onReceivedServerTrustAuthRequest: (controller, challenge) async {
                 final cert = challenge.protectionSpace.sslCertificate;
                 final certBytes = cert?.x509Certificate?.encoded;
@@ -292,8 +280,12 @@ class PVEWebConsoleState extends State<PVEWebConsole> {
         });
   }
 
-  Future<bool> showTLSWarning(BuildContext context, String sslError,
-      String issuedTo, String hash) async {
+  Future<bool> showTLSWarning(
+    BuildContext context,
+    String sslError,
+    String issuedTo,
+    String hash,
+  ) async {
     return await showDialog(
         context: context,
         barrierDismissible: false,
-- 
2.50.1



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


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

* Re: [pve-devel] [PATCH pve_flutter_frontend v4] ui: enable noVNC console on iOS
  2025-08-14  9:32 [pve-devel] [PATCH pve_flutter_frontend v4] ui: enable noVNC console on iOS Shan Shaji
@ 2025-08-14 11:39 ` Dominik Csapak
  2025-08-14 12:26   ` Shan Shaji
  0 siblings, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2025-08-14 11:39 UTC (permalink / raw)
  To: Proxmox VE development discussion, Shan Shaji

hi,

thanks for sending the patch again without dart format changes (mostly,
see my comment on the last hunk)

while the changes themselves look ok to me, i would have liked it more
for the different changes to be different patches

e.g. adding the appbar is ok, but does not have anything to do with
adding ios support (that change is only a single line FWICT)

also refactoring is also always good, but having those commits
separate makes it much more easy to see what is actually changed.

aside from that, there are a few comments inline

On 8/14/25 11:32, Shan Shaji wrote:
> The noVNC console was disabled in iOS and was only available in android
> 
> To fix the issue enabled the noVNC console view on iOS devices. The
> changes also includes a refactor of the function responsible for
> displaying the webview.
> 
> Additionally, the `PveAppBar` has been added to the console view
> to allow users to easily close the view. To avoid the body of the
> scaffold to resize when the keyboard pops up set
> `resizeToAvoidBottomInset` [0] to false.
> 
> - [0] https://api.flutter.dev/flutter/material/Scaffold/resizeToAvoidBottomInset.html
> 
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
> 
>   changes since v3:
>   - Remove dart formatting changes.
>   - Updated commit message.
> 
>   changes since v2:
>   - Set `resizeToAvoidBottomInset` to false to avoid the scaffold body
>     to resize automatically when the keyboard pops up.
>   - Update commit message.
> 
>   changes since v1:
>   - Rebased with master.
>   - Updated commit message.
> 
>   lib/widgets/pve_console_menu_widget.dart | 68 +++++++++++-------------
>   1 file changed, 30 insertions(+), 38 deletions(-)
> 
> diff --git a/lib/widgets/pve_console_menu_widget.dart b/lib/widgets/pve_console_menu_widget.dart
> index 473595d..ab98ce4 100644
> --- a/lib/widgets/pve_console_menu_widget.dart
> +++ b/lib/widgets/pve_console_menu_widget.dart
> @@ -9,6 +9,7 @@ import 'package:proxmox_dart_api_client/proxmox_dart_api_client.dart';
>   import 'package:proxmox_login_manager/proxmox_general_settings_model.dart';
>   import 'package:flutter_inappwebview/flutter_inappwebview.dart';
>   import 'package:crypto/crypto.dart';
> +import 'package:pve_flutter_frontend/widgets/pve_app_bar.dart';
>   
>   class PveConsoleMenu extends StatelessWidget {
>     static const platform =
> @@ -97,45 +98,18 @@ class PveConsoleMenu extends StatelessWidget {
>                     }
>                   },
>                 ),
> -            if (Platform.isAndroid) // web_view is only available for mobile :(
> +            // web_view is only available for mobile :(
> +            // xterm.js doesn't work that well on mobile

i would leave that comment on the novnc console part, since it does not
relate to the line below regarding android/ios

> +            if (Platform.isAndroid || Platform.isIOS)
>                 ListTile(
>                   title: const Text(
> -                  //type == "qemu" ? "noVNC Console" : "xterm.js Console",
> -                  "noVNC Console", // xterm.js doesn't work that well on mobile
> +                  "noVNC Console",
>                     style: TextStyle(fontWeight: FontWeight.bold),
>                   ),
>                   subtitle: const Text("Open console view"),
> -                onTap: () async {
> -                  if (Platform.isAndroid) {
> -                    if (['qemu', 'lxc'].contains(type)) {
> -                      SystemChrome.setEnabledSystemUIMode(
> -                          SystemUiMode.immersive);
> -                      Navigator.of(context)
> -                          .push(_createHTMLConsoleRoute())
> -                          .then((completion) {
> -                        SystemChrome.setEnabledSystemUIMode(
> -                            SystemUiMode.edgeToEdge,
> -                            overlays: [
> -                              SystemUiOverlay.top,
> -                              SystemUiOverlay.bottom
> -                            ]);
> -                      });
> -                    } else if (type == 'node') {
> -                      SystemChrome.setEnabledSystemUIMode(
> -                          SystemUiMode.immersive);
> -                      Navigator.of(context)
> -                          .push(_createHTMLConsoleRoute())
> -                          .then((completion) {
> -                        SystemChrome.setEnabledSystemUIMode(
> -                            SystemUiMode.edgeToEdge,
> -                            overlays: [
> -                              SystemUiOverlay.top,
> -                              SystemUiOverlay.bottom
> -                            ]);
> -                      });
> -                    }
> -                  } else {
> -                    print('not implemented for current platform');
> +                onTap: () {
> +                  if (['qemu', 'lxc'].contains(type) || type == 'node') {

this check now looks weird.

i'd either prefer `['qemu', 'lxc', 'node'].contains(type)` or
something like

type == 'qemu' || type == 'lxc' || type == 'node'

but please don't mix both ;)


> +                    _openNoVncConsole(context);
>                     }
>                   },
>                 ),
> @@ -145,6 +119,18 @@ class PveConsoleMenu extends StatelessWidget {
>       );
>     }
>   
> +  Future<void> _openNoVncConsole(BuildContext context) async {
> +    SystemChrome.setEnabledSystemUIMode(SystemUiMode.immersive);
> +    await Navigator.push(context, _createHTMLConsoleRoute());
> +    SystemChrome.setEnabledSystemUIMode(
> +      SystemUiMode.edgeToEdge,
> +      overlays: [
> +        SystemUiOverlay.top,
> +        SystemUiOverlay.bottom,
> +      ],
> +    );
> +  }
> +
>     void showTextDialog(BuildContext context, String title, String content) {
>       showDialog(
>         context: context,
> @@ -232,8 +218,10 @@ class PVEWebConsoleState extends State<PVEWebConsole> {
>             value: ticket,
>           ),
>           builder: (context, snapshot) {
> -          return SafeArea(
> -            child: InAppWebView(
> +          return Scaffold(
> +            resizeToAvoidBottomInset: false,> +            appBar: PveAppBar(),
> +            body: InAppWebView(
>                 onReceivedServerTrustAuthRequest: (controller, challenge) async {
>                   final cert = challenge.protectionSpace.sslCertificate;
>                   final certBytes = cert?.x509Certificate?.encoded;
> @@ -292,8 +280,12 @@ class PVEWebConsoleState extends State<PVEWebConsole> {
>           });
>     }
>   
> -  Future<bool> showTLSWarning(BuildContext context, String sslError,
> -      String issuedTo, String hash) async {
> +  Future<bool> showTLSWarning(
> +    BuildContext context,
> +    String sslError,
> +    String issuedTo,
> +    String hash,
> +  ) async {
>       return await showDialog(
>           context: context,
>           barrierDismissible: false,

AFAICS this hunk only changes the indenation/formatting.
i guess your editor did sneak in some auto formatting ;)




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


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

* Re: [pve-devel] [PATCH pve_flutter_frontend v4] ui: enable noVNC console on iOS
  2025-08-14 11:39 ` Dominik Csapak
@ 2025-08-14 12:26   ` Shan Shaji
  2025-08-18 10:04     ` Shan Shaji
  0 siblings, 1 reply; 4+ messages in thread
From: Shan Shaji @ 2025-08-14 12:26 UTC (permalink / raw)
  To: Dominik Csapak, Proxmox VE development discussion

Thank you so much for the review and feedback. Then would it be okay if
i send a patch series that seperates the changes into multiple patches. 

On Thu Aug 14, 2025 at 1:39 PM CEST, Dominik Csapak wrote:
> hi,
>
> thanks for sending the patch again without dart format changes (mostly,
> see my comment on the last hunk)
>
> while the changes themselves look ok to me, i would have liked it more
> for the different changes to be different patches
>
> e.g. adding the appbar is ok, but does not have anything to do with
> adding ios support (that change is only a single line FWICT)
>
> also refactoring is also always good, but having those commits
> separate makes it much more easy to see what is actually changed.
>
> aside from that, there are a few comments inline



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


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

* Re: [pve-devel] [PATCH pve_flutter_frontend v4] ui: enable noVNC console on iOS
  2025-08-14 12:26   ` Shan Shaji
@ 2025-08-18 10:04     ` Shan Shaji
  0 siblings, 0 replies; 4+ messages in thread
From: Shan Shaji @ 2025-08-18 10:04 UTC (permalink / raw)
  To: Shan Shaji, Dominik Csapak, Proxmox VE development discussion

Superseded by v5: https://lore.proxmox.com/pve-devel/20250818100134.30550-1-s.shaji@proxmox.com/T/#t

On Thu Aug 14, 2025 at 2:26 PM CEST, Shan Shaji wrote:
> > hi,
> >
> > thanks for sending the patch again without dart format changes (mostly,
> > see my comment on the last hunk)
> >
> > while the changes themselves look ok to me, i would have liked it more
> > for the different changes to be different patches
> >
> > e.g. adding the appbar is ok, but does not have anything to do with
> > adding ios support (that change is only a single line FWICT)
> >
> > also refactoring is also always good, but having those commits
> > separate makes it much more easy to see what is actually changed.
> >
> > aside from that, there are a few comments inline



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


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

end of thread, other threads:[~2025-08-18 10:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-14  9:32 [pve-devel] [PATCH pve_flutter_frontend v4] ui: enable noVNC console on iOS Shan Shaji
2025-08-14 11:39 ` Dominik Csapak
2025-08-14 12:26   ` Shan Shaji
2025-08-18 10:04     ` 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