Skip to content

Commit cb3fa1d

Browse files
committed
settings: Add the concept of experimental feature flags
Fixes #1409.
1 parent 5cfd4ae commit cb3fa1d

12 files changed

+131
-0
lines changed

assets/l10n/app_en.arb

+8
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,14 @@
823823
"@pollWidgetOptionsMissing": {
824824
"description": "Text to display for a poll when it has no options"
825825
},
826+
"experimentalFeatureSettingsPageTitle": "Experimental features",
827+
"@experimentalFeatureSettingsPageTitle": {
828+
"description": "Title of settings page for experimental, in-development features"
829+
},
830+
"experimentalFeatureSettingsWarning": "These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.",
831+
"@experimentalFeatureSettingsWarning": {
832+
"description": "Warning text on settings page for experimental, in-development features"
833+
},
826834
"errorNotificationOpenTitle": "Failed to open notification",
827835
"@errorNotificationOpenTitle": {
828836
"description": "Error title when notification opening fails"

lib/generated/l10n/zulip_localizations.dart

+12
Original file line numberDiff line numberDiff line change
@@ -1203,6 +1203,18 @@ abstract class ZulipLocalizations {
12031203
/// **'This poll has no options yet.'**
12041204
String get pollWidgetOptionsMissing;
12051205

1206+
/// Title of settings page for experimental, in-development features
1207+
///
1208+
/// In en, this message translates to:
1209+
/// **'Experimental features'**
1210+
String get experimentalFeatureSettingsPageTitle;
1211+
1212+
/// Warning text on settings page for experimental, in-development features
1213+
///
1214+
/// In en, this message translates to:
1215+
/// **'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.'**
1216+
String get experimentalFeatureSettingsWarning;
1217+
12061218
/// Error title when notification opening fails
12071219
///
12081220
/// In en, this message translates to:

lib/generated/l10n/zulip_localizations_ar.dart

+6
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,12 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
643643
@override
644644
String get pollWidgetOptionsMissing => 'This poll has no options yet.';
645645

646+
@override
647+
String get experimentalFeatureSettingsPageTitle => 'Experimental features';
648+
649+
@override
650+
String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.';
651+
646652
@override
647653
String get errorNotificationOpenTitle => 'Failed to open notification';
648654

lib/generated/l10n/zulip_localizations_en.dart

+6
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,12 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
643643
@override
644644
String get pollWidgetOptionsMissing => 'This poll has no options yet.';
645645

646+
@override
647+
String get experimentalFeatureSettingsPageTitle => 'Experimental features';
648+
649+
@override
650+
String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.';
651+
646652
@override
647653
String get errorNotificationOpenTitle => 'Failed to open notification';
648654

lib/generated/l10n/zulip_localizations_ja.dart

+6
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,12 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
643643
@override
644644
String get pollWidgetOptionsMissing => 'This poll has no options yet.';
645645

646+
@override
647+
String get experimentalFeatureSettingsPageTitle => 'Experimental features';
648+
649+
@override
650+
String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.';
651+
646652
@override
647653
String get errorNotificationOpenTitle => 'Failed to open notification';
648654

lib/generated/l10n/zulip_localizations_nb.dart

+6
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,12 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
643643
@override
644644
String get pollWidgetOptionsMissing => 'This poll has no options yet.';
645645

646+
@override
647+
String get experimentalFeatureSettingsPageTitle => 'Experimental features';
648+
649+
@override
650+
String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.';
651+
646652
@override
647653
String get errorNotificationOpenTitle => 'Failed to open notification';
648654

lib/generated/l10n/zulip_localizations_pl.dart

+6
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,12 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
643643
@override
644644
String get pollWidgetOptionsMissing => 'Ta sonda nie ma opcji do wyboru.';
645645

646+
@override
647+
String get experimentalFeatureSettingsPageTitle => 'Experimental features';
648+
649+
@override
650+
String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.';
651+
646652
@override
647653
String get errorNotificationOpenTitle => 'Otwieranie powiadomienia bez powodzenia';
648654

lib/generated/l10n/zulip_localizations_ru.dart

+6
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,12 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
643643
@override
644644
String get pollWidgetOptionsMissing => 'В опросе пока нет вариантов ответа.';
645645

646+
@override
647+
String get experimentalFeatureSettingsPageTitle => 'Experimental features';
648+
649+
@override
650+
String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.';
651+
646652
@override
647653
String get errorNotificationOpenTitle => 'Не удалось открыть оповещения';
648654

lib/generated/l10n/zulip_localizations_sk.dart

+6
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,12 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
643643
@override
644644
String get pollWidgetOptionsMissing => 'This poll has no options yet.';
645645

646+
@override
647+
String get experimentalFeatureSettingsPageTitle => 'Experimental features';
648+
649+
@override
650+
String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.';
651+
646652
@override
647653
String get errorNotificationOpenTitle => 'Nepodarilo sa otvoriť oznámenie';
648654

lib/model/settings.dart

+29
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,27 @@ enum GlobalSettingType {
5858
/// (so that it stands ready to accept a future feature flag),
5959
/// we give it a placeholder value which isn't a real setting.
6060
placeholder,
61+
62+
/// Describes a setting which enables an in-progress feature of the app.
63+
///
64+
/// Sometimes when building a complex feature it's useful to merge PRs that
65+
/// make partial progress, and then to have the feature's logic gated behind
66+
/// a setting that serves as a "feature flag".
67+
/// This enables those working on the feature to enable the flag in order to
68+
/// see the current incomplete behavior, while for everyone else it remains
69+
/// disabled and so (barring bugs in the use of the flag itself) has no effect.
70+
///
71+
/// These settings are primarily meant for people developing Zulip to use,
72+
/// and so appear in an out-of-the-way part of the settings UI.
73+
///
74+
/// Settings of this kind are costly to the health of the codebase if
75+
/// allowed to accumulate. Most features don't need one, even features that
76+
/// take two or three PRs to implement. See discussion at:
77+
/// https://github.com/zulip/zulip-flutter/issues/1409#issuecomment-2725793787
78+
/// When a feature flag is introduced, take care to drive the project to
79+
/// completion, either by merge or removal, so that the flag can be retired
80+
/// within a period of a few weeks or months.
81+
experimentalFeatureFlag,
6182
;
6283
}
6384

@@ -83,6 +104,10 @@ enum GlobalSettingType {
83104
/// finish an experimental feature.)
84105
enum BoolGlobalSetting {
85106
/// A non-setting to ensure this enum has at least one value.
107+
///
108+
/// Leave this in place even when there are experimental feature flags too.
109+
/// That way when we remove those, this is already here.
110+
/// (Having one stable value in this enum is also handy for tests.)
86111
placeholderIgnore(GlobalSettingType.placeholder, false),
87112

88113
// Former settings which might exist in the database,
@@ -117,6 +142,10 @@ class GlobalSettingsStore extends ChangeNotifier {
117142
required Map<BoolGlobalSetting, bool> boolData,
118143
}) : _backend = backend, _data = data, _boolData = boolData;
119144

145+
static final List<BoolGlobalSetting> experimentalFeatureFlags =
146+
BoolGlobalSetting.values.where((setting) =>
147+
setting.type == GlobalSettingType.experimentalFeatureFlag).toList();
148+
120149
final GlobalStoreBackend _backend;
121150

122151
/// A cache of the [GlobalSettingsData] singleton in the underlying data store.

lib/widgets/settings.dart

+33
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ class SettingsPage extends StatelessWidget {
2323
body: Column(children: [
2424
const _ThemeSetting(),
2525
const _BrowserPreferenceSetting(),
26+
if (GlobalSettingsStore.experimentalFeatureFlags.isNotEmpty)
27+
ListTile(
28+
title: Text(zulipLocalizations.experimentalFeatureSettingsPageTitle),
29+
onTap: () => Navigator.push(context,
30+
ExperimentalFeaturesPage.buildRoute()))
2631
]));
2732
}
2833
}
@@ -76,3 +81,31 @@ class _BrowserPreferenceSetting extends StatelessWidget {
7681
onChanged: (newValue) => _handleChange(context, newValue));
7782
}
7883
}
84+
85+
class ExperimentalFeaturesPage extends StatelessWidget {
86+
const ExperimentalFeaturesPage({super.key});
87+
88+
static WidgetRoute<void> buildRoute() {
89+
return MaterialWidgetRoute(page: const ExperimentalFeaturesPage());
90+
}
91+
92+
@override
93+
Widget build(BuildContext context) {
94+
final zulipLocalizations = ZulipLocalizations.of(context);
95+
final globalSettings = GlobalStoreWidget.settingsOf(context);
96+
final flags = GlobalSettingsStore.experimentalFeatureFlags;
97+
assert(flags.isNotEmpty);
98+
return Scaffold(
99+
appBar: AppBar(
100+
title: Text(zulipLocalizations.experimentalFeatureSettingsPageTitle)),
101+
body: Column(children: [
102+
ListTile(
103+
title: Text(zulipLocalizations.experimentalFeatureSettingsWarning)),
104+
for (final flag in flags)
105+
SwitchListTile.adaptive(
106+
title: Text(flag.name), // no i18n; these are developer-facing settings
107+
value: globalSettings.getBool(flag),
108+
onChanged: (value) => globalSettings.setBool(flag, value)),
109+
]));
110+
}
111+
}

test/widgets/settings_test.dart

+7
Original file line numberDiff line numberDiff line change
@@ -126,4 +126,11 @@ void main() {
126126
? BrowserPreference.inApp : BrowserPreference.external);
127127
}, variant: TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
128128
});
129+
130+
// TODO maybe test GlobalSettingType.experimentalFeatureFlag settings
131+
// Or maybe not; after all, it's a developer-facing feature, so
132+
// should be low risk.
133+
// (The main ingredient in writing such tests would be to wire up
134+
// [GlobalSettingsStore.experimentalFeatureFlags] so that tests can
135+
// control making it empty, or non-empty, at will.)
129136
}

0 commit comments

Comments
 (0)