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 064B61FF165 for ; Thu, 14 Aug 2025 13:38:08 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EB89710CE6; Thu, 14 Aug 2025 13:39:46 +0200 (CEST) Message-ID: Date: Thu, 14 Aug 2025 13:39:12 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Proxmox VE development discussion , Shan Shaji References: <20250814093219.75803-1-s.shaji@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20250814093219.75803-1-s.shaji@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1755171519699 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.021 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 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: [pve-devel] [PATCH pve_flutter_frontend v4] ui: enable noVNC console on iOS X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 > --- > > 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 _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 { > 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 { > }); > } > > - Future showTLSWarning(BuildContext context, String sslError, > - String issuedTo, String hash) async { > + Future 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