Skip to content

Commit eaf6a88

Browse files
committed
content: Follow browser preference setting when opening links
Signed-off-by: Zixuan James Li <[email protected]>
1 parent 5c7e741 commit eaf6a88

File tree

5 files changed

+66
-8
lines changed

5 files changed

+66
-8
lines changed

Diff for: lib/model/settings.dart

+17
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'package:flutter/foundation.dart';
22

33
import '../generated/l10n/zulip_localizations.dart';
4+
import 'database.dart';
45

56
/// The visual theme of the app.
67
///
@@ -44,3 +45,19 @@ enum BrowserPreference {
4445
/// Use the user's default browser app.
4546
external,
4647
}
48+
49+
extension GlobalSettingsHelpers on GlobalSettingsData {
50+
BrowserPreference get effectiveBrowserPreference {
51+
if (browserPreference != null) {
52+
return browserPreference!;
53+
}
54+
return switch (defaultTargetPlatform) {
55+
// On iOS we prefer LaunchMode.externalApplication because (for
56+
// HTTP URLs) LaunchMode.platformDefault uses SFSafariViewController,
57+
// which gives an awkward UX as described here:
58+
// https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
59+
TargetPlatform.iOS => BrowserPreference.external,
60+
_ => BrowserPreference.embedded,
61+
};
62+
}
63+
}

Diff for: lib/widgets/content.dart

+5-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import 'dart:async';
22

33
import 'package:flutter/cupertino.dart';
4-
import 'package:flutter/foundation.dart';
54
import 'package:flutter/gestures.dart';
65
import 'package:flutter/material.dart';
76
import 'package:flutter/services.dart';
@@ -15,6 +14,7 @@ import '../model/avatar_url.dart';
1514
import '../model/binding.dart';
1615
import '../model/content.dart';
1716
import '../model/internal_link.dart';
17+
import '../model/settings.dart';
1818
import 'code_block.dart';
1919
import 'dialog.dart';
2020
import 'icons.dart';
@@ -1343,17 +1343,14 @@ void _launchUrl(BuildContext context, String urlString) async {
13431343
return;
13441344
}
13451345

1346+
final globalSettings = GlobalStoreWidget.of(context).globalSettings;
13461347
bool launched = false;
13471348
String? errorMessage;
13481349
try {
13491350
launched = await ZulipBinding.instance.launchUrl(url,
1350-
mode: switch (defaultTargetPlatform) {
1351-
// On iOS we prefer LaunchMode.externalApplication because (for
1352-
// HTTP URLs) LaunchMode.platformDefault uses SFSafariViewController,
1353-
// which gives an awkward UX as described here:
1354-
// https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
1355-
TargetPlatform.iOS => UrlLaunchMode.externalApplication,
1356-
_ => UrlLaunchMode.platformDefault,
1351+
mode: switch (globalSettings.effectiveBrowserPreference) {
1352+
BrowserPreference.embedded => UrlLaunchMode.inAppBrowserView,
1353+
BrowserPreference.external => UrlLaunchMode.externalApplication,
13571354
});
13581355
} on PlatformException catch (e) {
13591356
errorMessage = e.message;

Diff for: test/model/settings_test.dart

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:flutter/foundation.dart';
3+
import 'package:flutter_test/flutter_test.dart';
4+
import 'package:zulip/model/settings.dart';
5+
6+
import '../example_data.dart' as eg;
7+
import 'store_checks.dart';
8+
import 'store_test.dart';
9+
10+
void main() {
11+
group('effectiveBrowserPreference', () {
12+
testAndroidIos('use non-default values', () {
13+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
14+
browserPreference: BrowserPreference.external));
15+
check(globalStore).globalSettings.effectiveBrowserPreference.equals(
16+
BrowserPreference.external);
17+
});
18+
19+
testAndroidIos('use default values by platform', () {
20+
final globalStore = eg.globalStore();
21+
if (defaultTargetPlatform == TargetPlatform.iOS) {
22+
check(globalStore).globalSettings.effectiveBrowserPreference.equals(
23+
BrowserPreference.external);
24+
} else {
25+
check(globalStore).globalSettings.effectiveBrowserPreference.equals(
26+
BrowserPreference.embedded);
27+
}
28+
});
29+
});
30+
}

Diff for: test/model/store_checks.dart

+1
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,5 @@ extension PerAccountStoreChecks on Subject<PerAccountStore> {
5454

5555
extension GlobalSettingsDataChecks on Subject<GlobalSettingsData> {
5656
Subject<ThemeSetting> get themeSetting => has((x) => x.themeSetting, 'themeSetting');
57+
Subject<BrowserPreference> get effectiveBrowserPreference => has((x) => x.effectiveBrowserPreference, 'effectiveBrowserPreference');
5758
}

Diff for: test/widgets/content_test.dart

+13
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'package:url_launcher/url_launcher.dart';
99
import 'package:zulip/api/core.dart';
1010
import 'package:zulip/model/content.dart';
1111
import 'package:zulip/model/narrow.dart';
12+
import 'package:zulip/model/settings.dart';
1213
import 'package:zulip/model/store.dart';
1314
import 'package:zulip/widgets/content.dart';
1415
import 'package:zulip/widgets/icons.dart';
@@ -759,6 +760,18 @@ void main() {
759760
.single.equals((url: Uri.parse('https://example/'), mode: expectedLaunchMode));
760761
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
761762

763+
testWidgets('override default with browser preference setting', (tester) async {
764+
await testBinding.globalStore.updateGlobalSettings(
765+
eg.globalSettings(
766+
browserPreference: BrowserPreference.external).toCompanion(false));
767+
await prepare(tester,
768+
'<p><a href="https://example/">hello</a></p>');
769+
770+
await tapText(tester, find.text('hello'));
771+
check(testBinding.takeLaunchUrlCalls())
772+
.single.equals((url: Uri.parse('https://example/'), mode: LaunchMode.externalApplication));
773+
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
774+
762775
testWidgets('multiple links in paragraph', (tester) async {
763776
const fontSize = kBaseFontSize;
764777

0 commit comments

Comments
 (0)