[pve-devel] [PATCH pve-flutter-frontend 0/9] small improvements

[pve-devel] [PATCH pve-flutter-frontend 0/9] small improvements

[Dominik Csapak]

a series of smaller improvements, noticed by rechecking the app
after updating to new dart/flutter version.

not all of them have to be applied, most are rather independent.
e.g. use of the NavigationBar, and the icon of the warning task logs are
just my preferences.

Dominik Csapak (9):
  replace BottomNavigationBar with NavigationBar
  resource tab: improve colors for headers
  node overview: don't throw permission errors on every update
  adapt to material 3 changes for themes
  fix clipping for data card widget
  set the color of the template indicator to that of the icon
  task logs: don't always say 'Last Task: '
  task logs: use separate color for warnings
  task log list: make warning icon more distinct

 lib/bloc/pve_node_overview_bloc.dart          | 11 +++-
 lib/main.dart                                 |  8 ++-
 lib/pages/main_layout_slim.dart               | 62 ++++++++++---------
 lib/widgets/pve_guest_icon_widget.dart        |  2 +-
 .../pve_resource_data_card_widget.dart        |  2 +
 .../pve_task_log_expansiontile_widget.dart    | 23 +++++--
 lib/widgets/pve_task_log_widget.dart          |  5 +-
 7 files changed, 74 insertions(+), 39 deletions(-)

-- 
2.39.2

[pve-devel] [PATCH pve-flutter-frontend 1/9] replace BottomNavigationBar with NavigationBar

[Dominik Csapak]

it has proper material 3 styling. For the animations to work correctly,
we have to put the bar outside of the part that we replace when we
navigate though

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 lib/pages/main_layout_slim.dart | 57 +++++++++++++++++----------------
 1 file changed, 30 insertions(+), 27 deletions(-)

diff --git a/lib/pages/main_layout_slim.dart b/lib/pages/main_layout_slim.dart
index 3da9beb..2a5e646 100644
--- a/lib/pages/main_layout_slim.dart
+++ b/lib/pages/main_layout_slim.dart
@@ -80,21 +80,12 @@ class _MainLayoutSlimState extends State<MainLayoutSlim> {
           stream: pageSelector.stream,
           initialData: pageSelector.value,
           builder: (context, snapshot) {
-            if (snapshot.hasData) {
-              switch (snapshot.data) {
-                case 0:
-                  return const MobileDashboard();
-                case 1:
-                  return const MobileResourceOverview();
-                case 2:
-                  Provider.of<PveAccessManagementBloc>(context)
-                      .events
-                      .add(LoadUsers());
-                  return const MobileAccessManagement();
-                default:
-              }
-            }
-            return Container();
+            return Scaffold(
+              body: getMainContent(snapshot, context),
+              // it cannot be const, but depends only on context
+              // ignore: prefer_const_constructors
+              bottomNavigationBar: PveMobileBottomNavigationbar(),
+            );
           },
         ),
       ),
@@ -108,6 +99,22 @@ class _MainLayoutSlimState extends State<MainLayoutSlim> {
   }
 }
 
+Widget getMainContent(snapshot, context) {
+  if (snapshot.hasData) {
+    switch (snapshot.data) {
+      case 0:
+        return const MobileDashboard();
+      case 1:
+        return const MobileResourceOverview();
+      case 2:
+        Provider.of<PveAccessManagementBloc>(context).events.add(LoadUsers());
+        return const MobileAccessManagement();
+      default:
+    }
+  }
+  return Container();
+}
+
 class PveMobileBottomNavigationbar extends StatelessWidget {
   const PveMobileBottomNavigationbar({super.key});
 
@@ -115,29 +122,28 @@ class PveMobileBottomNavigationbar extends StatelessWidget {
   Widget build(BuildContext context) {
     final pageSelector = Provider.of<BehaviorSubject<int>>(context);
     final light = Theme.of(context).colorScheme.brightness == Brightness.light;
-    return BottomNavigationBar(
-        type: BottomNavigationBarType.fixed,
+    return NavigationBar(
         backgroundColor: light ? Colors.white : ProxmoxColors.greyShade40,
-        items: const [
-          BottomNavigationBarItem(
+        destinations: const <Widget>[
+          NavigationDestination(
             icon: Icon(Icons.dashboard),
             label: "Dashboard",
           ),
-          BottomNavigationBarItem(
+          NavigationDestination(
             icon: Icon(Icons.developer_board),
             label: "Resources",
           ),
-          BottomNavigationBarItem(
+          NavigationDestination(
             icon: Icon(Icons.supervised_user_circle),
             label: "Access",
           ),
-          BottomNavigationBarItem(
+          NavigationDestination(
             icon: Icon(Icons.logout),
             label: "Sites",
           ),
         ],
-        currentIndex: pageSelector.value,
-        onTap: (index) {
+        selectedIndex: pageSelector.value,
+        onDestinationSelected: (int index) {
           if (index == 3) {
             Provider.of<PveAuthenticationBloc>(context, listen: false)
                 .events
@@ -543,7 +549,6 @@ class MobileDashboard extends StatelessWidget {
               ]);
             }),
       ]),
-      bottomNavigationBar: const PveMobileBottomNavigationbar(),
     );
   }
 }
@@ -656,7 +661,6 @@ class MobileResourceOverview extends StatelessWidget {
                 }
               },
             ),
-            bottomNavigationBar: const PveMobileBottomNavigationbar(),
           )),
         );
       },
@@ -1237,7 +1241,6 @@ class MobileAccessManagement extends StatelessWidget {
                     }),
               ]);
             }),
-        bottomNavigationBar: const PveMobileBottomNavigationbar(),
       ),
     );
   }
-- 
2.39.2

[pve-devel] [PATCH pve-flutter-frontend 2/9] resource tab: improve colors for headers

[Dominik Csapak]

since material 3, the colors are different, and there is no 'muted'
color for the unselected labels anymore. instead use secondary for the
selected, and onPrimary for the unselected ones.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 lib/pages/main_layout_slim.dart | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/pages/main_layout_slim.dart b/lib/pages/main_layout_slim.dart
index 2a5e646..ec80dba 100644
--- a/lib/pages/main_layout_slim.dart
+++ b/lib/pages/main_layout_slim.dart
@@ -1056,8 +1056,11 @@ class MobileAccessManagement extends StatelessWidget {
           automaticallyImplyLeading: false,
           bottom: TabBar(
               isScrollable: true,
-              labelStyle:
+              unselectedLabelStyle:
                   TextStyle(color: Theme.of(context).colorScheme.onPrimary),
+              labelStyle:
+                  TextStyle(color: Theme.of(context).colorScheme.secondary),
+              indicatorColor: Theme.of(context).colorScheme.secondary,
               tabs: const [
                 Tab(
                   text: 'Users',
-- 
2.39.2

[pve-devel] [PATCH pve-flutter-frontend 3/9] node overview: don't throw permission errors on every update

[Dominik Csapak]

getting the apt update status requires Sys.Modify, but the user does not
necessarily has that. So instead of showing a pop up every 10 seconds,
simply ignore permission errors and only show other exceptions here.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 lib/bloc/pve_node_overview_bloc.dart | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/bloc/pve_node_overview_bloc.dart b/lib/bloc/pve_node_overview_bloc.dart
index 7a093b0..caeb1c4 100644
--- a/lib/bloc/pve_node_overview_bloc.dart
+++ b/lib/bloc/pve_node_overview_bloc.dart
@@ -40,8 +40,15 @@ class PveNodeOverviewBloc
       yield latestState.rebuild((b) => b..rrdData.replace(rrdData));
       final services = await apiClient.getNodeServices(nodeID);
       yield latestState.rebuild((b) => b..services.replace(services));
-      final updates = await apiClient.getNodeAptUpdate(nodeID);
-      yield latestState.rebuild((b) => b..updates.replace(updates));
+      try {
+        final updates = await apiClient.getNodeAptUpdate(nodeID);
+        yield latestState.rebuild((b) => b..updates.replace(updates));
+      } on ProxmoxApiException catch (e) {
+        // only throw on non permission related errors
+        if (e.statusCode != 403) {
+          rethrow;
+        }
+      }
       final disks = await apiClient.getNodeDisksList(nodeID);
       yield latestState.rebuild((b) => b..disks.replace(disks));
     }
-- 
2.39.2

[pve-devel] [PATCH pve-flutter-frontend 4/9] adapt to material 3 changes for themes

[Dominik Csapak]

it's recommended to use fromSeed with what was the primary color
beforehand. With this, all colors are correctly deduced from it (e.g.
outline colors etc.).

But since we then set primaryContainer, we also have to set
onPrimaryContainer, otherwise it's deduced from the original primaryContainer
color that would have been generated.

this fixes the issue of the overly stark contrast of the divider color (which
uses the outline color).

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 lib/main.dart | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/main.dart b/lib/main.dart
index 5d49af7..4a56435 100644
--- a/lib/main.dart
+++ b/lib/main.dart
@@ -97,11 +97,13 @@ class MyApp extends StatelessWidget {
         title: 'Proxmox',
         //themeMode: ThemeMode.dark, // comment in/out to test
         theme: ThemeData(
-          colorScheme: const ColorScheme.light(
+          colorScheme: ColorScheme.fromSeed(
+            seedColor: ProxmoxColors.supportBlue,
             brightness: Brightness.light,
             primary: ProxmoxColors.supportBlue,
             onPrimary: Colors.white,
             primaryContainer: ProxmoxColors.blue900,
+            onPrimaryContainer: ProxmoxColors.blue50,
             secondary: ProxmoxColors.orange,
             secondaryContainer: ProxmoxColors.supportLightOrange,
             surface: ProxmoxColors.supportGreyTint50,
@@ -134,11 +136,13 @@ class MyApp extends StatelessWidget {
           ),
         ),
         darkTheme: ThemeData(
-          colorScheme: const ColorScheme.dark(
+          colorScheme: ColorScheme.fromSeed(
+            seedColor: ProxmoxColors.supportBlue,
             brightness: Brightness.dark,
             primary: ProxmoxColors.supportBlue,
             onPrimary: Colors.white,
             primaryContainer: ProxmoxColors.blue800,
+            onPrimaryContainer: ProxmoxColors.blue50,
             surface: ProxmoxColors.greyTint20,
             onSurface: Colors.white,
             secondary: ProxmoxColors.orange,
-- 
2.39.2

[pve-devel] [PATCH pve-flutter-frontend 5/9] fix clipping for data card widget

[Dominik Csapak]

since cards now have a very rounded corner, one can see the non clipped
shape of the contained ExpansionTile.

To fix that, make the shape a RoundedRectangleBorder (which also removes
the added top/bottom border on expansion) and set the clipBehavior of
the surrounding card to antiAlias.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 lib/widgets/pve_resource_data_card_widget.dart | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/widgets/pve_resource_data_card_widget.dart b/lib/widgets/pve_resource_data_card_widget.dart
index 38001a0..28e4460 100644
--- a/lib/widgets/pve_resource_data_card_widget.dart
+++ b/lib/widgets/pve_resource_data_card_widget.dart
@@ -33,10 +33,12 @@ class PveResourceDataCardWidget extends StatelessWidget {
         ),
         child: Card(
           color: Theme.of(context).colorScheme.surface,
+          clipBehavior: Clip.antiAlias,
           child: Builder(
             builder: (context) {
               if (expandable) {
                 return ExpansionTile(
+                  shape: const RoundedRectangleBorder(),
                   textColor: theme.colorScheme.onSurface,
                   title: Row(
                     mainAxisAlignment: MainAxisAlignment.spaceBetween,
-- 
2.39.2

[pve-devel] [PATCH pve-flutter-frontend 6/9] set the color of the template indicator to that of the icon

[Dominik Csapak]

otherwise we have a grey icon but black template indicator, which looks
a bit out of place.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 lib/widgets/pve_guest_icon_widget.dart | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/widgets/pve_guest_icon_widget.dart b/lib/widgets/pve_guest_icon_widget.dart
index a4b241b..4dd7a58 100644
--- a/lib/widgets/pve_guest_icon_widget.dart
+++ b/lib/widgets/pve_guest_icon_widget.dart
@@ -15,7 +15,7 @@ class PveGuestIcon extends StatelessWidget {
     this.template = false,
     required this.status,
     this.color = Colors.grey,
-    this.templateColor = Colors.black,
+    this.templateColor = Colors.grey,
   });
 
   @override
-- 
2.39.2

[pve-devel] [PATCH pve-flutter-frontend 7/9] task logs: don't always say 'Last Task: '

[Dominik Csapak]

only put that when we show the last task, which is only when we show the
'more tasks' button. Make the title otherwise just 'Task: '

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 lib/widgets/pve_task_log_expansiontile_widget.dart | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/widgets/pve_task_log_expansiontile_widget.dart b/lib/widgets/pve_task_log_expansiontile_widget.dart
index a3c36fd..2073840 100644
--- a/lib/widgets/pve_task_log_expansiontile_widget.dart
+++ b/lib/widgets/pve_task_log_expansiontile_widget.dart
@@ -52,6 +52,8 @@ class _PveTaskExpansionTileState extends State<PveTaskExpansionTile> {
         ? (widget.headerColorExpanded ?? colorScheme.onSurface)
         : (widget.headerColor ?? colorScheme.onBackground);
 
+    final taskText = widget.showMorePage != null ? 'Last Task' : 'Task';
+
     return ExpansionTile(
       onExpansionChanged: (value) {
         setState(() {
@@ -66,7 +68,7 @@ class _PveTaskExpansionTileState extends State<PveTaskExpansionTile> {
         color: hasError ? errorColor : headerColor,
       ),
       title: Text(
-        'Last Task: ${widget.task.type}',
+        '$taskText: ${widget.task.type}',
         style: TextStyle(color: headerColor),
       ),
       subtitle: isFinished
-- 
2.39.2

[pve-devel] [PATCH pve-flutter-frontend 8/9] task logs: use separate color for warnings

[Dominik Csapak]

orange instead of red, so one can differentiate between error and
warnings better.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 .../pve_task_log_expansiontile_widget.dart        | 15 +++++++++++----
 lib/widgets/pve_task_log_widget.dart              |  5 ++++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/lib/widgets/pve_task_log_expansiontile_widget.dart b/lib/widgets/pve_task_log_expansiontile_widget.dart
index 2073840..2ff5eb2 100644
--- a/lib/widgets/pve_task_log_expansiontile_widget.dart
+++ b/lib/widgets/pve_task_log_expansiontile_widget.dart
@@ -36,8 +36,10 @@ class _PveTaskExpansionTileState extends State<PveTaskExpansionTile> {
 
   @override
   Widget build(BuildContext context) {
-    final hasError =
-        widget.task.status != 'RUNNING' && widget.task.status != "OK";
+    final hasWarnings = widget.task.status.startsWith('WARNINGS:');
+    final hasError = widget.task.status != 'RUNNING' &&
+        widget.task.status != "OK" &&
+        !hasWarnings;
     final isFinished = widget.task.endTime != null;
     final taskLogBloc = Provider.of<PveTaskLogBloc>(context);
     Duration duration;
@@ -48,6 +50,7 @@ class _PveTaskExpansionTileState extends State<PveTaskExpansionTile> {
     }
     final colorScheme = Theme.of(context).colorScheme;
     final errorColor = widget.errorColor ?? colorScheme.error;
+    final warningColor = Colors.orange.shade200;
     final headerColor = isExpanded
         ? (widget.headerColorExpanded ?? colorScheme.onSurface)
         : (widget.headerColor ?? colorScheme.onBackground);
@@ -64,8 +67,12 @@ class _PveTaskExpansionTileState extends State<PveTaskExpansionTile> {
       collapsedBackgroundColor: Theme.of(context).colorScheme.background,
       key: PageStorageKey<PveClusterTasksModel>(widget.task),
       leading: Icon(
-        hasError ? Icons.warning : Icons.info,
-        color: hasError ? errorColor : headerColor,
+        hasError || hasWarnings ? Icons.warning : Icons.info,
+        color: hasError
+            ? errorColor
+            : hasWarnings
+                ? warningColor
+                : headerColor,
       ),
       title: Text(
         '$taskText: ${widget.task.type}',
diff --git a/lib/widgets/pve_task_log_widget.dart b/lib/widgets/pve_task_log_widget.dart
index 97cbf03..5a12676 100644
--- a/lib/widgets/pve_task_log_widget.dart
+++ b/lib/widgets/pve_task_log_widget.dart
@@ -198,12 +198,15 @@ class _PveTaskLogScrollViewState extends State<PveTaskLogScrollView> {
       child: ProxmoxStreamBuilder<PveTaskLogViewerBloc, PveTaskLogViewerState>(
           bloc: Provider.of<PveTaskLogViewerBloc>(context),
           builder: (context, state) {
-            // TODO: fix color (for dark theme) and better handle warnings
             var indicatorColor = Colors.teal.shade500;
             var statusChipColor = Colors.teal.shade100;
             if (state.status?.failed ?? false) {
               indicatorColor = Colors.red;
               statusChipColor = Colors.red.shade100;
+            } else if ((state.status?.exitStatus ?? '')
+                .startsWith('WARNINGS:')) {
+              indicatorColor = Colors.orange;
+              statusChipColor = Colors.orange.shade100;
             }
             return SizedBox(
               height: MediaQuery.of(context).size.height * 0.5,
-- 
2.39.2

[pve-devel] [PATCH pve-flutter-frontend 9/9] task log list: make warning icon more distinct

[Dominik Csapak]

by making it rounded (which appears as 'less dangerous').
Sadly the 'error' icon is not really usable for us, since it looks
almost the same as the 'info' icon (just 180 degree rotated).

We could use an 'outline' variant for either, but making the 'less
dangerous' one rounded feels alright.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 lib/widgets/pve_task_log_expansiontile_widget.dart | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/widgets/pve_task_log_expansiontile_widget.dart b/lib/widgets/pve_task_log_expansiontile_widget.dart
index 2ff5eb2..6491b6b 100644
--- a/lib/widgets/pve_task_log_expansiontile_widget.dart
+++ b/lib/widgets/pve_task_log_expansiontile_widget.dart
@@ -67,7 +67,11 @@ class _PveTaskExpansionTileState extends State<PveTaskExpansionTile> {
       collapsedBackgroundColor: Theme.of(context).colorScheme.background,
       key: PageStorageKey<PveClusterTasksModel>(widget.task),
       leading: Icon(
-        hasError || hasWarnings ? Icons.warning : Icons.info,
+        hasError
+            ? Icons.warning
+            : hasWarnings
+                ? Icons.warning_rounded
+                : Icons.info,
         color: hasError
             ? errorColor
             : hasWarnings
-- 
2.39.2

[pve-devel] applied: [PATCH pve-flutter-frontend 0/9] small improvements

[Thomas Lamprecht]

Am 12/04/2024 um 10:04 schrieb Dominik Csapak:
> a series of smaller improvements, noticed by rechecking the app
> after updating to new dart/flutter version.
> 
> not all of them have to be applied, most are rather independent.
> e.g. use of the NavigationBar, and the icon of the warning task logs are
> just my preferences.
> 
> Dominik Csapak (9):
>   replace BottomNavigationBar with NavigationBar
>   resource tab: improve colors for headers
>   node overview: don't throw permission errors on every update
>   adapt to material 3 changes for themes
>   fix clipping for data card widget
>   set the color of the template indicator to that of the icon
>   task logs: don't always say 'Last Task: '
>   task logs: use separate color for warnings
>   task log list: make warning icon more distinct
> 
>  lib/bloc/pve_node_overview_bloc.dart          | 11 +++-
>  lib/main.dart                                 |  8 ++-
>  lib/pages/main_layout_slim.dart               | 62 ++++++++++---------
>  lib/widgets/pve_guest_icon_widget.dart        |  2 +-
>  .../pve_resource_data_card_widget.dart        |  2 +
>  .../pve_task_log_expansiontile_widget.dart    | 23 +++++--
>  lib/widgets/pve_task_log_widget.dart          |  5 +-
>  7 files changed, 74 insertions(+), 39 deletions(-)
> 


applied series, thanks!

Made one follow-up to avoid suggesting that there are no updates
if the user did not have the permission to query that.