* [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen @ 2025-07-03 11:19 Shan Shaji 2025-07-03 14:00 ` Dominik Csapak 0 siblings, 1 reply; 9+ messages in thread From: Shan Shaji @ 2025-07-03 11:19 UTC (permalink / raw) To: pve-devel According to Apple's App Store review guidelines all apps must include a link to their privacy policy within the App [0]. To fix the issue add a new list item in the settings screen that will allow users to access the privacy policy. [0] - https://developer.apple.com/app-store/review/guidelines/#legal Signed-off-by: Shan Shaji <s.shaji@proxmox.com> --- lib/constants/pve_links.dart | 6 + lib/proxmox_general_settings_form.dart | 32 ++++++ lib/utils/pve_url_launcher_util.dart | 24 ++++ lib/utils/result.dart | 34 ++++++ lib/widgets/pve_settings_item.dart | 61 ++++++++++ lib/widgets/pve_settings_section.dart | 43 +++++++ pubspec.lock | 150 ++++++++++++++++++------- pubspec.yaml | 1 + 8 files changed, 308 insertions(+), 43 deletions(-) create mode 100644 lib/constants/pve_links.dart create mode 100644 lib/utils/pve_url_launcher_util.dart create mode 100644 lib/utils/result.dart create mode 100644 lib/widgets/pve_settings_item.dart create mode 100644 lib/widgets/pve_settings_section.dart diff --git a/lib/constants/pve_links.dart b/lib/constants/pve_links.dart new file mode 100644 index 0000000..196b58c --- /dev/null +++ b/lib/constants/pve_links.dart @@ -0,0 +1,6 @@ +class PveLinks { + PveLinks._(); + + static const String privacyPolicyUrl = + 'https://pve.proxmox.com/wiki/Proxmox_VE_Mobile_Companion_Data_Protection'; +} diff --git a/lib/proxmox_general_settings_form.dart b/lib/proxmox_general_settings_form.dart index cb0afef..b81af3a 100644 --- a/lib/proxmox_general_settings_form.dart +++ b/lib/proxmox_general_settings_form.dart @@ -1,5 +1,10 @@ import 'package:flutter/material.dart'; +import 'package:proxmox_login_manager/constants/pve_links.dart'; import 'package:proxmox_login_manager/proxmox_general_settings_model.dart'; +import 'package:proxmox_login_manager/utils/pve_url_launcher_util.dart'; +import 'package:proxmox_login_manager/utils/result.dart'; +import 'package:proxmox_login_manager/widgets/pve_settings_item.dart'; +import 'package:proxmox_login_manager/widgets/pve_settings_section.dart'; class ProxmoxGeneralSettingsForm extends StatefulWidget { const ProxmoxGeneralSettingsForm({super.key}); @@ -21,6 +26,7 @@ class _ProxmoxGeneralSettingsFormState @override Widget build(BuildContext context) { return Scaffold( + backgroundColor: Theme.of(context).colorScheme.surfaceContainer, appBar: AppBar( title: const Text('Settings'), ), @@ -45,6 +51,32 @@ class _ProxmoxGeneralSettingsFormState ProxmoxGeneralSettingsModel.fromLocalStorage(); }); }, + ), + PveSettingsSection( + title: 'LEGAL', + children: [ + PveSettingsItem( + titleString: 'Privacy Policy', + leadingIcon: Icons.privacy_tip_outlined, + trailingIcon: Icons.open_in_new, + onTap: () async { + final result = await PveUrlLauncherUtil.openUrl( + Uri.parse(PveLinks.privacyPolicyUrl), + ); + + if (result is Error) { + if (!context.mounted) return; + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + result.error.toString(), + ), + ), + ); + } + }, + ), + ], ) ], ), diff --git a/lib/utils/pve_url_launcher_util.dart b/lib/utils/pve_url_launcher_util.dart new file mode 100644 index 0000000..8820d75 --- /dev/null +++ b/lib/utils/pve_url_launcher_util.dart @@ -0,0 +1,24 @@ +import 'package:proxmox_login_manager/utils/result.dart'; +import 'package:url_launcher/url_launcher.dart'; + +class PveUrlLauncherUtil { + PveUrlLauncherUtil._(); + + static Future<Result> openUrl( + Uri uri, { + LaunchMode mode = LaunchMode.inAppBrowserView, + }) async { + const errorMessage = 'Could not launch url'; + try { + if (await canLaunchUrl(uri)) { + final result = await launchUrl(uri, mode: mode); + if (result) return Result.ok(true); + return Result.error(Exception(errorMessage)); + } else { + return Result.error(Exception(errorMessage)); + } + } on Exception catch (e, _) { + return Result.error(e); + } + } +} diff --git a/lib/utils/result.dart b/lib/utils/result.dart new file mode 100644 index 0000000..5454e9e --- /dev/null +++ b/lib/utils/result.dart @@ -0,0 +1,34 @@ +/// Utility class that simplifies handling errors. +/// +/// Return a [Result] from a function to indicate success or failure. +/// +/// A [Result] is either an [Ok] with a value of type [T] +/// or an [Error] with an [Exception]. +/// +/// Use [Result.ok] to create a successful result with a value of type [T]. +/// Use [Result.error] to create an error result with an [Exception]. +sealed class Result<T> { + const Result(); + + /// Creates an instance of Result containing a value + factory Result.ok(T value) => Ok(value); + + /// Create an instance of Result containing an error + factory Result.error(Exception error) => Error(error); +} + +/// Subclass of Result for values +final class Ok<T> extends Result<T> { + const Ok(this.value); + + /// Returned value in result + final T value; +} + +/// Subclass of Result for errors +final class Error<T> extends Result<T> { + const Error(this.error); + + /// Returned error in result + final Exception error; +} diff --git a/lib/widgets/pve_settings_item.dart b/lib/widgets/pve_settings_item.dart new file mode 100644 index 0000000..a1648aa --- /dev/null +++ b/lib/widgets/pve_settings_item.dart @@ -0,0 +1,61 @@ +import 'package:flutter/material.dart'; + +class PveSettingsItem extends StatelessWidget { + const PveSettingsItem({ + super.key, + this.titleString, + this.leadingIcon, + this.trailingIcon, + this.leading, + this.trailing, + this.title, + this.onTap, + }) : assert( + leading != null || leadingIcon != null, + "Either leading or leadingIcon must be provided", + ), + assert( + trailing != null || trailingIcon != null, + 'Either trailing or trailingIcon must be provided', + ), + assert( + titleString != null || title != null, + 'Either titleString or title must be provided', + ); + + /// The text to display as the title. + final String? titleString; + + /// An icon to display before the title. + /// + /// If [leading] widget is not null, [leadingIcon] is ignored. + final IconData? leadingIcon; + + /// An icon to display after the title. + /// + /// If [trailing] property is not null, [trailingIcon] is ignored. + final IconData? trailingIcon; + + /// A widget to display before the title. + final Widget? leading; + + /// A widget to display after the title. + final Widget? trailing; + + /// A widget to display as the title. + final Widget? title; + + /// A callback that is called when the user taps on the ListTile. + final VoidCallback? onTap; + + @override + Widget build(BuildContext context) { + return ListTile( + shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(10)), + leading: leading ?? Icon(leadingIcon), + title: title ?? (titleString != null ? Text(titleString!) : null), + trailing: trailing ?? Icon(trailingIcon), + onTap: onTap, + ); + } +} diff --git a/lib/widgets/pve_settings_section.dart b/lib/widgets/pve_settings_section.dart new file mode 100644 index 0000000..d22c630 --- /dev/null +++ b/lib/widgets/pve_settings_section.dart @@ -0,0 +1,43 @@ +import 'package:flutter/material.dart'; +import 'package:proxmox_login_manager/widgets/pve_settings_item.dart'; + +class PveSettingsSection extends StatelessWidget { + const PveSettingsSection({ + super.key, + required this.title, + required this.children, + }); + + /// The title of the section. + final String title; + + /// The list of items in the section. + final List<PveSettingsItem> children; + + @override + Widget build(BuildContext context) { + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + ListTile( + title: Text( + title, + style: const TextStyle( + fontWeight: FontWeight.bold, + ), + ), + ), + Container( + decoration: BoxDecoration( + color: Theme.of(context).colorScheme.surface, + borderRadius: BorderRadius.circular(10), + ), + margin: const EdgeInsets.symmetric(horizontal: 16), + child: Column( + children: children, + ), + ) + ], + ); + } +} diff --git a/pubspec.lock b/pubspec.lock index 06b82b6..6163a50 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -29,10 +29,10 @@ packages: dependency: transitive description: name: async - sha256: "947bfcf187f74dbc5e146c9eb9c0f10c9f8b30743e341481c1e2ed3ecc18c20c" + sha256: d2872f9c19731c2e5f10444b14686eb7cc85c76274bd6c16e1816bff9a3bab63 url: "https://pub.dev" source: hosted - version: "2.11.0" + version: "2.12.0" biometric_storage: dependency: "direct main" description: @@ -45,10 +45,10 @@ packages: dependency: transitive description: name: boolean_selector - sha256: "6cfb5af12253eaf2b368f07bacc5a80d1301a071c73360d746b7f2e32d762c66" + sha256: "8aab1771e1243a5063b8b0ff68042d67334e3feab9e95b9490f9a6ebf73b42ea" url: "https://pub.dev" source: hosted - version: "2.1.1" + version: "2.1.2" build: dependency: transitive description: @@ -125,10 +125,10 @@ packages: dependency: transitive description: name: characters - sha256: "04a925763edad70e8443c99234dc3328f442e811f1d8fd1a72f1c8ad0f69a605" + sha256: f71061c654a3380576a52b451dd5532377954cf9dbd272a78fc8479606670803 url: "https://pub.dev" source: hosted - version: "1.3.0" + version: "1.4.0" checked_yaml: dependency: transitive description: @@ -141,10 +141,10 @@ packages: dependency: transitive description: name: clock - sha256: cb6d7f03e1de671e34607e909a7213e31d7752be4fb66a86d29fe1eb14bfb5cf + sha256: fddb70d9b5277016c77a80201021d40a2247104d9f4aa7bab7157b7e3f05b84b url: "https://pub.dev" source: hosted - version: "1.1.1" + version: "1.1.2" code_builder: dependency: transitive description: @@ -157,10 +157,10 @@ packages: dependency: "direct main" description: name: collection - sha256: ee67cb0715911d28db6bf4af1026078bd6f0128b07a5f66fb2ed94ec6783c09a + sha256: "2f5709ae4d3d59dd8f7cd309b4e023046b57d8a6c82130785d2b0e5868084e76" url: "https://pub.dev" source: hosted - version: "1.18.0" + version: "1.19.1" convert: dependency: transitive description: @@ -189,10 +189,10 @@ packages: dependency: transitive description: name: fake_async - sha256: "511392330127add0b769b75a987850d136345d9227c6b94c96a04cf4a391bf78" + sha256: "6a95e56b2449df2273fd8c45a662d6947ce1ebb7aafe80e550a3f68297f3cacc" url: "https://pub.dev" source: hosted - version: "1.3.1" + version: "1.3.2" ffi: dependency: transitive description: @@ -316,26 +316,26 @@ packages: dependency: transitive description: name: leak_tracker - sha256: "78eb209deea09858f5269f5a5b02be4049535f568c07b275096836f01ea323fa" + sha256: c35baad643ba394b40aac41080300150a4f08fd0fd6a10378f8f7c6bc161acec url: "https://pub.dev" source: hosted - version: "10.0.0" + version: "10.0.8" leak_tracker_flutter_testing: dependency: transitive description: name: leak_tracker_flutter_testing - sha256: b46c5e37c19120a8a01918cfaf293547f47269f7cb4b0058f21531c2465d6ef0 + sha256: f8b613e7e6a13ec79cfdc0e97638fddb3ab848452eff057653abd3edba760573 url: "https://pub.dev" source: hosted - version: "2.0.1" + version: "3.0.9" leak_tracker_testing: dependency: transitive description: name: leak_tracker_testing - sha256: a597f72a664dbd293f3bfc51f9ba69816f84dcd403cdac7066cb3f6003f3ab47 + sha256: "6ba465d5d76e67ddf503e1161d1f4a6bc42306f9d66ca1e8f079a47290fb06d3" url: "https://pub.dev" source: hosted - version: "2.0.1" + version: "3.0.1" lints: dependency: transitive description: @@ -356,26 +356,26 @@ packages: dependency: transitive description: name: matcher - sha256: d2323aa2060500f906aa31a895b4030b6da3ebdcc5619d14ce1aada65cd161cb + sha256: dc58c723c3c24bf8d3e2d3ad3f2f9d7bd9cf43ec6feaa64181775e60190153f2 url: "https://pub.dev" source: hosted - version: "0.12.16+1" + version: "0.12.17" material_color_utilities: dependency: transitive description: name: material_color_utilities - sha256: "0e0a020085b65b6083975e499759762399b4475f766c21668c4ecca34ea74e5a" + sha256: f7142bb1154231d7ea5f96bc7bde4bda2a0945d2806bb11670e30b850d56bdec url: "https://pub.dev" source: hosted - version: "0.8.0" + version: "0.11.1" meta: dependency: transitive description: name: meta - sha256: d584fa6707a52763a52446f02cc621b077888fb63b93bbcb1143a7be5a0c0c04 + sha256: e3641ec5d63ebf0d9b41bd43201a66e3fc79a65db5f61fc181f04cd27aab950c url: "https://pub.dev" source: hosted - version: "1.11.0" + version: "1.16.0" mime: dependency: transitive description: @@ -396,10 +396,10 @@ packages: dependency: transitive description: name: path - sha256: "087ce49c3f0dc39180befefc60fdb4acd8f8620e5682fe2476afd0b3688bb4af" + sha256: "75cca69d1490965be98c73ceaea117e8a04dd21217b37b292c9ddbec0d955bc5" url: "https://pub.dev" source: hosted - version: "1.9.0" + version: "1.9.1" path_provider_linux: dependency: transitive description: @@ -555,7 +555,7 @@ packages: dependency: transitive description: flutter source: sdk - version: "0.0.99" + version: "0.0.0" source_gen: dependency: transitive description: @@ -568,26 +568,26 @@ packages: dependency: transitive description: name: source_span - sha256: "53e943d4206a5e30df338fd4c6e7a077e02254531b138a15aec3bd143c1a8b3c" + sha256: "254ee5351d6cb365c859e20ee823c3bb479bf4a293c22d17a9f1bf144ce86f7c" url: "https://pub.dev" source: hosted - version: "1.10.0" + version: "1.10.1" stack_trace: dependency: transitive description: name: stack_trace - sha256: "73713990125a6d93122541237550ee3352a2d84baad52d375a4cad2eb9b7ce0b" + sha256: "8b27215b45d22309b5cddda1aa2b19bdfec9df0e765f2de506401c071d38d1b1" url: "https://pub.dev" source: hosted - version: "1.11.1" + version: "1.12.1" stream_channel: dependency: transitive description: name: stream_channel - sha256: ba2aa5d8cc609d96bbb2899c28934f9e1af5cddbd60a827822ea467161eb54e7 + sha256: "969e04c80b8bcdf826f8f16579c7b14d780458bd97f56d107d3950fdbeef059d" url: "https://pub.dev" source: hosted - version: "2.1.2" + version: "2.1.4" stream_transform: dependency: transitive description: @@ -600,26 +600,26 @@ packages: dependency: transitive description: name: string_scanner - sha256: "556692adab6cfa87322a115640c11f13cb77b3f076ddcc5d6ae3c20242bedcde" + sha256: "921cd31725b72fe181906c6a94d987c78e3b98c2e205b397ea399d4054872b43" url: "https://pub.dev" source: hosted - version: "1.2.0" + version: "1.4.1" term_glyph: dependency: transitive description: name: term_glyph - sha256: a29248a84fbb7c79282b40b8c72a1209db169a2e0542bce341da992fe1bc7e84 + sha256: "7f554798625ea768a7518313e58f83891c7f5024f88e46e7182a4558850a4b8e" url: "https://pub.dev" source: hosted - version: "1.2.1" + version: "1.2.2" test_api: dependency: transitive description: name: test_api - sha256: "5c2f730018264d276c20e4f1503fd1308dfbbae39ec8ee63c5236311ac06954b" + sha256: fb31f383e2ee25fbbfe06b40fe21e1e458d14080e3c67e7ba0acfde4df4e0bbd url: "https://pub.dev" source: hosted - version: "0.6.1" + version: "0.7.4" timing: dependency: transitive description: @@ -636,6 +636,70 @@ packages: url: "https://pub.dev" source: hosted version: "1.3.2" + url_launcher: + dependency: "direct main" + description: + name: url_launcher + sha256: "9d06212b1362abc2f0f0d78e6f09f726608c74e3b9462e8368bb03314aa8d603" + url: "https://pub.dev" + source: hosted + version: "6.3.1" + url_launcher_android: + dependency: transitive + description: + name: url_launcher_android + sha256: "8582d7f6fe14d2652b4c45c9b6c14c0b678c2af2d083a11b604caeba51930d79" + url: "https://pub.dev" + source: hosted + version: "6.3.16" + url_launcher_ios: + dependency: transitive + description: + name: url_launcher_ios + sha256: "7f2022359d4c099eea7df3fdf739f7d3d3b9faf3166fb1dd390775176e0b76cb" + url: "https://pub.dev" + source: hosted + version: "6.3.3" + url_launcher_linux: + dependency: transitive + description: + name: url_launcher_linux + sha256: "4e9ba368772369e3e08f231d2301b4ef72b9ff87c31192ef471b380ef29a4935" + url: "https://pub.dev" + source: hosted + version: "3.2.1" + url_launcher_macos: + dependency: transitive + description: + name: url_launcher_macos + sha256: "17ba2000b847f334f16626a574c702b196723af2a289e7a93ffcb79acff855c2" + url: "https://pub.dev" + source: hosted + version: "3.2.2" + url_launcher_platform_interface: + dependency: transitive + description: + name: url_launcher_platform_interface + sha256: "552f8a1e663569be95a8190206a38187b531910283c3e982193e4f2733f01029" + url: "https://pub.dev" + source: hosted + version: "2.3.2" + url_launcher_web: + dependency: transitive + description: + name: url_launcher_web + sha256: "4bd2b7b4dc4d4d0b94e5babfffbca8eac1a126c7f3d6ecbc1a11013faa3abba2" + url: "https://pub.dev" + source: hosted + version: "2.4.1" + url_launcher_windows: + dependency: transitive + description: + name: url_launcher_windows + sha256: "3284b6d2ac454cf34f114e1d3319866fdd1e19cdc329999057e44ffe936cfa77" + url: "https://pub.dev" + source: hosted + version: "3.1.4" vector_math: dependency: transitive description: @@ -648,10 +712,10 @@ packages: dependency: transitive description: name: vm_service - sha256: b3d56ff4341b8f182b96aceb2fa20e3dcb336b9f867bc0eafc0de10f1048e957 + sha256: "0968250880a6c5fe7edc067ed0a13d4bae1577fe2771dcf3010d52c4a9d3ca14" url: "https://pub.dev" source: hosted - version: "13.0.0" + version: "14.3.1" watcher: dependency: transitive description: @@ -701,5 +765,5 @@ packages: source: hosted version: "3.1.2" sdks: - dart: ">=3.3.0 <4.0.0" - flutter: ">=3.19.0" + dart: ">=3.7.0-0 <4.0.0" + flutter: ">=3.27.0" diff --git a/pubspec.yaml b/pubspec.yaml index 99f9870..1e48217 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -14,6 +14,7 @@ dependencies: biometric_storage: ^5.0.0 built_value: ^8.4.1 built_collection: ^5.0.0 + url_launcher: ^6.3.1 proxmox_dart_api_client: path: ../proxmox_dart_api_client -- 2.39.5 (Apple Git-154) _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen 2025-07-03 11:19 [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen Shan Shaji @ 2025-07-03 14:00 ` Dominik Csapak 2025-07-04 7:16 ` Shan Shaji 2025-07-04 14:40 ` Shan Shaji 0 siblings, 2 replies; 9+ messages in thread From: Dominik Csapak @ 2025-07-03 14:00 UTC (permalink / raw) To: Proxmox VE development discussion, Shan Shaji hi, high level comment: I don't think it's necessary or wise to refactor upfront. You create new layouting widgets that are only used once(PveSettingsSection, PveSettingsItem) and create classes with functionality that is never uses (the mode in openUrl?) I think it's best to just use the regular flutter widgets inline and keep methods and classes small to do what is actually needed. We can still extend them when more is required, but there is no advantage from having dead code. a few comments inline On 7/3/25 13:19, Shan Shaji wrote: > According to Apple's App Store review guidelines all apps must include a > link to their privacy policy within the App [0]. To fix the issue add a > new list item in the settings screen that will allow users to access the > privacy policy. > > [0] - https://developer.apple.com/app-store/review/guidelines/#legal > > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > lib/constants/pve_links.dart | 6 + > lib/proxmox_general_settings_form.dart | 32 ++++++ > lib/utils/pve_url_launcher_util.dart | 24 ++++ > lib/utils/result.dart | 34 ++++++ > lib/widgets/pve_settings_item.dart | 61 ++++++++++ > lib/widgets/pve_settings_section.dart | 43 +++++++ > pubspec.lock | 150 ++++++++++++++++++------- > pubspec.yaml | 1 + > 8 files changed, 308 insertions(+), 43 deletions(-) > create mode 100644 lib/constants/pve_links.dart > create mode 100644 lib/utils/pve_url_launcher_util.dart > create mode 100644 lib/utils/result.dart > create mode 100644 lib/widgets/pve_settings_item.dart > create mode 100644 lib/widgets/pve_settings_section.dart > > diff --git a/lib/constants/pve_links.dart b/lib/constants/pve_links.dart > new file mode 100644 > index 0000000..196b58c > --- /dev/null > +++ b/lib/constants/pve_links.dart > @@ -0,0 +1,6 @@ > +class PveLinks { > + PveLinks._(); > + > + static const String privacyPolicyUrl = > + 'https://pve.proxmox.com/wiki/Proxmox_VE_Mobile_Companion_Data_Protection'; > +} IMHO it does not really make sense to define a class to hold things just for a single element. I'd put the link where we actually call it, and only refactor it when it's necessary, e.g. when we often use the same link from different places, etc. > diff --git a/lib/proxmox_general_settings_form.dart b/lib/proxmox_general_settings_form.dart > index cb0afef..b81af3a 100644 > --- a/lib/proxmox_general_settings_form.dart > +++ b/lib/proxmox_general_settings_form.dart > @@ -1,5 +1,10 @@ > import 'package:flutter/material.dart'; > +import 'package:proxmox_login_manager/constants/pve_links.dart'; > import 'package:proxmox_login_manager/proxmox_general_settings_model.dart'; > +import 'package:proxmox_login_manager/utils/pve_url_launcher_util.dart'; > +import 'package:proxmox_login_manager/utils/result.dart'; > +import 'package:proxmox_login_manager/widgets/pve_settings_item.dart'; > +import 'package:proxmox_login_manager/widgets/pve_settings_section.dart'; > > class ProxmoxGeneralSettingsForm extends StatefulWidget { > const ProxmoxGeneralSettingsForm({super.key}); > @@ -21,6 +26,7 @@ class _ProxmoxGeneralSettingsFormState > @override > Widget build(BuildContext context) { > return Scaffold( > + backgroundColor: Theme.of(context).colorScheme.surfaceContainer, this change seems to be unrelated? > appBar: AppBar( > title: const Text('Settings'), > ), > @@ -45,6 +51,32 @@ class _ProxmoxGeneralSettingsFormState > ProxmoxGeneralSettingsModel.fromLocalStorage(); > }); > }, > + ), > + PveSettingsSection( > + title: 'LEGAL', > + children: [ > + PveSettingsItem( > + titleString: 'Privacy Policy', > + leadingIcon: Icons.privacy_tip_outlined, > + trailingIcon: Icons.open_in_new, > + onTap: () async { > + final result = await PveUrlLauncherUtil.openUrl( > + Uri.parse(PveLinks.privacyPolicyUrl), > + ); > + > + if (result is Error) { > + if (!context.mounted) return; > + ScaffoldMessenger.of(context).showSnackBar( > + SnackBar( > + content: Text( > + result.error.toString(), > + ), > + ), > + ); > + } > + }, > + ), > + ], here you could simply use the column with 2 listtiles. > ) > ], > ), > diff --git a/lib/utils/pve_url_launcher_util.dart b/lib/utils/pve_url_launcher_util.dart > new file mode 100644 > index 0000000..8820d75 > --- /dev/null > +++ b/lib/utils/pve_url_launcher_util.dart > @@ -0,0 +1,24 @@ > +import 'package:proxmox_login_manager/utils/result.dart'; > +import 'package:url_launcher/url_launcher.dart'; > + > +class PveUrlLauncherUtil { > + PveUrlLauncherUtil._(); > + > + static Future<Result> openUrl( > + Uri uri, { > + LaunchMode mode = LaunchMode.inAppBrowserView, FWICT the mode parameter is not used anywhere, so it's not needed. Actually we already have a wrapper around url_launcher in pve_flutter_frontend We don't have a good place to put things that are shared between the pve_flutter_frontend and login_manager, maybe this is a sign that we should create such a place. Otherwise you could simply copy the tryLaunchUrl method here. > + }) async { > + const errorMessage = 'Could not launch url'; > + try { > + if (await canLaunchUrl(uri)) { > + final result = await launchUrl(uri, mode: mode); > + if (result) return Result.ok(true); > + return Result.error(Exception(errorMessage)); > + } else { > + return Result.error(Exception(errorMessage)); > + } > + } on Exception catch (e, _) { > + return Result.error(e); > + } > + } > +} > diff --git a/lib/utils/result.dart b/lib/utils/result.dart > new file mode 100644 > index 0000000..5454e9e > --- /dev/null > +++ b/lib/utils/result.dart > @@ -0,0 +1,34 @@ > +/// Utility class that simplifies handling errors. > +/// > +/// Return a [Result] from a function to indicate success or failure. > +/// > +/// A [Result] is either an [Ok] with a value of type [T] > +/// or an [Error] with an [Exception]. > +/// > +/// Use [Result.ok] to create a successful result with a value of type [T]. > +/// Use [Result.error] to create an error result with an [Exception]. > +sealed class Result<T> { > + const Result(); > + > + /// Creates an instance of Result containing a value > + factory Result.ok(T value) => Ok(value); > + > + /// Create an instance of Result containing an error > + factory Result.error(Exception error) => Error(error); > +} > + > +/// Subclass of Result for values > +final class Ok<T> extends Result<T> { > + const Ok(this.value); > + > + /// Returned value in result > + final T value; > +} > + > +/// Subclass of Result for errors > +final class Error<T> extends Result<T> { > + const Error(this.error); > + > + /// Returned error in result > + final Exception error; > +} while I appreciate the sentiment of having a result class akin to what we have in rust. creating this class just to use it in one or two places is overkill IMHO If we want to do something like this, it would be better to introduce this in it's own patch and use it everywhere it makes sense (also in pve_flutter_frontent, which indicates we probably should invent a package that can be used by both...) but I think we could just live with usual exception handling too.. > diff --git a/lib/widgets/pve_settings_item.dart b/lib/widgets/pve_settings_item.dart > new file mode 100644 > index 0000000..a1648aa > --- /dev/null > +++ b/lib/widgets/pve_settings_item.dart > @@ -0,0 +1,61 @@ > +import 'package:flutter/material.dart'; > + > +class PveSettingsItem extends StatelessWidget { > + const PveSettingsItem({ > + super.key, > + this.titleString, > + this.leadingIcon, > + this.trailingIcon, > + this.leading, > + this.trailing, > + this.title, > + this.onTap, > + }) : assert( > + leading != null || leadingIcon != null, > + "Either leading or leadingIcon must be provided", > + ), > + assert( > + trailing != null || trailingIcon != null, > + 'Either trailing or trailingIcon must be provided', > + ), > + assert( > + titleString != null || title != null, > + 'Either titleString or title must be provided', > + ); > + > + /// The text to display as the title. > + final String? titleString; > + > + /// An icon to display before the title. > + /// > + /// If [leading] widget is not null, [leadingIcon] is ignored. > + final IconData? leadingIcon; > + > + /// An icon to display after the title. > + /// > + /// If [trailing] property is not null, [trailingIcon] is ignored. > + final IconData? trailingIcon; > + > + /// A widget to display before the title. > + final Widget? leading; > + > + /// A widget to display after the title. > + final Widget? trailing; > + > + /// A widget to display as the title. > + final Widget? title; > + > + /// A callback that is called when the user taps on the ListTile. > + final VoidCallback? onTap; > + > + @override > + Widget build(BuildContext context) { > + return ListTile( > + shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(10)), > + leading: leading ?? Icon(leadingIcon), > + title: title ?? (titleString != null ? Text(titleString!) : null), > + trailing: trailing ?? Icon(trailingIcon), > + onTap: onTap, > + ); > + } > +} This widget is only used once, and mostly just passes through options to a listtile, so i don't think it's worthy of it's own widget... > diff --git a/lib/widgets/pve_settings_section.dart b/lib/widgets/pve_settings_section.dart > new file mode 100644 > index 0000000..d22c630 > --- /dev/null > +++ b/lib/widgets/pve_settings_section.dart > @@ -0,0 +1,43 @@ > +import 'package:flutter/material.dart'; > +import 'package:proxmox_login_manager/widgets/pve_settings_item.dart'; > + > +class PveSettingsSection extends StatelessWidget { > + const PveSettingsSection({ > + super.key, > + required this.title, > + required this.children, > + }); > + > + /// The title of the section. > + final String title; > + > + /// The list of items in the section. > + final List<PveSettingsItem> children; > + > + @override > + Widget build(BuildContext context) { > + return Column( > + crossAxisAlignment: CrossAxisAlignment.start, > + children: [ > + ListTile( > + title: Text( > + title, > + style: const TextStyle( > + fontWeight: FontWeight.bold, > + ), > + ), > + ), > + Container( > + decoration: BoxDecoration( > + color: Theme.of(context).colorScheme.surface, > + borderRadius: BorderRadius.circular(10), > + ), > + margin: const EdgeInsets.symmetric(horizontal: 16), > + child: Column( > + children: children, > + ), > + ) > + ], > + ); > + } > +} similar here, it's only used once.... > diff --git a/pubspec.lock b/pubspec.lock please don't include unrelated pubspec.lock changes in your patches > index 06b82b6..6163a50 100644 > --- a/pubspec.lock > +++ b/pubspec.lock > @@ -29,10 +29,10 @@ packages: > dependency: transitive > description: > name: async > - sha256: "947bfcf187f74dbc5e146c9eb9c0f10c9f8b30743e341481c1e2ed3ecc18c20c" > + sha256: d2872f9c19731c2e5f10444b14686eb7cc85c76274bd6c16e1816bff9a3bab63 > url: "https://pub.dev" > source: hosted > - version: "2.11.0" > + version: "2.12.0" > biometric_storage: > dependency: "direct main" > description: > @@ -45,10 +45,10 @@ packages: > dependency: transitive > description: > name: boolean_selector > - sha256: "6cfb5af12253eaf2b368f07bacc5a80d1301a071c73360d746b7f2e32d762c66" > + sha256: "8aab1771e1243a5063b8b0ff68042d67334e3feab9e95b9490f9a6ebf73b42ea" > url: "https://pub.dev" > source: hosted > - version: "2.1.1" > + version: "2.1.2" > build: > dependency: transitive > description: > @@ -125,10 +125,10 @@ packages: > dependency: transitive > description: > name: characters > - sha256: "04a925763edad70e8443c99234dc3328f442e811f1d8fd1a72f1c8ad0f69a605" > + sha256: f71061c654a3380576a52b451dd5532377954cf9dbd272a78fc8479606670803 > url: "https://pub.dev" > source: hosted > - version: "1.3.0" > + version: "1.4.0" > checked_yaml: > dependency: transitive > description: > @@ -141,10 +141,10 @@ packages: > dependency: transitive > description: > name: clock > - sha256: cb6d7f03e1de671e34607e909a7213e31d7752be4fb66a86d29fe1eb14bfb5cf > + sha256: fddb70d9b5277016c77a80201021d40a2247104d9f4aa7bab7157b7e3f05b84b > url: "https://pub.dev" > source: hosted > - version: "1.1.1" > + version: "1.1.2" > code_builder: > dependency: transitive > description: > @@ -157,10 +157,10 @@ packages: > dependency: "direct main" > description: > name: collection > - sha256: ee67cb0715911d28db6bf4af1026078bd6f0128b07a5f66fb2ed94ec6783c09a > + sha256: "2f5709ae4d3d59dd8f7cd309b4e023046b57d8a6c82130785d2b0e5868084e76" > url: "https://pub.dev" > source: hosted > - version: "1.18.0" > + version: "1.19.1" > convert: > dependency: transitive > description: > @@ -189,10 +189,10 @@ packages: > dependency: transitive > description: > name: fake_async > - sha256: "511392330127add0b769b75a987850d136345d9227c6b94c96a04cf4a391bf78" > + sha256: "6a95e56b2449df2273fd8c45a662d6947ce1ebb7aafe80e550a3f68297f3cacc" > url: "https://pub.dev" > source: hosted > - version: "1.3.1" > + version: "1.3.2" > ffi: > dependency: transitive > description: > @@ -316,26 +316,26 @@ packages: > dependency: transitive > description: > name: leak_tracker > - sha256: "78eb209deea09858f5269f5a5b02be4049535f568c07b275096836f01ea323fa" > + sha256: c35baad643ba394b40aac41080300150a4f08fd0fd6a10378f8f7c6bc161acec > url: "https://pub.dev" > source: hosted > - version: "10.0.0" > + version: "10.0.8" > leak_tracker_flutter_testing: > dependency: transitive > description: > name: leak_tracker_flutter_testing > - sha256: b46c5e37c19120a8a01918cfaf293547f47269f7cb4b0058f21531c2465d6ef0 > + sha256: f8b613e7e6a13ec79cfdc0e97638fddb3ab848452eff057653abd3edba760573 > url: "https://pub.dev" > source: hosted > - version: "2.0.1" > + version: "3.0.9" > leak_tracker_testing: > dependency: transitive > description: > name: leak_tracker_testing > - sha256: a597f72a664dbd293f3bfc51f9ba69816f84dcd403cdac7066cb3f6003f3ab47 > + sha256: "6ba465d5d76e67ddf503e1161d1f4a6bc42306f9d66ca1e8f079a47290fb06d3" > url: "https://pub.dev" > source: hosted > - version: "2.0.1" > + version: "3.0.1" > lints: > dependency: transitive > description: > @@ -356,26 +356,26 @@ packages: > dependency: transitive > description: > name: matcher > - sha256: d2323aa2060500f906aa31a895b4030b6da3ebdcc5619d14ce1aada65cd161cb > + sha256: dc58c723c3c24bf8d3e2d3ad3f2f9d7bd9cf43ec6feaa64181775e60190153f2 > url: "https://pub.dev" > source: hosted > - version: "0.12.16+1" > + version: "0.12.17" > material_color_utilities: > dependency: transitive > description: > name: material_color_utilities > - sha256: "0e0a020085b65b6083975e499759762399b4475f766c21668c4ecca34ea74e5a" > + sha256: f7142bb1154231d7ea5f96bc7bde4bda2a0945d2806bb11670e30b850d56bdec > url: "https://pub.dev" > source: hosted > - version: "0.8.0" > + version: "0.11.1" > meta: > dependency: transitive > description: > name: meta > - sha256: d584fa6707a52763a52446f02cc621b077888fb63b93bbcb1143a7be5a0c0c04 > + sha256: e3641ec5d63ebf0d9b41bd43201a66e3fc79a65db5f61fc181f04cd27aab950c > url: "https://pub.dev" > source: hosted > - version: "1.11.0" > + version: "1.16.0" > mime: > dependency: transitive > description: > @@ -396,10 +396,10 @@ packages: > dependency: transitive > description: > name: path > - sha256: "087ce49c3f0dc39180befefc60fdb4acd8f8620e5682fe2476afd0b3688bb4af" > + sha256: "75cca69d1490965be98c73ceaea117e8a04dd21217b37b292c9ddbec0d955bc5" > url: "https://pub.dev" > source: hosted > - version: "1.9.0" > + version: "1.9.1" > path_provider_linux: > dependency: transitive > description: > @@ -555,7 +555,7 @@ packages: > dependency: transitive > description: flutter > source: sdk > - version: "0.0.99" > + version: "0.0.0" > source_gen: > dependency: transitive > description: > @@ -568,26 +568,26 @@ packages: > dependency: transitive > description: > name: source_span > - sha256: "53e943d4206a5e30df338fd4c6e7a077e02254531b138a15aec3bd143c1a8b3c" > + sha256: "254ee5351d6cb365c859e20ee823c3bb479bf4a293c22d17a9f1bf144ce86f7c" > url: "https://pub.dev" > source: hosted > - version: "1.10.0" > + version: "1.10.1" > stack_trace: > dependency: transitive > description: > name: stack_trace > - sha256: "73713990125a6d93122541237550ee3352a2d84baad52d375a4cad2eb9b7ce0b" > + sha256: "8b27215b45d22309b5cddda1aa2b19bdfec9df0e765f2de506401c071d38d1b1" > url: "https://pub.dev" > source: hosted > - version: "1.11.1" > + version: "1.12.1" > stream_channel: > dependency: transitive > description: > name: stream_channel > - sha256: ba2aa5d8cc609d96bbb2899c28934f9e1af5cddbd60a827822ea467161eb54e7 > + sha256: "969e04c80b8bcdf826f8f16579c7b14d780458bd97f56d107d3950fdbeef059d" > url: "https://pub.dev" > source: hosted > - version: "2.1.2" > + version: "2.1.4" > stream_transform: > dependency: transitive > description: > @@ -600,26 +600,26 @@ packages: > dependency: transitive > description: > name: string_scanner > - sha256: "556692adab6cfa87322a115640c11f13cb77b3f076ddcc5d6ae3c20242bedcde" > + sha256: "921cd31725b72fe181906c6a94d987c78e3b98c2e205b397ea399d4054872b43" > url: "https://pub.dev" > source: hosted > - version: "1.2.0" > + version: "1.4.1" > term_glyph: > dependency: transitive > description: > name: term_glyph > - sha256: a29248a84fbb7c79282b40b8c72a1209db169a2e0542bce341da992fe1bc7e84 > + sha256: "7f554798625ea768a7518313e58f83891c7f5024f88e46e7182a4558850a4b8e" > url: "https://pub.dev" > source: hosted > - version: "1.2.1" > + version: "1.2.2" > test_api: > dependency: transitive > description: > name: test_api > - sha256: "5c2f730018264d276c20e4f1503fd1308dfbbae39ec8ee63c5236311ac06954b" > + sha256: fb31f383e2ee25fbbfe06b40fe21e1e458d14080e3c67e7ba0acfde4df4e0bbd > url: "https://pub.dev" > source: hosted > - version: "0.6.1" > + version: "0.7.4" > timing: > dependency: transitive > description: > @@ -636,6 +636,70 @@ packages: > url: "https://pub.dev" > source: hosted > version: "1.3.2" > + url_launcher: > + dependency: "direct main" > + description: > + name: url_launcher > + sha256: "9d06212b1362abc2f0f0d78e6f09f726608c74e3b9462e8368bb03314aa8d603" > + url: "https://pub.dev" > + source: hosted > + version: "6.3.1" > + url_launcher_android: > + dependency: transitive > + description: > + name: url_launcher_android > + sha256: "8582d7f6fe14d2652b4c45c9b6c14c0b678c2af2d083a11b604caeba51930d79" > + url: "https://pub.dev" > + source: hosted > + version: "6.3.16" > + url_launcher_ios: > + dependency: transitive > + description: > + name: url_launcher_ios > + sha256: "7f2022359d4c099eea7df3fdf739f7d3d3b9faf3166fb1dd390775176e0b76cb" > + url: "https://pub.dev" > + source: hosted > + version: "6.3.3" > + url_launcher_linux: > + dependency: transitive > + description: > + name: url_launcher_linux > + sha256: "4e9ba368772369e3e08f231d2301b4ef72b9ff87c31192ef471b380ef29a4935" > + url: "https://pub.dev" > + source: hosted > + version: "3.2.1" > + url_launcher_macos: > + dependency: transitive > + description: > + name: url_launcher_macos > + sha256: "17ba2000b847f334f16626a574c702b196723af2a289e7a93ffcb79acff855c2" > + url: "https://pub.dev" > + source: hosted > + version: "3.2.2" > + url_launcher_platform_interface: > + dependency: transitive > + description: > + name: url_launcher_platform_interface > + sha256: "552f8a1e663569be95a8190206a38187b531910283c3e982193e4f2733f01029" > + url: "https://pub.dev" > + source: hosted > + version: "2.3.2" > + url_launcher_web: > + dependency: transitive > + description: > + name: url_launcher_web > + sha256: "4bd2b7b4dc4d4d0b94e5babfffbca8eac1a126c7f3d6ecbc1a11013faa3abba2" > + url: "https://pub.dev" > + source: hosted > + version: "2.4.1" > + url_launcher_windows: > + dependency: transitive > + description: > + name: url_launcher_windows > + sha256: "3284b6d2ac454cf34f114e1d3319866fdd1e19cdc329999057e44ffe936cfa77" > + url: "https://pub.dev" > + source: hosted > + version: "3.1.4" > vector_math: > dependency: transitive > description: > @@ -648,10 +712,10 @@ packages: > dependency: transitive > description: > name: vm_service > - sha256: b3d56ff4341b8f182b96aceb2fa20e3dcb336b9f867bc0eafc0de10f1048e957 > + sha256: "0968250880a6c5fe7edc067ed0a13d4bae1577fe2771dcf3010d52c4a9d3ca14" > url: "https://pub.dev" > source: hosted > - version: "13.0.0" > + version: "14.3.1" > watcher: > dependency: transitive > description: > @@ -701,5 +765,5 @@ packages: > source: hosted > version: "3.1.2" > sdks: > - dart: ">=3.3.0 <4.0.0" > - flutter: ">=3.19.0" > + dart: ">=3.7.0-0 <4.0.0" > + flutter: ">=3.27.0" > diff --git a/pubspec.yaml b/pubspec.yaml > index 99f9870..1e48217 100644 > --- a/pubspec.yaml > +++ b/pubspec.yaml > @@ -14,6 +14,7 @@ dependencies: > biometric_storage: ^5.0.0 > built_value: ^8.4.1 > built_collection: ^5.0.0 > + url_launcher: ^6.3.1 > proxmox_dart_api_client: > path: ../proxmox_dart_api_client > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen 2025-07-03 14:00 ` Dominik Csapak @ 2025-07-04 7:16 ` Shan Shaji 2025-07-04 7:31 ` Dominik Csapak 2025-07-04 14:40 ` Shan Shaji 1 sibling, 1 reply; 9+ messages in thread From: Shan Shaji @ 2025-07-04 7:16 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Thanks for the review. I will update the changes according to the reviews. I have some doubts that i have added as inline comments. On Thu Jul 3, 2025 at 4:00 PM CEST, Dominik Csapak wrote: > hi, > > high level comment: > > I don't think it's necessary or wise to refactor upfront. You create new layouting widgets > that are only used once(PveSettingsSection, PveSettingsItem) and create classes with functionality > that is never uses (the mode in openUrl?) > > I think it's best to just use the regular flutter widgets inline and keep methods > and classes small to do what is actually needed. We can still extend them when > more is required, but there is no advantage from having dead code. > > a few comments inline > > On 7/3/25 13:19, Shan Shaji wrote: > > According to Apple's App Store review guidelines all apps must include a > > link to their privacy policy within the App [0]. To fix the issue add a > > new list item in the settings screen that will allow users to access the > > privacy policy. > > > > [0] - https://developer.apple.com/app-store/review/guidelines/#legal > > > > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > > --- > > lib/constants/pve_links.dart | 6 + > > lib/proxmox_general_settings_form.dart | 32 ++++++ > > lib/utils/pve_url_launcher_util.dart | 24 ++++ > > lib/utils/result.dart | 34 ++++++ > > lib/widgets/pve_settings_item.dart | 61 ++++++++++ > > lib/widgets/pve_settings_section.dart | 43 +++++++ > > pubspec.lock | 150 ++++++++++++++++++------- > > pubspec.yaml | 1 + > > 8 files changed, 308 insertions(+), 43 deletions(-) > > create mode 100644 lib/constants/pve_links.dart > > create mode 100644 lib/utils/pve_url_launcher_util.dart > > create mode 100644 lib/utils/result.dart > > create mode 100644 lib/widgets/pve_settings_item.dart > > create mode 100644 lib/widgets/pve_settings_section.dart > > > > diff --git a/lib/constants/pve_links.dart b/lib/constants/pve_links.dart > > new file mode 100644 > > index 0000000..196b58c > > --- /dev/null > > +++ b/lib/constants/pve_links.dart > > @@ -0,0 +1,6 @@ > > +class PveLinks { > > + PveLinks._(); > > + > > + static const String privacyPolicyUrl = > > + 'https://pve.proxmox.com/wiki/Proxmox_VE_Mobile_Companion_Data_Protection'; > > +} > > IMHO it does not really make sense to define a class to hold things just for a single element. > I'd put the link where we actually call it, and only refactor it when it's necessary, > e.g. when we often use the same link from different places, etc. Rather than defining the class to hold the constant string. Would like to know your opinion about just defining the `privacyPolicyUrl` as variable in the file? Usualy we do seperate constants like label strings, links, spacings etc.. to its own file as it can be re-used and new values can be directly added to those files. > > diff --git a/lib/proxmox_general_settings_form.dart b/lib/proxmox_general_settings_form.dart > > index cb0afef..b81af3a 100644 > > --- a/lib/proxmox_general_settings_form.dart > > +++ b/lib/proxmox_general_settings_form.dart > > @@ -1,5 +1,10 @@ > > import 'package:flutter/material.dart'; > > +import 'package:proxmox_login_manager/constants/pve_links.dart'; > > import 'package:proxmox_login_manager/proxmox_general_settings_model.dart'; > > +import 'package:proxmox_login_manager/utils/pve_url_launcher_util.dart'; > > +import 'package:proxmox_login_manager/utils/result.dart'; > > +import 'package:proxmox_login_manager/widgets/pve_settings_item.dart'; > > +import 'package:proxmox_login_manager/widgets/pve_settings_section.dart'; > > > > class ProxmoxGeneralSettingsForm extends StatefulWidget { > > const ProxmoxGeneralSettingsForm({super.key}); > > @@ -21,6 +26,7 @@ class _ProxmoxGeneralSettingsFormState > > @override > > Widget build(BuildContext context) { > > return Scaffold( > > + backgroundColor: Theme.of(context).colorScheme.surfaceContainer, > > this change seems to be unrelated? > > > appBar: AppBar( > > title: const Text('Settings'), > > ), > > @@ -45,6 +51,32 @@ class _ProxmoxGeneralSettingsFormState > > ProxmoxGeneralSettingsModel.fromLocalStorage(); > > }); > > }, > > + ), > > + PveSettingsSection( > > + title: 'LEGAL', > > + children: [ > > + PveSettingsItem( > > + titleString: 'Privacy Policy', > > + leadingIcon: Icons.privacy_tip_outlined, > > + trailingIcon: Icons.open_in_new, > > + onTap: () async { > > + final result = await PveUrlLauncherUtil.openUrl( > > + Uri.parse(PveLinks.privacyPolicyUrl), > > + ); > > + > > + if (result is Error) { > > + if (!context.mounted) return; > > + ScaffoldMessenger.of(context).showSnackBar( > > + SnackBar( > > + content: Text( > > + result.error.toString(), > > + ), > > + ), > > + ); > > + } > > + }, > > + ), > > + ], > > here you could simply use the column with 2 listtiles. > > > ) > > ], > > ), > > diff --git a/lib/utils/pve_url_launcher_util.dart b/lib/utils/pve_url_launcher_util.dart > > new file mode 100644 > > index 0000000..8820d75 > > --- /dev/null > > +++ b/lib/utils/pve_url_launcher_util.dart > > @@ -0,0 +1,24 @@ > > +import 'package:proxmox_login_manager/utils/result.dart'; > > +import 'package:url_launcher/url_launcher.dart'; > > + > > +class PveUrlLauncherUtil { > > + PveUrlLauncherUtil._(); > > + > > + static Future<Result> openUrl( > > + Uri uri, { > > + LaunchMode mode = LaunchMode.inAppBrowserView, > > FWICT the mode parameter is not used anywhere, so it's not needed. > Actually we already have a wrapper around url_launcher in pve_flutter_frontend > > We don't have a good place to put things that are shared between > the pve_flutter_frontend and login_manager, maybe this is a sign > that we should create such a place. > > Otherwise you could simply copy the tryLaunchUrl method here. That makes sense. We will need a seperate package that shares common things between the login manager and the pve front end. This is just my doubt. Can i ask why we seperated the proxmox_login_manager as a seperate package? > > + }) async { > > + const errorMessage = 'Could not launch url'; > > + try { > > + if (await canLaunchUrl(uri)) { > > + final result = await launchUrl(uri, mode: mode); > > + if (result) return Result.ok(true); > > + return Result.error(Exception(errorMessage)); > > + } else { > > + return Result.error(Exception(errorMessage)); > > + } > > + } on Exception catch (e, _) { > > + return Result.error(e); > > + } > > + } > > +} > > diff --git a/lib/utils/result.dart b/lib/utils/result.dart > > new file mode 100644 > > index 0000000..5454e9e > > --- /dev/null > > +++ b/lib/utils/result.dart > > @@ -0,0 +1,34 @@ > > +/// Utility class that simplifies handling errors. > > +/// > > +/// Return a [Result] from a function to indicate success or failure. > > +/// > > +/// A [Result] is either an [Ok] with a value of type [T] > > +/// or an [Error] with an [Exception]. > > +/// > > +/// Use [Result.ok] to create a successful result with a value of type [T]. > > +/// Use [Result.error] to create an error result with an [Exception]. > > +sealed class Result<T> { > > + const Result(); > > + > > + /// Creates an instance of Result containing a value > > + factory Result.ok(T value) => Ok(value); > > + > > + /// Create an instance of Result containing an error > > + factory Result.error(Exception error) => Error(error); > > +} > > + > > +/// Subclass of Result for values > > +final class Ok<T> extends Result<T> { > > + const Ok(this.value); > > + > > + /// Returned value in result > > + final T value; > > +} > > + > > +/// Subclass of Result for errors > > +final class Error<T> extends Result<T> { > > + const Error(this.error); > > + > > + /// Returned error in result > > + final Exception error; > > +} > > while I appreciate the sentiment of having a result class akin to what we have in rust. > creating this class just to use it in one or two places is overkill IMHO > > If we want to do something like this, it would be better to introduce this in it's > own patch and use it everywhere it makes sense (also in pve_flutter_frontent, which > indicates we probably should invent a package that can be used by both...) > > but I think we could just live with usual exception handling too.. Makes sense. Thank you > > diff --git a/lib/widgets/pve_settings_item.dart b/lib/widgets/pve_settings_item.dart > > new file mode 100644 > > index 0000000..a1648aa > > --- /dev/null > > +++ b/lib/widgets/pve_settings_item.dart > > @@ -0,0 +1,61 @@ > > +import 'package:flutter/material.dart'; > > + > > +class PveSettingsItem extends StatelessWidget { > > + const PveSettingsItem({ > > + super.key, > > + this.titleString, > > + this.leadingIcon, > > + this.trailingIcon, > > + this.leading, > > + this.trailing, > > + this.title, > > + this.onTap, > > + }) : assert( > > + leading != null || leadingIcon != null, > > + "Either leading or leadingIcon must be provided", > > + ), > > + assert( > > + trailing != null || trailingIcon != null, > > + 'Either trailing or trailingIcon must be provided', > > + ), > > + assert( > > + titleString != null || title != null, > > + 'Either titleString or title must be provided', > > + ); > > + > > + /// The text to display as the title. > > + final String? titleString; > > + > > + /// An icon to display before the title. > > + /// > > + /// If [leading] widget is not null, [leadingIcon] is ignored. > > + final IconData? leadingIcon; > > + > > + /// An icon to display after the title. > > + /// > > + /// If [trailing] property is not null, [trailingIcon] is ignored. > > + final IconData? trailingIcon; > > + > > + /// A widget to display before the title. > > + final Widget? leading; > > + > > + /// A widget to display after the title. > > + final Widget? trailing; > > + > > + /// A widget to display as the title. > > + final Widget? title; > > + > > + /// A callback that is called when the user taps on the ListTile. > > + final VoidCallback? onTap; > > + > > + @override > > + Widget build(BuildContext context) { > > + return ListTile( > > + shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(10)), > > + leading: leading ?? Icon(leadingIcon), > > + title: title ?? (titleString != null ? Text(titleString!) : null), > > + trailing: trailing ?? Icon(trailingIcon), > > + onTap: onTap, > > + ); > > + } > > +} > > This widget is only used once, and mostly just passes through options to a listtile, > so i don't think it's worthy of it's own widget... > > > diff --git a/lib/widgets/pve_settings_section.dart b/lib/widgets/pve_settings_section.dart > > new file mode 100644 > > index 0000000..d22c630 > > --- /dev/null > > +++ b/lib/widgets/pve_settings_section.dart > > @@ -0,0 +1,43 @@ > > +import 'package:flutter/material.dart'; > > +import 'package:proxmox_login_manager/widgets/pve_settings_item.dart'; > > + > > +class PveSettingsSection extends StatelessWidget { > > + const PveSettingsSection({ > > + super.key, > > + required this.title, > > + required this.children, > > + }); > > + > > + /// The title of the section. > > + final String title; > > + > > + /// The list of items in the section. > > + final List<PveSettingsItem> children; > > + > > + @override > > + Widget build(BuildContext context) { > > + return Column( > > + crossAxisAlignment: CrossAxisAlignment.start, > > + children: [ > > + ListTile( > > + title: Text( > > + title, > > + style: const TextStyle( > > + fontWeight: FontWeight.bold, > > + ), > > + ), > > + ), > > + Container( > > + decoration: BoxDecoration( > > + color: Theme.of(context).colorScheme.surface, > > + borderRadius: BorderRadius.circular(10), > > + ), > > + margin: const EdgeInsets.symmetric(horizontal: 16), > > + child: Column( > > + children: children, > > + ), > > + ) > > + ], > > + ); > > + } > > +} > > similar here, it's only used once.... > > > diff --git a/pubspec.lock b/pubspec.lock > > please don't include unrelated pubspec.lock changes in your patches The .lock file is not up to date with our new flutter version we are using. I will create a seperate patch for this. > > index 06b82b6..6163a50 100644 > > --- a/pubspec.lock > > +++ b/pubspec.lock > > @@ -29,10 +29,10 @@ packages: > > dependency: transitive > > description: > > name: async > > - sha256: "947bfcf187f74dbc5e146c9eb9c0f10c9f8b30743e341481c1e2ed3ecc18c20c" > > + sha256: d2872f9c19731c2e5f10444b14686eb7cc85c76274bd6c16e1816bff9a3bab63 > > url: "https://pub.dev" > > source: hosted > > - version: "2.11.0" > > + version: "2.12.0" > > biometric_storage: > > dependency: "direct main" > > description: > > @@ -45,10 +45,10 @@ packages: > > dependency: transitive > > description: > > name: boolean_selector > > - sha256: "6cfb5af12253eaf2b368f07bacc5a80d1301a071c73360d746b7f2e32d762c66" > > + sha256: "8aab1771e1243a5063b8b0ff68042d67334e3feab9e95b9490f9a6ebf73b42ea" > > url: "https://pub.dev" > > source: hosted > > - version: "2.1.1" > > + version: "2.1.2" > > build: > > dependency: transitive > > description: > > @@ -125,10 +125,10 @@ packages: > > dependency: transitive > > description: > > name: characters > > - sha256: "04a925763edad70e8443c99234dc3328f442e811f1d8fd1a72f1c8ad0f69a605" > > + sha256: f71061c654a3380576a52b451dd5532377954cf9dbd272a78fc8479606670803 > > url: "https://pub.dev" > > source: hosted > > - version: "1.3.0" > > + version: "1.4.0" > > checked_yaml: > > dependency: transitive > > description: > > @@ -141,10 +141,10 @@ packages: > > dependency: transitive > > description: > > name: clock > > - sha256: cb6d7f03e1de671e34607e909a7213e31d7752be4fb66a86d29fe1eb14bfb5cf > > + sha256: fddb70d9b5277016c77a80201021d40a2247104d9f4aa7bab7157b7e3f05b84b > > url: "https://pub.dev" > > source: hosted > > - version: "1.1.1" > > + version: "1.1.2" > > code_builder: > > dependency: transitive > > description: > > @@ -157,10 +157,10 @@ packages: > > dependency: "direct main" > > description: > > name: collection > > - sha256: ee67cb0715911d28db6bf4af1026078bd6f0128b07a5f66fb2ed94ec6783c09a > > + sha256: "2f5709ae4d3d59dd8f7cd309b4e023046b57d8a6c82130785d2b0e5868084e76" > > url: "https://pub.dev" > > source: hosted > > - version: "1.18.0" > > + version: "1.19.1" > > convert: > > dependency: transitive > > description: > > @@ -189,10 +189,10 @@ packages: > > dependency: transitive > > description: > > name: fake_async > > - sha256: "511392330127add0b769b75a987850d136345d9227c6b94c96a04cf4a391bf78" > > + sha256: "6a95e56b2449df2273fd8c45a662d6947ce1ebb7aafe80e550a3f68297f3cacc" > > url: "https://pub.dev" > > source: hosted > > - version: "1.3.1" > > + version: "1.3.2" > > ffi: > > dependency: transitive > > description: > > @@ -316,26 +316,26 @@ packages: > > dependency: transitive > > description: > > name: leak_tracker > > - sha256: "78eb209deea09858f5269f5a5b02be4049535f568c07b275096836f01ea323fa" > > + sha256: c35baad643ba394b40aac41080300150a4f08fd0fd6a10378f8f7c6bc161acec > > url: "https://pub.dev" > > source: hosted > > - version: "10.0.0" > > + version: "10.0.8" > > leak_tracker_flutter_testing: > > dependency: transitive > > description: > > name: leak_tracker_flutter_testing > > - sha256: b46c5e37c19120a8a01918cfaf293547f47269f7cb4b0058f21531c2465d6ef0 > > + sha256: f8b613e7e6a13ec79cfdc0e97638fddb3ab848452eff057653abd3edba760573 > > url: "https://pub.dev" > > source: hosted > > - version: "2.0.1" > > + version: "3.0.9" > > leak_tracker_testing: > > dependency: transitive > > description: > > name: leak_tracker_testing > > - sha256: a597f72a664dbd293f3bfc51f9ba69816f84dcd403cdac7066cb3f6003f3ab47 > > + sha256: "6ba465d5d76e67ddf503e1161d1f4a6bc42306f9d66ca1e8f079a47290fb06d3" > > url: "https://pub.dev" > > source: hosted > > - version: "2.0.1" > > + version: "3.0.1" > > lints: > > dependency: transitive > > description: > > @@ -356,26 +356,26 @@ packages: > > dependency: transitive > > description: > > name: matcher > > - sha256: d2323aa2060500f906aa31a895b4030b6da3ebdcc5619d14ce1aada65cd161cb > > + sha256: dc58c723c3c24bf8d3e2d3ad3f2f9d7bd9cf43ec6feaa64181775e60190153f2 > > url: "https://pub.dev" > > source: hosted > > - version: "0.12.16+1" > > + version: "0.12.17" > > material_color_utilities: > > dependency: transitive > > description: > > name: material_color_utilities > > - sha256: "0e0a020085b65b6083975e499759762399b4475f766c21668c4ecca34ea74e5a" > > + sha256: f7142bb1154231d7ea5f96bc7bde4bda2a0945d2806bb11670e30b850d56bdec > > url: "https://pub.dev" > > source: hosted > > - version: "0.8.0" > > + version: "0.11.1" > > meta: > > dependency: transitive > > description: > > name: meta > > - sha256: d584fa6707a52763a52446f02cc621b077888fb63b93bbcb1143a7be5a0c0c04 > > + sha256: e3641ec5d63ebf0d9b41bd43201a66e3fc79a65db5f61fc181f04cd27aab950c > > url: "https://pub.dev" > > source: hosted > > - version: "1.11.0" > > + version: "1.16.0" > > mime: > > dependency: transitive > > description: > > @@ -396,10 +396,10 @@ packages: > > dependency: transitive > > description: > > name: path > > - sha256: "087ce49c3f0dc39180befefc60fdb4acd8f8620e5682fe2476afd0b3688bb4af" > > + sha256: "75cca69d1490965be98c73ceaea117e8a04dd21217b37b292c9ddbec0d955bc5" > > url: "https://pub.dev" > > source: hosted > > - version: "1.9.0" > > + version: "1.9.1" > > path_provider_linux: > > dependency: transitive > > description: > > @@ -555,7 +555,7 @@ packages: > > dependency: transitive > > description: flutter > > source: sdk > > - version: "0.0.99" > > + version: "0.0.0" > > source_gen: > > dependency: transitive > > description: > > @@ -568,26 +568,26 @@ packages: > > dependency: transitive > > description: > > name: source_span > > - sha256: "53e943d4206a5e30df338fd4c6e7a077e02254531b138a15aec3bd143c1a8b3c" > > + sha256: "254ee5351d6cb365c859e20ee823c3bb479bf4a293c22d17a9f1bf144ce86f7c" > > url: "https://pub.dev" > > source: hosted > > - version: "1.10.0" > > + version: "1.10.1" > > stack_trace: > > dependency: transitive > > description: > > name: stack_trace > > - sha256: "73713990125a6d93122541237550ee3352a2d84baad52d375a4cad2eb9b7ce0b" > > + sha256: "8b27215b45d22309b5cddda1aa2b19bdfec9df0e765f2de506401c071d38d1b1" > > url: "https://pub.dev" > > source: hosted > > - version: "1.11.1" > > + version: "1.12.1" > > stream_channel: > > dependency: transitive > > description: > > name: stream_channel > > - sha256: ba2aa5d8cc609d96bbb2899c28934f9e1af5cddbd60a827822ea467161eb54e7 > > + sha256: "969e04c80b8bcdf826f8f16579c7b14d780458bd97f56d107d3950fdbeef059d" > > url: "https://pub.dev" > > source: hosted > > - version: "2.1.2" > > + version: "2.1.4" > > stream_transform: > > dependency: transitive > > description: > > @@ -600,26 +600,26 @@ packages: > > dependency: transitive > > description: > > name: string_scanner > > - sha256: "556692adab6cfa87322a115640c11f13cb77b3f076ddcc5d6ae3c20242bedcde" > > + sha256: "921cd31725b72fe181906c6a94d987c78e3b98c2e205b397ea399d4054872b43" > > url: "https://pub.dev" > > source: hosted > > - version: "1.2.0" > > + version: "1.4.1" > > term_glyph: > > dependency: transitive > > description: > > name: term_glyph > > - sha256: a29248a84fbb7c79282b40b8c72a1209db169a2e0542bce341da992fe1bc7e84 > > + sha256: "7f554798625ea768a7518313e58f83891c7f5024f88e46e7182a4558850a4b8e" > > url: "https://pub.dev" > > source: hosted > > - version: "1.2.1" > > + version: "1.2.2" > > test_api: > > dependency: transitive > > description: > > name: test_api > > - sha256: "5c2f730018264d276c20e4f1503fd1308dfbbae39ec8ee63c5236311ac06954b" > > + sha256: fb31f383e2ee25fbbfe06b40fe21e1e458d14080e3c67e7ba0acfde4df4e0bbd > > url: "https://pub.dev" > > source: hosted > > - version: "0.6.1" > > + version: "0.7.4" > > timing: > > dependency: transitive > > description: > > @@ -636,6 +636,70 @@ packages: > > url: "https://pub.dev" > > source: hosted > > version: "1.3.2" > > + url_launcher: > > + dependency: "direct main" > > + description: > > + name: url_launcher > > + sha256: "9d06212b1362abc2f0f0d78e6f09f726608c74e3b9462e8368bb03314aa8d603" > > + url: "https://pub.dev" > > + source: hosted > > + version: "6.3.1" > > + url_launcher_android: > > + dependency: transitive > > + description: > > + name: url_launcher_android > > + sha256: "8582d7f6fe14d2652b4c45c9b6c14c0b678c2af2d083a11b604caeba51930d79" > > + url: "https://pub.dev" > > + source: hosted > > + version: "6.3.16" > > + url_launcher_ios: > > + dependency: transitive > > + description: > > + name: url_launcher_ios > > + sha256: "7f2022359d4c099eea7df3fdf739f7d3d3b9faf3166fb1dd390775176e0b76cb" > > + url: "https://pub.dev" > > + source: hosted > > + version: "6.3.3" > > + url_launcher_linux: > > + dependency: transitive > > + description: > > + name: url_launcher_linux > > + sha256: "4e9ba368772369e3e08f231d2301b4ef72b9ff87c31192ef471b380ef29a4935" > > + url: "https://pub.dev" > > + source: hosted > > + version: "3.2.1" > > + url_launcher_macos: > > + dependency: transitive > > + description: > > + name: url_launcher_macos > > + sha256: "17ba2000b847f334f16626a574c702b196723af2a289e7a93ffcb79acff855c2" > > + url: "https://pub.dev" > > + source: hosted > > + version: "3.2.2" > > + url_launcher_platform_interface: > > + dependency: transitive > > + description: > > + name: url_launcher_platform_interface > > + sha256: "552f8a1e663569be95a8190206a38187b531910283c3e982193e4f2733f01029" > > + url: "https://pub.dev" > > + source: hosted > > + version: "2.3.2" > > + url_launcher_web: > > + dependency: transitive > > + description: > > + name: url_launcher_web > > + sha256: "4bd2b7b4dc4d4d0b94e5babfffbca8eac1a126c7f3d6ecbc1a11013faa3abba2" > > + url: "https://pub.dev" > > + source: hosted > > + version: "2.4.1" > > + url_launcher_windows: > > + dependency: transitive > > + description: > > + name: url_launcher_windows > > + sha256: "3284b6d2ac454cf34f114e1d3319866fdd1e19cdc329999057e44ffe936cfa77" > > + url: "https://pub.dev" > > + source: hosted > > + version: "3.1.4" > > vector_math: > > dependency: transitive > > description: > > @@ -648,10 +712,10 @@ packages: > > dependency: transitive > > description: > > name: vm_service > > - sha256: b3d56ff4341b8f182b96aceb2fa20e3dcb336b9f867bc0eafc0de10f1048e957 > > + sha256: "0968250880a6c5fe7edc067ed0a13d4bae1577fe2771dcf3010d52c4a9d3ca14" > > url: "https://pub.dev" > > source: hosted > > - version: "13.0.0" > > + version: "14.3.1" > > watcher: > > dependency: transitive > > description: > > @@ -701,5 +765,5 @@ packages: > > source: hosted > > version: "3.1.2" > > sdks: > > - dart: ">=3.3.0 <4.0.0" > > - flutter: ">=3.19.0" > > + dart: ">=3.7.0-0 <4.0.0" > > + flutter: ">=3.27.0" > > diff --git a/pubspec.yaml b/pubspec.yaml > > index 99f9870..1e48217 100644 > > --- a/pubspec.yaml > > +++ b/pubspec.yaml > > @@ -14,6 +14,7 @@ dependencies: > > biometric_storage: ^5.0.0 > > built_value: ^8.4.1 > > built_collection: ^5.0.0 > > + url_launcher: ^6.3.1 > > proxmox_dart_api_client: > > path: ../proxmox_dart_api_client > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen 2025-07-04 7:16 ` Shan Shaji @ 2025-07-04 7:31 ` Dominik Csapak 2025-07-04 9:34 ` Thomas Lamprecht 0 siblings, 1 reply; 9+ messages in thread From: Dominik Csapak @ 2025-07-04 7:31 UTC (permalink / raw) To: Shan Shaji, Proxmox VE development discussion On 7/4/25 09:16, Shan Shaji wrote: > Thanks for the review. I will update the changes according to the > reviews. I have some doubts that i have added as inline comments. > [snip] >>> diff --git a/lib/constants/pve_links.dart b/lib/constants/pve_links.dart >>> new file mode 100644 >>> index 0000000..196b58c >>> --- /dev/null >>> +++ b/lib/constants/pve_links.dart >>> @@ -0,0 +1,6 @@ >>> +class PveLinks { >>> + PveLinks._(); >>> + >>> + static const String privacyPolicyUrl = >>> + 'https://pve.proxmox.com/wiki/Proxmox_VE_Mobile_Companion_Data_Protection'; >>> +} >> >> IMHO it does not really make sense to define a class to hold things just for a single element. >> I'd put the link where we actually call it, and only refactor it when it's necessary, >> e.g. when we often use the same link from different places, etc. > > Rather than defining the class to hold the constant string. Would like > to know your opinion about just defining the `privacyPolicyUrl` as variable in the > file? Usualy we do seperate constants like label strings, links, > spacings etc.. to its own file as it can be re-used and new values can > be directly added to those files. > yes that sounds like a good middle ground.. [snip] >>> diff --git a/lib/utils/pve_url_launcher_util.dart b/lib/utils/pve_url_launcher_util.dart >>> new file mode 100644 >>> index 0000000..8820d75 >>> --- /dev/null >>> +++ b/lib/utils/pve_url_launcher_util.dart >>> @@ -0,0 +1,24 @@ >>> +import 'package:proxmox_login_manager/utils/result.dart'; >>> +import 'package:url_launcher/url_launcher.dart'; >>> + >>> +class PveUrlLauncherUtil { >>> + PveUrlLauncherUtil._(); >>> + >>> + static Future<Result> openUrl( >>> + Uri uri, { >>> + LaunchMode mode = LaunchMode.inAppBrowserView, >> >> FWICT the mode parameter is not used anywhere, so it's not needed. >> Actually we already have a wrapper around url_launcher in pve_flutter_frontend >> >> We don't have a good place to put things that are shared between >> the pve_flutter_frontend and login_manager, maybe this is a sign >> that we should create such a place. >> >> Otherwise you could simply copy the tryLaunchUrl method here. > > That makes sense. We will need a seperate package that shares > common things between the login manager and the pve front end. > > This is just my doubt. Can i ask why we seperated the > proxmox_login_manager as a seperate package? I'm not sure why we did this, but my guess would be that it was planned to maybe have multiple apps in the future? @Tim i think you were intially the one doing that, do you remember why we did the split? _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen 2025-07-04 7:31 ` Dominik Csapak @ 2025-07-04 9:34 ` Thomas Lamprecht 2025-07-04 12:11 ` Shan Shaji 0 siblings, 1 reply; 9+ messages in thread From: Thomas Lamprecht @ 2025-07-04 9:34 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak, Shan Shaji Am 04.07.25 um 09:31 schrieb Dominik Csapak: >> >> This is just my doubt. Can i ask why we seperated the >> proxmox_login_manager as a seperate package? > I'm not sure why we did this, but my guess would be that it was planned to maybe have > multiple apps in the future? > > @Tim i think you were intially the one doing that, do you remember why we did the split? > For having a shared basis for other products, there is a prototype for PBS on the staff repos IIRC. While we do not plan to develop that further at the moment, having to split it out again in the future is definitively quite a bit of extra work, while keeping it split shouldn't be so hard I think. So, while not a must, I'd I'd keep it split for the time being, if there are no bigger reasons speaking against that. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen 2025-07-04 9:34 ` Thomas Lamprecht @ 2025-07-04 12:11 ` Shan Shaji 0 siblings, 0 replies; 9+ messages in thread From: Shan Shaji @ 2025-07-04 12:11 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion, Dominik Csapak Got it. Thanks for sharing. On Fri Jul 4, 2025 at 11:34 AM CEST, Thomas Lamprecht wrote: > Am 04.07.25 um 09:31 schrieb Dominik Csapak: > >> > >> This is just my doubt. Can i ask why we seperated the > >> proxmox_login_manager as a seperate package? > > I'm not sure why we did this, but my guess would be that it was planned to maybe have > > multiple apps in the future? > > > > @Tim i think you were intially the one doing that, do you remember why we did the split? > > > > For having a shared basis for other products, there is a prototype for > PBS on the staff repos IIRC. While we do not plan to develop that further > at the moment, having to split it out again in the future is definitively > quite a bit of extra work, while keeping it split shouldn't be so hard > I think. So, while not a must, I'd I'd keep it split for the time being, > if there are no bigger reasons speaking against that. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen 2025-07-03 14:00 ` Dominik Csapak 2025-07-04 7:16 ` Shan Shaji @ 2025-07-04 14:40 ` Shan Shaji 2025-07-07 6:28 ` Dominik Csapak 1 sibling, 1 reply; 9+ messages in thread From: Shan Shaji @ 2025-07-04 14:40 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion On Thu Jul 3, 2025 at 4:00 PM CEST, Dominik Csapak wrote: > > + backgroundColor: Theme.of(context).colorScheme.surfaceContainer, > > this change seems to be unrelated? This was intended. The change was made to match the backgroundColor of `ProxmoxLoginSelector`. Previously it was using the default scaffold backgroundColor. > > > appBar: AppBar( > > title: const Text('Settings'), > > ), > > @@ -45,6 +51,32 @@ class _ProxmoxGeneralSettingsFormState > > ProxmoxGeneralSettingsModel.fromLocalStorage(); > > }); > > }, > > + ), > > + PveSettingsSection( > > + title: 'LEGAL', > > + children: [ > > + PveSettingsItem( > > + titleString: 'Privacy Policy', > > + leadingIcon: Icons.privacy_tip_outlined, _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen 2025-07-04 14:40 ` Shan Shaji @ 2025-07-07 6:28 ` Dominik Csapak 2025-07-07 7:09 ` Shan Shaji 0 siblings, 1 reply; 9+ messages in thread From: Dominik Csapak @ 2025-07-07 6:28 UTC (permalink / raw) To: Shan Shaji, Proxmox VE development discussion On 7/4/25 16:40, Shan Shaji wrote: > On Thu Jul 3, 2025 at 4:00 PM CEST, Dominik Csapak wrote: >>> + backgroundColor: Theme.of(context).colorScheme.surfaceContainer, >> >> this change seems to be unrelated? > This was intended. The change was made to match the backgroundColor of > `ProxmoxLoginSelector`. Previously it was using the default scaffold > backgroundColor. ok. In this case it was not really clear why it was done in this patch, so a short sentence that this is changed too wouldn't hurt IMHO. >> >>> appBar: AppBar( >>> title: const Text('Settings'), >>> ), >>> @@ -45,6 +51,32 @@ class _ProxmoxGeneralSettingsFormState >>> ProxmoxGeneralSettingsModel.fromLocalStorage(); >>> }); >>> }, >>> + ), >>> + PveSettingsSection( >>> + title: 'LEGAL', >>> + children: [ >>> + PveSettingsItem( >>> + titleString: 'Privacy Policy', >>> + leadingIcon: Icons.privacy_tip_outlined, _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen 2025-07-07 6:28 ` Dominik Csapak @ 2025-07-07 7:09 ` Shan Shaji 0 siblings, 0 replies; 9+ messages in thread From: Shan Shaji @ 2025-07-07 7:09 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Thank you Dominik, Will keep that in mind. I have created a new patch by including the changes that you mentioned. superseeded by v2: https://lore.proxmox.com/pve-devel/20250704152654.21221-1-s.shaji@proxmox.com/T/#t On Mon Jul 7, 2025 at 8:28 AM CEST, Dominik Csapak wrote: > On 7/4/25 16:40, Shan Shaji wrote: > > On Thu Jul 3, 2025 at 4:00 PM CEST, Dominik Csapak wrote: > >>> + backgroundColor: Theme.of(context).colorScheme.surfaceContainer, > >> > >> this change seems to be unrelated? > > This was intended. The change was made to match the backgroundColor of > > `ProxmoxLoginSelector`. Previously it was using the default scaffold > > backgroundColor. > > ok. In this case it was not really clear why it was done in this patch, so a short > sentence that this is changed too wouldn't hurt IMHO. Also this change is not included in the new patch. I have removed it. > >> > >>> appBar: AppBar( > >>> title: const Text('Settings'), > >>> ), > >>> @@ -45,6 +51,32 @@ class _ProxmoxGeneralSettingsFormState > >>> ProxmoxGeneralSettingsModel.fromLocalStorage(); > >>> }); > >>> }, > >>> + ), > >>> + PveSettingsSection( > >>> + title: 'LEGAL', > >>> + children: [ > >>> + PveSettingsItem( > >>> + titleString: 'Privacy Policy', > >>> + leadingIcon: Icons.privacy_tip_outlined, _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-07-07 7:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-07-03 11:19 [pve-devel] [PATCH proxmox_login_manager] ui: settings: add privacy policy url link in settings screen Shan Shaji 2025-07-03 14:00 ` Dominik Csapak 2025-07-04 7:16 ` Shan Shaji 2025-07-04 7:31 ` Dominik Csapak 2025-07-04 9:34 ` Thomas Lamprecht 2025-07-04 12:11 ` Shan Shaji 2025-07-04 14:40 ` Shan Shaji 2025-07-07 6:28 ` Dominik Csapak 2025-07-07 7:09 ` Shan Shaji
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.