Skip to content

Commit 6decffd

Browse files
committed
content [nfc]: Follow browser preference setting when opening links
This preserves the previous behavior (external application on iOS, and platform default on Android) when browserPreference is `null`. There is no way for the client to update the setting for now, hence the NFC. The helper could have been a static method BrowserPreference.getUrlLaunchMode, similar to ThemeSetting.displayName, but that's a lot more verbose for the caller. This also updates references to use the alias `UrlLaunchMode` in the comment instead, since that's the type the developer sees in this context. Signed-off-by: Zixuan James Li <[email protected]>
1 parent e4b3c31 commit 6decffd

File tree

5 files changed

+68
-9
lines changed

5 files changed

+68
-9
lines changed

lib/model/settings.dart

+23
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1+
import 'package:flutter/foundation.dart';
2+
13
import '../generated/l10n/zulip_localizations.dart';
4+
import 'binding.dart';
5+
import 'database.dart';
26

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

lib/widgets/content.dart

+3-9
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/rendering.dart';
@@ -16,6 +15,7 @@ import '../model/avatar_url.dart';
1615
import '../model/binding.dart';
1716
import '../model/content.dart';
1817
import '../model/internal_link.dart';
18+
import '../model/settings.dart';
1919
import 'code_block.dart';
2020
import 'dialog.dart';
2121
import 'icons.dart';
@@ -1439,18 +1439,12 @@ void _launchUrl(BuildContext context, String urlString) async {
14391439
return;
14401440
}
14411441

1442+
final globalSettings = GlobalStoreWidget.of(context).globalSettings;
14421443
bool launched = false;
14431444
String? errorMessage;
14441445
try {
14451446
launched = await ZulipBinding.instance.launchUrl(url,
1446-
mode: switch (defaultTargetPlatform) {
1447-
// On iOS we prefer LaunchMode.externalApplication because (for
1448-
// HTTP URLs) LaunchMode.platformDefault uses SFSafariViewController,
1449-
// which gives an awkward UX as described here:
1450-
// https://chat.zulip.org/#narrow/stream/48-mobile/topic/in-app.20browser/near/1169118
1451-
TargetPlatform.iOS => UrlLaunchMode.externalApplication,
1452-
_ => UrlLaunchMode.platformDefault,
1453-
});
1447+
mode: globalSettings.urlLaunchMode);
14541448
} on PlatformException catch (e) {
14551449
errorMessage = e.message;
14561450
}

test/model/settings_test.dart

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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/binding.dart';
5+
import 'package:zulip/model/settings.dart';
6+
7+
import '../example_data.dart' as eg;
8+
import 'store_checks.dart';
9+
import 'store_test.dart';
10+
11+
void main() {
12+
group('urlLaunchMode', () {
13+
testAndroidIos('globalSettings.browserPreference is null; use platform-specific defaults', () {
14+
check(eg.globalStore()).globalSettings.urlLaunchMode.equals(
15+
defaultTargetPlatform == TargetPlatform.iOS
16+
? UrlLaunchMode.externalApplication : UrlLaunchMode.platformDefault);
17+
});
18+
19+
testAndroidIos('globalSettings.browserPreference is non-null; follow the preference', () {
20+
final globalStore = eg.globalStore(globalSettings: eg.globalSettings(
21+
browserPreference: BrowserPreference.external));
22+
check(globalStore).globalSettings.urlLaunchMode.equals(
23+
UrlLaunchMode.externalApplication);
24+
});
25+
});
26+
}

test/model/store_checks.dart

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'package:zulip/api/core.dart';
33
import 'package:zulip/api/model/initial_snapshot.dart';
44
import 'package:zulip/api/model/model.dart';
55
import 'package:zulip/model/autocomplete.dart';
6+
import 'package:zulip/model/binding.dart';
67
import 'package:zulip/model/database.dart';
78
import 'package:zulip/model/recent_dm_conversations.dart';
89
import 'package:zulip/model/settings.dart';
@@ -32,6 +33,7 @@ extension GlobalStoreChecks on Subject<GlobalStore> {
3233
extension GlobalSettingsDataChecks on Subject<GlobalSettingsData> {
3334
Subject<ThemeSetting?> get themeSetting => has((x) => x.themeSetting, 'themeSetting');
3435
Subject<BrowserPreference?> get browserPreference => has((x) => x.browserPreference, 'browserPreference');
36+
Subject<UrlLaunchMode> get urlLaunchMode => has((x) => x.urlLaunchMode, 'urlLaunchMode');
3537
}
3638

3739
extension PerAccountStoreChecks on Subject<PerAccountStore> {

test/widgets/content_test.dart

+14
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';
@@ -798,6 +799,19 @@ void main() {
798799
.single.equals((url: Uri.parse('https://example/'), mode: expectedLaunchMode));
799800
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
800801

802+
testWidgets('follow browser preference setting to open URL', (tester) async {
803+
await testBinding.globalStore.updateGlobalSettings(
804+
eg.globalSettings(
805+
browserPreference: BrowserPreference.embedded).toCompanion(false));
806+
await prepare(tester,
807+
'<p><a href="https://example/">hello</a></p>');
808+
809+
await tapText(tester, find.text('hello'));
810+
check(testBinding.takeLaunchUrlCalls())
811+
.single.equals((
812+
url: Uri.parse('https://example/'), mode: LaunchMode.platformDefault));
813+
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
814+
801815
testWidgets('multiple links in paragraph', (tester) async {
802816
const fontSize = kBaseFontSize;
803817

0 commit comments

Comments
 (0)