From 92b6c4bf0a04cc42049172bf5b9a9f5169107c91 Mon Sep 17 00:00:00 2001 From: Nikolas Rimikis Date: Fri, 8 Sep 2023 17:31:45 +0200 Subject: [PATCH 1/2] refactor(neon): seal option and OptionSettingsTile Signed-off-by: Nikolas Rimikis --- packages/neon/neon/lib/settings.dart | 3 +- .../lib/src/pages/nextcloud_app_settings.dart | 15 +- .../neon/lib/src/settings/models/option.dart | 169 +++++++++++++++++- .../src/settings/models/select_option.dart | 99 ---------- .../src/settings/models/toggle_option.dart | 47 ----- .../widgets/checkbox_settings_tile.dart | 2 +- .../dropdown_button_settings_tile.dart | 2 +- .../widgets/option_settings_tile.dart | 20 +++ .../lib/src/sort_box/sort_box_builder.dart | 2 +- .../neon/lib/src/utils/account_options.dart | 1 - .../neon/lib/src/utils/global_options.dart | 2 - packages/neon/neon/test/option_test.dart | 3 +- .../neon/test/options_collection_test.dart | 13 +- 13 files changed, 200 insertions(+), 178 deletions(-) delete mode 100644 packages/neon/neon/lib/src/settings/models/select_option.dart delete mode 100644 packages/neon/neon/lib/src/settings/models/toggle_option.dart create mode 100644 packages/neon/neon/lib/src/settings/widgets/option_settings_tile.dart diff --git a/packages/neon/neon/lib/settings.dart b/packages/neon/neon/lib/settings.dart index 08d29b5c..a8697491 100644 --- a/packages/neon/neon/lib/settings.dart +++ b/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'; diff --git a/packages/neon/neon/lib/src/pages/nextcloud_app_settings.dart b/packages/neon/neon/lib/src/pages/nextcloud_app_settings.dart index f3a2d4cf..9ec070c4 100644 --- a/packages/neon/neon/lib/src/pages/nextcloud_app_settings.dart +++ b/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/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/option_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) ...[ - CheckBoxSettingsTile( - option: option, - ), - ] else if (option is SelectOption) ...[ - DropdownButtonSettingsTile( - option: option, - ), - ], + OptionSettingsTile(option: option), ], ], ), diff --git a/packages/neon/neon/lib/src/settings/models/option.dart b/packages/neon/neon/lib/src/settings/models/option.dart index d6433c25..53f05231 100644 --- a/packages/neon/neon/lib/src/settings/models/option.dart +++ b/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 -abstract class Option extends ChangeNotifier implements ValueListenable { +/// Listenable option that is persisted in the [SettingsStorage]. +/// +/// See: +/// * [ToggleOption] for an Option +/// * [SelectOption] for an Option with multiple values +sealed class Option extends ChangeNotifier implements ValueListenable { /// Creates an Option Option({ required this.storage, @@ -37,10 +42,23 @@ abstract class Option extends ChangeNotifier implements ValueListenable { }); } + /// 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 extends ChangeNotifier implements ValueListenable { super.dispose(); } } + +/// [Option] with multiple available values. +/// +/// See: +/// * [SelectOption] for an Option with multiple values + +class SelectOption extends Option { + /// Creates a SelectOption + SelectOption({ + required super.storage, + required super.key, + required super.label, + required super.defaultValue, + required final Map 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 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(final Map 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 _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 get values => _values; + + set values(final Map newValues) { + if (_values == newValues) { + return; + } + _values = newValues; + notifyListeners(); + } + + @override + String? serialize() => _serialize(value); + + static String? _serialize(final T value) => value?.toString(); + + @override + T? deserialize(final Object? data) => _deserialize(_values, data as String?); + + static T? _deserialize(final Map 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 { + /// 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?; +} diff --git a/packages/neon/neon/lib/src/settings/models/select_option.dart b/packages/neon/neon/lib/src/settings/models/select_option.dart deleted file mode 100644 index 6139cbe5..00000000 --- a/packages/neon/neon/lib/src/settings/models/select_option.dart +++ /dev/null @@ -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 extends Option { - /// Creates a SelectOption - SelectOption({ - required super.storage, - required super.key, - required super.label, - required super.defaultValue, - required final Map 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 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(final Map 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 _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 get values => _values; - - set values(final Map newValues) { - if (_values == newValues) { - return; - } - _values = newValues; - notifyListeners(); - } - - @override - String? serialize() => _serialize(value); - - static String? _serialize(final T value) => value?.toString(); - - @override - T? deserialize(final Object? data) => _deserialize(_values, data as String?); - - static T? _deserialize(final Map vs, final String? valueStr) { - if (valueStr == null) { - return null; - } - - return vs.keys.firstWhereOrNull((final e) => _serialize(e) == valueStr); - } -} diff --git a/packages/neon/neon/lib/src/settings/models/toggle_option.dart b/packages/neon/neon/lib/src/settings/models/toggle_option.dart deleted file mode 100644 index 9b3ae94b..00000000 --- a/packages/neon/neon/lib/src/settings/models/toggle_option.dart +++ /dev/null @@ -1,47 +0,0 @@ -import 'dart:async'; - -import 'package:neon/src/settings/models/option.dart'; - -class ToggleOption extends Option { - /// 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?; -} diff --git a/packages/neon/neon/lib/src/settings/widgets/checkbox_settings_tile.dart b/packages/neon/neon/lib/src/settings/widgets/checkbox_settings_tile.dart index 3d2941f3..e868a3ab 100644 --- a/packages/neon/neon/lib/src/settings/widgets/checkbox_settings_tile.dart +++ b/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 diff --git a/packages/neon/neon/lib/src/settings/widgets/dropdown_button_settings_tile.dart b/packages/neon/neon/lib/src/settings/widgets/dropdown_button_settings_tile.dart index 1157a9d9..5df717cf 100644 --- a/packages/neon/neon/lib/src/settings/widgets/dropdown_button_settings_tile.dart +++ b/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 diff --git a/packages/neon/neon/lib/src/settings/widgets/option_settings_tile.dart b/packages/neon/neon/lib/src/settings/widgets/option_settings_tile.dart new file mode 100644 index 00000000..8757c630 --- /dev/null +++ b/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), + }; +} diff --git a/packages/neon/neon/lib/src/sort_box/sort_box_builder.dart b/packages/neon/neon/lib/src/sort_box/sort_box_builder.dart index afa7227b..35803216 100644 --- a/packages/neon/neon/lib/src/sort_box/sort_box_builder.dart +++ b/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. diff --git a/packages/neon/neon/lib/src/utils/account_options.dart b/packages/neon/neon/lib/src/utils/account_options.dart index f89bc393..6e71534f 100644 --- a/packages/neon/neon/lib/src/utils/account_options.dart +++ b/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 diff --git a/packages/neon/neon/lib/src/utils/global_options.dart b/packages/neon/neon/lib/src/utils/global_options.dart index 0979608c..69285960 100644 --- a/packages/neon/neon/lib/src/utils/global_options.dart +++ b/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'; diff --git a/packages/neon/neon/test/option_test.dart b/packages/neon/neon/test/option_test.dart index 962ce645..6e5268e4 100644 --- a/packages/neon/neon/test/option_test.dart +++ b/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 {} diff --git a/packages/neon/neon/test/options_collection_test.dart b/packages/neon/neon/test/options_collection_test.dart index f97f42e2..92e157c0 100644 --- a/packages/neon/neon/test/options_collection_test.dart +++ b/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 {} +class OptionMock extends Mock implements ToggleOption {} class Collection extends NextcloudAppOptions { Collection(final List> 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); }); }); From ad3e0ba6b6a3396d32ebe5be24b241d57019c657 Mon Sep 17 00:00:00 2001 From: Nikolas Rimikis Date: Fri, 8 Sep 2023 18:46:17 +0200 Subject: [PATCH 2/2] feat(neon): better emphazize SettingsCategory.title Signed-off-by: Nikolas Rimikis --- .../settings/widgets/settings_category.dart | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/packages/neon/neon/lib/src/settings/widgets/settings_category.dart b/packages/neon/neon/lib/src/settings/widgets/settings_category.dart index 674d3432..fd2b1374 100644 --- a/packages/neon/neon/lib/src/settings/widgets/settings_category.dart +++ b/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,19 +15,27 @@ class SettingsCategory extends StatelessWidget { final List tiles; @override - Widget build(final BuildContext context) => Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - if (title != null) ...[ - title!, - ], - ...tiles, - ] - .intersperse( - const SizedBox( - height: 10, - ), - ) - .toList(), - ); + Widget build(final BuildContext context) { + final textTheme = Theme.of(context).textTheme; + + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + if (title != null) + DefaultTextStyle( + style: textTheme.titleMedium!.copyWith( + fontWeight: FontWeight.bold, + ), + child: title!, + ), + ...tiles, + ] + .intersperse( + const SizedBox( + height: 10, + ), + ) + .toList(), + ); + } }