Skip to content

Commit 8ed6915

Browse files
committed
settings [nfc]: Split out a GlobalSettingsStore, and GlobalStoreWidget.settingsOf
This allows widgets to listen separately for changes to settings or to the rest of the global store's data, which is a bit nice. More fundamentally, as a matter of code organization, it gives us a place to put code related to the store of global settings where there's more room to spread out than if we kept it in GlobalStore, and where the namespace is already specific to global settings. Those two points in turn will also help us adjust to be more ergonomic the API the rest of the app uses for interacting with these settings. The TODO comments added in this commit mark places where this API will be more coherent after upcoming commits in this series.
1 parent 3ab3296 commit 8ed6915

File tree

8 files changed

+95
-11
lines changed

8 files changed

+95
-11
lines changed

lib/model/settings.dart

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,18 @@ extension GlobalSettingsHelpers on GlobalSettingsData {
8585
}
8686
}
8787
}
88+
89+
/// Store for the user's account-independent settings.
90+
///
91+
/// From UI code, use [GlobalStoreWidget.settingsOf] to get hold of
92+
/// the settings data.
93+
///
94+
/// (At the moment the actual settings data lives on [GlobalStore];
95+
/// but when the settings change, the notification goes to listeners
96+
/// of this class, not [GlobalStore]. Soon the actual data will
97+
/// move to this class too.)
98+
class GlobalSettingsStore extends ChangeNotifier {
99+
// TODO move the actual settings content to this class
100+
101+
void markUpdated() => notifyListeners();
102+
}

lib/model/store.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import 'message_list.dart';
2929
import 'recent_dm_conversations.dart';
3030
import 'recent_senders.dart';
3131
import 'channel.dart';
32+
import 'settings.dart';
3233
import 'typing_status.dart';
3334
import 'unreads.dart';
3435
import 'user.dart';
@@ -60,15 +61,22 @@ abstract class GlobalStore extends ChangeNotifier {
6061
: _globalSettings = globalSettings,
6162
_accounts = Map.fromEntries(accounts.map((a) => MapEntry(a.id, a)));
6263

64+
// TODO use this as the actual store
65+
final GlobalSettingsStore settingsNotifier = GlobalSettingsStore();
66+
6367
/// A cache of the [GlobalSettingsData] singleton in the underlying data store.
68+
///
69+
/// To be notified for changes to this value, subscribe to [settingsNotifier]
70+
/// (usually by calling [GlobalStoreWidget.settingsOf]).
71+
/// The [GlobalStore] itself will not notify its own listeners.
6472
GlobalSettingsData get globalSettings => _globalSettings;
6573
GlobalSettingsData _globalSettings;
6674

6775
/// Update the global settings in the store.
6876
Future<void> updateGlobalSettings(GlobalSettingsCompanion data) async {
6977
await doUpdateGlobalSettings(data);
7078
_globalSettings = _globalSettings.copyWithCompanion(data);
71-
notifyListeners();
79+
settingsNotifier.markUpdated();
7280
}
7381

7482
/// Update the global settings in the underlying data store.

lib/widgets/content.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1439,7 +1439,7 @@ void _launchUrl(BuildContext context, String urlString) async {
14391439
return;
14401440
}
14411441

1442-
final globalSettings = GlobalStoreWidget.of(context).globalSettings;
1442+
final globalSettings = GlobalStoreWidget.settingsOf(context);
14431443
bool launched = false;
14441444
String? errorMessage;
14451445
try {

lib/widgets/settings.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class _ThemeSetting extends StatelessWidget {
4040
@override
4141
Widget build(BuildContext context) {
4242
final zulipLocalizations = ZulipLocalizations.of(context);
43-
final globalStore = GlobalStoreWidget.of(context);
43+
final globalSettings = GlobalStoreWidget.settingsOf(context);
4444
return Column(
4545
children: [
4646
ListTile(title: Text(zulipLocalizations.themeSettingTitle)),
@@ -50,7 +50,7 @@ class _ThemeSetting extends StatelessWidget {
5050
themeSetting: themeSettingOption,
5151
zulipLocalizations: zulipLocalizations)),
5252
value: themeSettingOption,
53-
groupValue: globalStore.globalSettings.themeSetting,
53+
groupValue: globalSettings.themeSetting,
5454
onChanged: (newValue) => _handleChange(context, newValue)),
5555
]);
5656
}
@@ -69,9 +69,9 @@ class _BrowserPreferenceSetting extends StatelessWidget {
6969
@override
7070
Widget build(BuildContext context) {
7171
final zulipLocalizations = ZulipLocalizations.of(context);
72+
final globalSettings = GlobalStoreWidget.settingsOf(context);
7273
final openLinksWithInAppBrowser =
73-
GlobalStoreWidget.of(context).globalSettings.effectiveBrowserPreference
74-
== BrowserPreference.inApp;
74+
globalSettings.effectiveBrowserPreference == BrowserPreference.inApp;
7575
return SwitchListTile.adaptive(
7676
title: Text(zulipLocalizations.openLinksWithInAppBrowser),
7777
value: openLinksWithInAppBrowser,

lib/widgets/store.dart

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import 'package:flutter/material.dart';
22
import 'package:flutter/scheduler.dart';
33

44
import '../model/binding.dart';
5+
import '../model/database.dart';
6+
import '../model/settings.dart';
57
import '../model/store.dart';
68
import 'page.dart';
79

@@ -51,6 +53,28 @@ class GlobalStoreWidget extends StatefulWidget {
5153
return widget!.store;
5254
}
5355

56+
/// The user's [GlobalSettings] data, from the app's global data store.
57+
///
58+
/// The given build context will be registered as a dependency and
59+
/// subscribed to changes in the [GlobalSettingsStore].
60+
/// This means that when the setting values in the store change,
61+
/// the element at that build context will be rebuilt.
62+
///
63+
/// This method is typically called near the top of a build method or a
64+
/// [State.didChangeDependencies] method, like so:
65+
/// ```
66+
/// @override
67+
/// Widget build(BuildContext context) {
68+
/// final globalSettings = GlobalStoreWidget.settingsOf(context);
69+
/// ```
70+
///
71+
/// See [of] for further discussion of how to use this kind of method.
72+
static GlobalSettingsData settingsOf(BuildContext context) {
73+
final widget = context.dependOnInheritedWidgetOfExactType<_GlobalSettingsStoreInheritedWidget>();
74+
assert(widget != null, 'No GlobalStoreWidget ancestor');
75+
return widget!.store.globalSettings;
76+
}
77+
5478
@override
5579
State<GlobalStoreWidget> createState() => _GlobalStoreWidgetState();
5680
}
@@ -81,14 +105,27 @@ class _GlobalStoreWidgetState extends State<GlobalStoreWidget> {
81105
// a [StatefulWidget] to get hold of the store, and an [InheritedWidget] to
82106
// provide it to descendants, and one widget can't be both of those.
83107
class _GlobalStoreInheritedWidget extends InheritedNotifier<GlobalStore> {
84-
const _GlobalStoreInheritedWidget({
108+
_GlobalStoreInheritedWidget({
85109
required GlobalStore store,
86-
required super.child,
87-
}) : super(notifier: store);
110+
required Widget child,
111+
}) : super(notifier: store,
112+
child: _GlobalSettingsStoreInheritedWidget(store: store, child: child));
88113

89114
GlobalStore get store => notifier!;
90115
}
91116

117+
// This is like [_GlobalStoreInheritedWidget] except it subscribes to the
118+
// [GlobalSettingsStore] instead of the overall [GlobalStore].
119+
// That enables [settingsOf] to do the same.
120+
class _GlobalSettingsStoreInheritedWidget extends InheritedNotifier<GlobalSettingsStore> {
121+
_GlobalSettingsStoreInheritedWidget({
122+
required this.store,
123+
required super.child,
124+
}) : super(notifier: store.settingsNotifier);
125+
126+
final GlobalStore store;
127+
}
128+
92129
/// Provides access to the user's data for a particular Zulip account.
93130
///
94131
/// Widgets that need information that comes from the Zulip server, or need to

lib/widgets/theme.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import 'text.dart';
1313
ThemeData zulipThemeData(BuildContext context) {
1414
final DesignVariables designVariables;
1515
final List<ThemeExtension> themeExtensions;
16-
final globalSettings = GlobalStoreWidget.of(context).globalSettings;
16+
final globalSettings = GlobalStoreWidget.settingsOf(context);
1717
Brightness brightness = switch (globalSettings.themeSetting) {
1818
null => MediaQuery.platformBrightnessOf(context),
1919
ThemeSetting.light => Brightness.light,

test/model/store_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void main() {
4545
test('should notify listeners', () async {
4646
int notifyCount = 0;
4747
final globalStore = eg.globalStore();
48-
globalStore.addListener(() => notifyCount++);
48+
globalStore.settingsNotifier.addListener(() => notifyCount++);
4949
check(notifyCount).equals(0);
5050

5151
await globalStore.updateGlobalSettings(

test/widgets/store_test.dart

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ 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';
9+
import 'package:zulip/model/settings.dart';
810
import 'package:zulip/model/store.dart';
911
import 'package:zulip/widgets/app.dart';
1012
import 'package:zulip/widgets/inbox.dart';
@@ -102,6 +104,28 @@ void main() {
102104
check(accountIds).isNotNull().deepEquals([eg.selfAccount.id]);
103105
});
104106

107+
testWidgets('GlobalStoreWidget.settingsOf updates on settings update', (tester) async {
108+
addTearDown(testBinding.reset);
109+
await testBinding.globalStore.updateGlobalSettings(
110+
GlobalSettingsCompanion(themeSetting: Value(ThemeSetting.dark)));
111+
112+
ThemeSetting? themeSetting;
113+
await tester.pumpWidget(
114+
GlobalStoreWidget(
115+
child: Builder(
116+
builder: (context) {
117+
themeSetting = GlobalStoreWidget.settingsOf(context).themeSetting;
118+
return const SizedBox.shrink();
119+
})));
120+
await tester.pump();
121+
check(themeSetting).equals(ThemeSetting.dark);
122+
123+
await testBinding.globalStore.updateGlobalSettings(
124+
GlobalSettingsCompanion(themeSetting: Value(ThemeSetting.light)));
125+
await tester.pump();
126+
check(themeSetting).equals(ThemeSetting.light);
127+
});
128+
105129
testWidgets('PerAccountStoreWidget basic', (tester) async {
106130
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
107131
addTearDown(testBinding.reset);

0 commit comments

Comments
 (0)