Merge pull request #744 from nextcloud/refactor/neon/settings

Refactor/neon/settings

packages/neon/neon/lib/settings.dart

@@ -1,7 +1,6 @@
 export 'package:neon/src/models/label_builder.dart';
+export 'package:neon/src/settings/models/option.dart';
 export 'package:neon/src/settings/models/options_category.dart';
 export 'package:neon/src/settings/models/options_collection.dart';
-export 'package:neon/src/settings/models/select_option.dart';
 export 'package:neon/src/settings/models/storage.dart' show Storable;
-export 'package:neon/src/settings/models/toggle_option.dart';
 export 'package:neon/src/settings/widgets/settings_list.dart';

packages/neon/neon/lib/src/pages/nextcloud_app_settings.dart

@@ -3,10 +3,7 @@ import 'package:flutter_material_design_icons/flutter_material_design_icons.dart
 import 'package:meta/meta.dart';
 import 'package:neon/l10n/localizations.dart';
 import 'package:neon/src/models/app_implementation.dart';
-import 'package:neon/src/settings/models/select_option.dart';
+import 'package:neon/src/settings/widgets/option_settings_tile.dart';
-import 'package:neon/src/settings/models/toggle_option.dart';
-import 'package:neon/src/settings/widgets/checkbox_settings_tile.dart';
-import 'package:neon/src/settings/widgets/dropdown_button_settings_tile.dart';
 import 'package:neon/src/settings/widgets/settings_category.dart';
 import 'package:neon/src/settings/widgets/settings_list.dart';
 import 'package:neon/src/theme/dialog.dart';
@@ -52,15 +49,7 @@ class NextcloudAppSettingsPage extends StatelessWidget {
               tiles: [
                 for (final option
                     in appImplementation.options.options.where((final option) => option.category == category)) ...[
-                  if (option is ToggleOption) ...[
+                  OptionSettingsTile(option: option),
-                    CheckBoxSettingsTile(
-                      option: option,
-                    ),
-                  ] else if (option is SelectOption) ...[
-                    DropdownButtonSettingsTile(
-                      option: option,
-                    ),
-                  ],
                 ],
               ],
             ),

packages/neon/neon/lib/src/settings/models/option.dart

@@ -1,5 +1,6 @@
 import 'dart:async';
 
+import 'package:collection/collection.dart';
 import 'package:flutter/foundation.dart';
 import 'package:meta/meta.dart';
 import 'package:neon/src/models/label_builder.dart';
@@ -7,8 +8,12 @@ import 'package:neon/src/settings/models/options_category.dart';
 import 'package:neon/src/settings/models/storage.dart';
 import 'package:rxdart/rxdart.dart';
 
-@internal
+/// Listenable option that is persisted in the [SettingsStorage].
-abstract class Option<T> extends ChangeNotifier implements ValueListenable<T> {
+///
+/// See:
+///   * [ToggleOption] for an Option<bool>
+///   * [SelectOption] for an Option with multiple values
+sealed class Option<T> extends ChangeNotifier implements ValueListenable<T> {
   /// Creates an Option
   Option({
     required this.storage,
@@ -37,10 +42,23 @@ abstract class Option<T> extends ChangeNotifier implements ValueListenable<T> {
     });
   }
 
+  /// Storage to persist the state.
   final SettingsStorage storage;
+
+  /// Storage key to save the state at.
   final Storable key;
+
+  /// Label of the option.
   final LabelBuilder label;
+
+  /// Default value of the option.
+  ///
+  /// [reset] will restore this value.
   final T defaultValue;
+
+  /// Category of this option.
+  ///
+  /// This can be used to group multiple options
   final OptionsCategory? category;
 
   T _value;
@@ -122,3 +140,150 @@ abstract class Option<T> extends ChangeNotifier implements ValueListenable<T> {
     super.dispose();
   }
 }
+
+/// [Option] with multiple available values.
+///
+/// See:
+///   * [SelectOption] for an Option with multiple values
+
+class SelectOption<T> extends Option<T> {
+  /// Creates a SelectOption
+  SelectOption({
+    required super.storage,
+    required super.key,
+    required super.label,
+    required super.defaultValue,
+    required final Map<T, LabelBuilder> values,
+
+    /// Force loading the stored value.
+    ///
+    /// This is needed when [values] is empty but the stored value should still be loaded.
+    /// This only works when [T] is of type String?.
+    final bool forceLoadValue = true,
+    super.category,
+    super.enabled,
+  })  : _values = values,
+        super(initialValue: _loadValue(values, storage.getString(key.value), forceLoad: forceLoadValue));
+
+  /// Creates a SelectOption depending on the State of another [Option].
+  SelectOption.depend({
+    required super.storage,
+    required super.key,
+    required super.label,
+    required super.defaultValue,
+    required final Map<T, LabelBuilder> values,
+    required super.enabled,
+
+    /// Force loading the stored value.
+    ///
+    /// This is needed when [values] is empty but the stored value should still be loaded.
+    /// This only works when [T] is of type String?.
+    final bool forceLoadValue = true,
+    super.category,
+  })  : _values = values,
+        super.depend(initialValue: _loadValue(values, storage.getString(key.value), forceLoad: forceLoadValue));
+
+  static T? _loadValue<T>(final Map<T, LabelBuilder> vs, final String? stored, {final bool forceLoad = true}) {
+    if (forceLoad && vs.isEmpty && stored is T) {
+      return stored as T;
+    }
+
+    return _deserialize(vs, stored);
+  }
+
+  @override
+  void reset() {
+    unawaited(storage.remove(key.value));
+
+    super.reset();
+  }
+
+  Map<T, LabelBuilder> _values;
+
+  @override
+  set value(final T value) {
+    super.value = value;
+
+    if (value != null) {
+      unawaited(storage.setString(key.value, serialize()!));
+    }
+  }
+
+  /// A collection of different values this can have.
+  ///
+  /// See:
+  /// * [value] for the currently selected one
+  Map<T, LabelBuilder> get values => _values;
+
+  set values(final Map<T, LabelBuilder> newValues) {
+    if (_values == newValues) {
+      return;
+    }
+    _values = newValues;
+    notifyListeners();
+  }
+
+  @override
+  String? serialize() => _serialize(value);
+
+  static String? _serialize<T>(final T value) => value?.toString();
+
+  @override
+  T? deserialize(final Object? data) => _deserialize(_values, data as String?);
+
+  static T? _deserialize<T>(final Map<T, LabelBuilder> vs, final String? valueStr) {
+    if (valueStr == null) {
+      return null;
+    }
+
+    return vs.keys.firstWhereOrNull((final e) => _serialize(e) == valueStr);
+  }
+}
+
+/// [Option] with a boolean value.
+///
+/// See:
+///   * [SelectOption] for an Option with multiple values
+class ToggleOption extends Option<bool> {
+  /// Creates a ToggleOption
+  ToggleOption({
+    required super.storage,
+    required super.key,
+    required super.label,
+    required final bool defaultValue,
+    super.category,
+    super.enabled,
+  }) : super(defaultValue: storage.getBool(key.value) ?? defaultValue);
+
+  /// Creates a ToggleOption depending on the State of another [Option].
+  ToggleOption.depend({
+    required super.storage,
+    required super.key,
+    required super.label,
+    required final bool defaultValue,
+    required super.enabled,
+    super.category,
+  }) : super.depend(
+          defaultValue: storage.getBool(key.value) ?? defaultValue,
+        );
+
+  @override
+  void reset() {
+    unawaited(storage.remove(key.value));
+
+    super.reset();
+  }
+
+  @override
+  set value(final bool value) {
+    super.value = value;
+
+    unawaited(storage.setBool(key.value, serialize()));
+  }
+
+  @override
+  bool serialize() => value;
+
+  @override
+  bool? deserialize(final Object? data) => data as bool?;
+}

packages/neon/neon/lib/src/settings/models/select_option.dart

@@ -1,99 +0,0 @@
-import 'dart:async';
-
-import 'package:collection/collection.dart';
-import 'package:neon/src/models/label_builder.dart';
-import 'package:neon/src/settings/models/option.dart';
-
-class SelectOption<T> extends Option<T> {
-  /// Creates a SelectOption
-  SelectOption({
-    required super.storage,
-    required super.key,
-    required super.label,
-    required super.defaultValue,
-    required final Map<T, LabelBuilder> values,
-
-    /// Force loading the stored value.
-    ///
-    /// This is needed when [values] is empty but the stored value should still be loaded.
-    /// This only works when [T] is of type String?.
-    final bool forceLoadValue = true,
-    super.category,
-    super.enabled,
-  })  : _values = values,
-        super(initialValue: loadValue(values, storage.getString(key.value), forceLoad: forceLoadValue));
-
-  /// Creates a SelectOption depending on the State of another [Option].
-  SelectOption.depend({
-    required super.storage,
-    required super.key,
-    required super.label,
-    required super.defaultValue,
-    required final Map<T, LabelBuilder> values,
-    required super.enabled,
-
-    /// Force loading the stored value.
-    ///
-    /// This is needed when [values] is empty but the stored value should still be loaded.
-    /// This only works when [T] is of type String?.
-    final bool forceLoadValue = true,
-    super.category,
-  })  : _values = values,
-        super.depend(initialValue: loadValue(values, storage.getString(key.value), forceLoad: forceLoadValue));
-
-  static T? loadValue<T>(final Map<T, LabelBuilder> vs, final String? stored, {final bool forceLoad = true}) {
-    if (forceLoad && vs.isEmpty && stored is T) {
-      return stored as T;
-    }
-
-    return _deserialize(vs, stored);
-  }
-
-  @override
-  void reset() {
-    unawaited(storage.remove(key.value));
-
-    super.reset();
-  }
-
-  Map<T, LabelBuilder> _values;
-
-  @override
-  set value(final T value) {
-    super.value = value;
-
-    if (value != null) {
-      unawaited(storage.setString(key.value, serialize()!));
-    }
-  }
-
-  /// A collection of different values this can have.
-  ///
-  /// See:
-  /// * [value] for the currently selected one
-  Map<T, LabelBuilder> get values => _values;
-
-  set values(final Map<T, LabelBuilder> newValues) {
-    if (_values == newValues) {
-      return;
-    }
-    _values = newValues;
-    notifyListeners();
-  }
-
-  @override
-  String? serialize() => _serialize(value);
-
-  static String? _serialize<T>(final T value) => value?.toString();
-
-  @override
-  T? deserialize(final Object? data) => _deserialize(_values, data as String?);
-
-  static T? _deserialize<T>(final Map<T, LabelBuilder> vs, final String? valueStr) {
-    if (valueStr == null) {
-      return null;
-    }
-
-    return vs.keys.firstWhereOrNull((final e) => _serialize(e) == valueStr);
-  }
-}

packages/neon/neon/lib/src/settings/models/toggle_option.dart

@@ -1,47 +0,0 @@
-import 'dart:async';
-
-import 'package:neon/src/settings/models/option.dart';
-
-class ToggleOption extends Option<bool> {
-  /// Creates a ToggleOption
-  ToggleOption({
-    required super.storage,
-    required super.key,
-    required super.label,
-    required final bool defaultValue,
-    super.category,
-    super.enabled,
-  }) : super(defaultValue: storage.getBool(key.value) ?? defaultValue);
-
-  /// Creates a ToggleOption depending on the State of another [Option].
-  ToggleOption.depend({
-    required super.storage,
-    required super.key,
-    required super.label,
-    required final bool defaultValue,
-    required super.enabled,
-    super.category,
-  }) : super.depend(
-          defaultValue: storage.getBool(key.value) ?? defaultValue,
-        );
-
-  @override
-  void reset() {
-    unawaited(storage.remove(key.value));
-
-    super.reset();
-  }
-
-  @override
-  set value(final bool value) {
-    super.value = value;
-
-    unawaited(storage.setBool(key.value, serialize()));
-  }
-
-  @override
-  bool serialize() => value;
-
-  @override
-  bool? deserialize(final Object? data) => data as bool?;
-}

packages/neon/neon/lib/src/settings/widgets/checkbox_settings_tile.dart

@@ -1,6 +1,6 @@
 import 'package:flutter/material.dart';
 import 'package:meta/meta.dart';
-import 'package:neon/src/settings/models/toggle_option.dart';
+import 'package:neon/src/settings/models/option.dart';
 import 'package:neon/src/settings/widgets/settings_tile.dart';
 
 @internal

packages/neon/neon/lib/src/settings/widgets/dropdown_button_settings_tile.dart

@@ -1,6 +1,6 @@
 import 'package:flutter/material.dart';
 import 'package:meta/meta.dart';
-import 'package:neon/src/settings/models/select_option.dart';
+import 'package:neon/src/settings/models/option.dart';
 import 'package:neon/src/settings/widgets/settings_tile.dart';
 
 @internal

packages/neon/neon/lib/src/settings/widgets/option_settings_tile.dart

@@ -0,0 +1,20 @@
+import 'package:flutter/widgets.dart';
+import 'package:meta/meta.dart';
+import 'package:neon/settings.dart';
+import 'package:neon/src/settings/widgets/checkbox_settings_tile.dart';
+import 'package:neon/src/settings/widgets/dropdown_button_settings_tile.dart';
+import 'package:neon/src/settings/widgets/settings_tile.dart';
+
+@internal
+class OptionSettingsTile extends InputSettingsTile {
+  const OptionSettingsTile({
+    required super.option,
+    super.key,
+  });
+
+  @override
+  Widget build(final BuildContext context) => switch (option) {
+        ToggleOption() => CheckBoxSettingsTile(option: option as ToggleOption),
+        SelectOption() => DropdownButtonSettingsTile(option: option as SelectOption),
+      };
+}

packages/neon/neon/lib/src/settings/widgets/settings_category.dart

@@ -1,4 +1,4 @@
-import 'package:flutter/widgets.dart';
+import 'package:flutter/material.dart';
 import 'package:intersperse/intersperse.dart';
 import 'package:meta/meta.dart';
 import 'package:neon/src/settings/widgets/settings_tile.dart';
@@ -15,12 +15,19 @@ class SettingsCategory extends StatelessWidget {
   final List<SettingsTile> tiles;
 
   @override
-  Widget build(final BuildContext context) => Column(
+  Widget build(final BuildContext context) {
+    final textTheme = Theme.of(context).textTheme;
+
+    return Column(
       crossAxisAlignment: CrossAxisAlignment.start,
       children: [
-          if (title != null) ...[
+        if (title != null)
-            title!,
+          DefaultTextStyle(
-          ],
+            style: textTheme.titleMedium!.copyWith(
+              fontWeight: FontWeight.bold,
+            ),
+            child: title!,
+          ),
         ...tiles,
       ]
           .intersperse(
@@ -30,4 +37,5 @@ class SettingsCategory extends StatelessWidget {
           )
           .toList(),
     );
+  }
 }

packages/neon/neon/lib/src/sort_box/sort_box_builder.dart

@@ -1,5 +1,5 @@
 import 'package:flutter/widgets.dart';
-import 'package:neon/src/settings/models/select_option.dart';
+import 'package:neon/src/settings/models/option.dart';
 import 'package:sort_box/sort_box.dart';
 
 /// Signature for a function that creates a widget for a given sorted list.

packages/neon/neon/lib/src/utils/account_options.dart

@@ -3,7 +3,6 @@ import 'package:neon/l10n/localizations.dart';
 import 'package:neon/src/blocs/apps.dart';
 import 'package:neon/src/settings/models/option.dart';
 import 'package:neon/src/settings/models/options_collection.dart';
-import 'package:neon/src/settings/models/select_option.dart';
 import 'package:neon/src/settings/models/storage.dart';
 
 @internal

packages/neon/neon/lib/src/utils/global_options.dart

@@ -7,9 +7,7 @@ import 'package:neon/src/models/account.dart';
 import 'package:neon/src/models/label_builder.dart';
 import 'package:neon/src/settings/models/option.dart';
 import 'package:neon/src/settings/models/options_collection.dart';
-import 'package:neon/src/settings/models/select_option.dart';
 import 'package:neon/src/settings/models/storage.dart';
-import 'package:neon/src/settings/models/toggle_option.dart';
 import 'package:package_info_plus/package_info_plus.dart';
 import 'package:permission_handler/permission_handler.dart';
 

packages/neon/neon/test/option_test.dart

@@ -4,9 +4,8 @@ import 'dart:async';
 
 import 'package:flutter/foundation.dart';
 import 'package:mocktail/mocktail.dart';
-import 'package:neon/src/settings/models/select_option.dart';
+import 'package:neon/src/settings/models/option.dart';
 import 'package:neon/src/settings/models/storage.dart';
-import 'package:neon/src/settings/models/toggle_option.dart';
 import 'package:test/test.dart';
 
 class MockStorage extends Mock implements SettingsStorage {}

packages/neon/neon/test/options_collection_test.dart

@@ -1,11 +1,10 @@
 import 'package:mocktail/mocktail.dart';
 import 'package:neon/settings.dart';
-import 'package:neon/src/settings/models/option.dart';
 import 'package:neon/src/settings/models/storage.dart';
 import 'package:test/test.dart';
 
 // ignore: missing_override_of_must_be_overridden
-class OptionMock extends Mock implements Option<Object> {}
+class OptionMock extends Mock implements ToggleOption {}
 
 class Collection extends NextcloudAppOptions {
   Collection(final List<Option<Object>> options) : super(const AppStorage(StorageKeys.apps)) {
@@ -48,15 +47,15 @@ void main() {
 
     test('export', () {
       when(() => option1.key).thenReturn(Keys.key1);
-      when(option1.serialize).thenReturn('value1');
+      when(option1.serialize).thenReturn(true);
       when(() => option1.enabled).thenReturn(true);
 
       when(() => option2.key).thenReturn(Keys.key2);
-      when(option2.serialize).thenReturn('value2');
+      when(option2.serialize).thenReturn(true);
       when(() => option2.enabled).thenReturn(false);
 
       const json = {
-        'app': {'key1': 'value1'},
+        'app': {'key1': true},
       };
 
       final export = collection.export();
@@ -70,14 +69,14 @@ void main() {
 
       const json = {
         'app': {
-          'key1': 'value1',
+          'key1': false,
           'key2': null,
         },
       };
 
       collection.import(json);
 
-      verify(() => option1.load('value1')).called(1);
+      verify(() => option1.load(false)).called(1);
       verify(option2.reset).called(1);
     });
   });