Skip to content

Commit 9201ae4

Browse files
committed
settings [nfc]: Give settings specific setter methods
This makes the call sites significantly nicer to read. Relatedly, it turns the specifics of how the settings are represented in the database into more of an implementation detail of the settings store.
1 parent 1cf31a4 commit 9201ae4

File tree

7 files changed

+29
-34
lines changed

7 files changed

+29
-34
lines changed

Diff for: lib/model/settings.dart

+15-2
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,16 @@ class GlobalSettingsStore extends ChangeNotifier {
6262

6363
/// The user's choice of [ThemeSetting];
6464
/// null means the device-level choice of theme.
65+
///
66+
/// See also [setThemeSetting].
6567
ThemeSetting? get themeSetting => _data.themeSetting;
6668

6769
/// The user's choice of [BrowserPreference];
6870
/// null means use our default choice.
6971
///
7072
/// Consider using [effectiveBrowserPreference] or [getUrlLaunchMode].
73+
///
74+
/// See also [setBrowserPreference].
7175
BrowserPreference? get browserPreference => _data.browserPreference;
7276

7377
/// The value of [BrowserPreference] to use:
@@ -110,10 +114,19 @@ class GlobalSettingsStore extends ChangeNotifier {
110114
}
111115
}
112116

113-
/// Update the global settings in the store.
114-
Future<void> update(GlobalSettingsCompanion data) async {
117+
Future<void> _update(GlobalSettingsCompanion data) async {
115118
await _backend.doUpdateGlobalSettings(data);
116119
_data = _data.copyWithCompanion(data);
117120
notifyListeners();
118121
}
122+
123+
/// Set [themeSetting], persistently for future runs of the app.
124+
Future<void> setThemeSetting(ThemeSetting? value) async {
125+
await _update(GlobalSettingsCompanion(themeSetting: Value(value)));
126+
}
127+
128+
/// Set [browserPreference], persistently for future runs of the app.
129+
Future<void> setBrowserPreference(BrowserPreference? value) async {
130+
await _update(GlobalSettingsCompanion(browserPreference: Value(value)));
131+
}
119132
}

Diff for: lib/widgets/settings.dart

+3-6
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1-
import 'package:drift/drift.dart' hide Column;
21
import 'package:flutter/material.dart';
32

43
import '../generated/l10n/zulip_localizations.dart';
5-
import '../model/database.dart';
64
import '../model/settings.dart';
75
import 'app_bar.dart';
86
import 'page.dart';
@@ -34,8 +32,7 @@ class _ThemeSetting extends StatelessWidget {
3432

3533
void _handleChange(BuildContext context, ThemeSetting? newThemeSetting) {
3634
final globalSettings = GlobalStoreWidget.settingsOf(context);
37-
globalSettings.update(
38-
GlobalSettingsCompanion(themeSetting: Value(newThemeSetting)));
35+
globalSettings.setThemeSetting(newThemeSetting);
3936
}
4037

4138
@override
@@ -62,9 +59,9 @@ class _BrowserPreferenceSetting extends StatelessWidget {
6259

6360
void _handleChange(BuildContext context, bool newOpenLinksWithInAppBrowser) {
6461
final globalSettings = GlobalStoreWidget.settingsOf(context);
65-
globalSettings.update(GlobalSettingsCompanion(browserPreference: Value(
62+
globalSettings.setBrowserPreference(
6663
newOpenLinksWithInAppBrowser ? BrowserPreference.inApp
67-
: BrowserPreference.external)));
64+
: BrowserPreference.external);
6865
}
6966

7067
@override

Diff for: test/model/store_test.dart

+2-5
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import 'package:zulip/api/route/messages.dart';
1515
import 'package:zulip/api/route/realm.dart';
1616
import 'package:zulip/log.dart';
1717
import 'package:zulip/model/actions.dart';
18-
import 'package:zulip/model/database.dart';
1918
import 'package:zulip/model/settings.dart';
2019
import 'package:zulip/model/store.dart';
2120
import 'package:zulip/notifications/receive.dart';
@@ -37,8 +36,7 @@ void main() {
3736
final globalStore = eg.globalStore();
3837
check(globalStore).settings.themeSetting.equals(null);
3938

40-
await globalStore.settings.update(
41-
GlobalSettingsCompanion(themeSetting: Value(ThemeSetting.dark)));
39+
await globalStore.settings.setThemeSetting(ThemeSetting.dark);
4240
check(globalStore).settings.themeSetting.equals(ThemeSetting.dark);
4341
});
4442

@@ -48,8 +46,7 @@ void main() {
4846
globalStore.settings.addListener(() => notifyCount++);
4947
check(notifyCount).equals(0);
5048

51-
await globalStore.settings.update(
52-
GlobalSettingsCompanion(themeSetting: Value(ThemeSetting.light)));
49+
await globalStore.settings.setThemeSetting(ThemeSetting.light);
5350
check(notifyCount).equals(1);
5451
});
5552

Diff for: test/widgets/content_test.dart

+2-4
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import 'package:flutter_test/flutter_test.dart';
88
import 'package:url_launcher/url_launcher.dart';
99
import 'package:zulip/api/core.dart';
1010
import 'package:zulip/model/content.dart';
11-
import 'package:zulip/model/database.dart';
1211
import 'package:zulip/model/narrow.dart';
1312
import 'package:zulip/model/settings.dart';
1413
import 'package:zulip/model/store.dart';
@@ -801,9 +800,8 @@ void main() {
801800
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
802801

803802
testWidgets('follow browser preference setting to open URL', (tester) async {
804-
await testBinding.globalStore.settings.update(
805-
GlobalSettingsData(
806-
browserPreference: BrowserPreference.inApp).toCompanion(false));
803+
await testBinding.globalStore.settings
804+
.setBrowserPreference(BrowserPreference.inApp);
807805
await prepare(tester,
808806
'<p><a href="https://example/">hello</a></p>');
809807

Diff for: test/widgets/settings_test.dart

+3-6
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import 'package:checks/checks.dart';
22
import 'package:flutter/foundation.dart';
33
import 'package:flutter/material.dart';
44
import 'package:flutter_test/flutter_test.dart';
5-
import 'package:zulip/model/database.dart';
65
import 'package:zulip/model/settings.dart';
76
import 'package:zulip/widgets/settings.dart';
87

@@ -51,8 +50,7 @@ void main() {
5150
testWidgets('smoke', (tester) async {
5251
debugBrightnessOverride = Brightness.light;
5352

54-
await testBinding.globalStore.settings.update(
55-
GlobalSettingsData(themeSetting: ThemeSetting.light).toCompanion(false));
53+
await testBinding.globalStore.settings.setThemeSetting(ThemeSetting.light);
5654
await prepare(tester);
5755
final element = tester.element(find.byType(SettingsPage));
5856
check(Theme.of(element)).brightness.equals(Brightness.light);
@@ -101,9 +99,8 @@ void main() {
10199
}
102100

103101
testWidgets('smoke', (tester) async {
104-
await testBinding.globalStore.settings.update(
105-
GlobalSettingsData(
106-
browserPreference: BrowserPreference.external).toCompanion(false));
102+
await testBinding.globalStore.settings
103+
.setBrowserPreference(BrowserPreference.external);
107104
await prepare(tester);
108105
checkSwitchAndGlobalSettings(tester,
109106
checked: false, expectedBrowserPreference: BrowserPreference.external);

Diff for: test/widgets/store_test.dart

+2-5
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import 'package:flutter/material.dart';
55
import 'package:flutter_checks/flutter_checks.dart';
66
import 'package:flutter_test/flutter_test.dart';
77
import 'package:zulip/model/actions.dart';
8-
import 'package:zulip/model/database.dart';
98
import 'package:zulip/model/settings.dart';
109
import 'package:zulip/model/store.dart';
1110
import 'package:zulip/widgets/app.dart';
@@ -106,8 +105,7 @@ void main() {
106105

107106
testWidgets('GlobalStoreWidget.settingsOf updates on settings update', (tester) async {
108107
addTearDown(testBinding.reset);
109-
await testBinding.globalStore.settings.update(
110-
GlobalSettingsCompanion(themeSetting: Value(ThemeSetting.dark)));
108+
await testBinding.globalStore.settings.setThemeSetting(ThemeSetting.dark);
111109

112110
ThemeSetting? themeSetting;
113111
await tester.pumpWidget(
@@ -120,8 +118,7 @@ void main() {
120118
await tester.pump();
121119
check(themeSetting).equals(ThemeSetting.dark);
122120

123-
await testBinding.globalStore.settings.update(
124-
GlobalSettingsCompanion(themeSetting: Value(ThemeSetting.light)));
121+
await testBinding.globalStore.settings.setThemeSetting(ThemeSetting.light);
125122
await tester.pump();
126123
check(themeSetting).equals(ThemeSetting.light);
127124
});

Diff for: test/widgets/theme_test.dart

+2-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
import 'package:checks/checks.dart';
2-
import 'package:drift/drift.dart';
32
import 'package:flutter/material.dart';
43
import 'package:flutter/rendering.dart';
54
import 'package:flutter_test/flutter_test.dart';
6-
import 'package:zulip/model/database.dart';
75
import 'package:zulip/model/settings.dart';
86
import 'package:zulip/widgets/channel_colors.dart';
97
import 'package:zulip/widgets/text.dart';
@@ -134,12 +132,10 @@ void main() {
134132
final element = tester.element(find.byType(Placeholder));
135133
check(zulipThemeData(element)).brightness.equals(Brightness.light);
136134

137-
await testBinding.globalStore.settings.update(
138-
const GlobalSettingsCompanion(themeSetting: Value(ThemeSetting.dark)));
135+
await testBinding.globalStore.settings.setThemeSetting(ThemeSetting.dark);
139136
check(zulipThemeData(element)).brightness.equals(Brightness.dark);
140137

141-
await testBinding.globalStore.settings.update(
142-
const GlobalSettingsCompanion(themeSetting: Value(null)));
138+
await testBinding.globalStore.settings.setThemeSetting(null);
143139
check(zulipThemeData(element)).brightness.equals(Brightness.light);
144140
});
145141

0 commit comments

Comments
 (0)