Skip to content

Commit

Permalink
content: Open links in-app on Android
Browse files Browse the repository at this point in the history
`url_launcher` plugin now supports the desired behavior, which is using
Android Custom Tabs, so we don't need the workaround of opening the
links in external browser anymore, thus removed them.

Fixes: zulip#279
  • Loading branch information
rajveermalviya authored and gnprice committed Nov 7, 2023
1 parent 362f4d1 commit 806a81a
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 33 deletions.
10 changes: 1 addition & 9 deletions lib/widgets/content.dart
Original file line number Diff line number Diff line change
Expand Up @@ -680,15 +680,7 @@ void _launchUrl(BuildContext context, String urlString) async {
bool launched = false;
String? errorMessage;
try {
launched = await ZulipBinding.instance.launchUrl(url,
mode: switch (Theme.of(context).platform) {
// TODO(#279): On Android we settle for LaunchMode.externalApplication
// because url_launcher's in-app is a weirdly bare UX.
// Switch once that's fixed upstream (by us or otherwise).
TargetPlatform.android => UrlLaunchMode.externalApplication,
_ => UrlLaunchMode.platformDefault,
},
);
launched = await ZulipBinding.instance.launchUrl(url);
} on PlatformException catch (e) {
errorMessage = e.message;
}
Expand Down
25 changes: 10 additions & 15 deletions test/widgets/content_test.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import 'dart:io';

import 'package:checks/checks.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
import 'package:flutter_test/flutter_test.dart';
Expand Down Expand Up @@ -64,8 +63,6 @@ void main() {
});

group('LinkNode interactions', () {
const expectedModeAndroid = LaunchMode.externalApplication;

// The Flutter test font uses square glyphs, so width equals height:
// https://github.com/flutter/flutter/wiki/Flutter-Test-Fonts
const fontSize = 48.0;
Expand All @@ -89,10 +86,8 @@ void main() {
'<p><a href="https://example/">hello</a></p>');

await tester.tap(find.text('hello'));
final expectedMode = defaultTargetPlatform == TargetPlatform.android ?
LaunchMode.externalApplication : LaunchMode.platformDefault;
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('https://example/'), mode: expectedMode));
.single.equals((url: Uri.parse('https://example/'), mode: LaunchMode.platformDefault));
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));

testWidgets('multiple links in paragraph', (tester) async {
Expand All @@ -106,19 +101,19 @@ void main() {

await tester.tapAt(base.translate(1*fontSize, 0)); // "fXo bar baz"
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('https://a/'), mode: expectedModeAndroid));
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));

await tester.tapAt(base.translate(9*fontSize, 0)); // "foo bar bXz"
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('https://b/'), mode: expectedModeAndroid));
.single.equals((url: Uri.parse('https://b/'), mode: LaunchMode.platformDefault));
});

testWidgets('link nested in other spans', (tester) async {
await prepareContent(tester,
'<p><strong><em><a href="https://a/">word</a></em></strong></p>');
await tester.tap(find.text('word'));
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('https://a/'), mode: expectedModeAndroid));
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
});

testWidgets('link containing other spans', (tester) async {
Expand All @@ -129,27 +124,27 @@ void main() {

await tester.tapAt(base.translate(1*fontSize, 0)); // "tXo words"
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('https://a/'), mode: expectedModeAndroid));
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));

await tester.tapAt(base.translate(6*fontSize, 0)); // "two woXds"
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('https://a/'), mode: expectedModeAndroid));
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
});

testWidgets('relative links are resolved', (tester) async {
await prepareContent(tester,
'<p><a href="/a/b?c#d">word</a></p>');
await tester.tap(find.text('word'));
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('${eg.realmUrl}a/b?c#d'), mode: expectedModeAndroid));
.single.equals((url: Uri.parse('${eg.realmUrl}a/b?c#d'), mode: LaunchMode.platformDefault));
});

testWidgets('link inside HeadingNode', (tester) async {
await prepareContent(tester,
'<h6><a href="https://a/">word</a></h6>');
await tester.tap(find.text('word'));
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('https://a/'), mode: expectedModeAndroid));
.single.equals((url: Uri.parse('https://a/'), mode: LaunchMode.platformDefault));
});

testWidgets('error dialog if invalid link', (tester) async {
Expand All @@ -159,7 +154,7 @@ void main() {
await tester.tap(find.text('word'));
await tester.pump();
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('file:///etc/bad'), mode: expectedModeAndroid));
.single.equals((url: Uri.parse('file:///etc/bad'), mode: LaunchMode.platformDefault));
checkErrorDialog(tester, expectedTitle: 'Unable to open link');
});
});
Expand Down Expand Up @@ -206,7 +201,7 @@ void main() {
await tester.tap(find.text('invalid'));
final expectedUrl = eg.realmUrl.resolve('/#narrow/stream/1-check/topic');
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: expectedUrl, mode: LaunchMode.externalApplication));
.single.equals((url: expectedUrl, mode: LaunchMode.platformDefault));
check(pushedRoutes).isEmpty();
});
});
Expand Down
17 changes: 8 additions & 9 deletions test/widgets/profile_test.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import 'package:checks/checks.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
import 'package:flutter_test/flutter_test.dart';
Expand Down Expand Up @@ -167,10 +166,10 @@ void main() {
);

await tester.tap(find.text(testUrl));
final expectedMode = defaultTargetPlatform == TargetPlatform.android ?
LaunchMode.externalApplication : LaunchMode.platformDefault;
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse(testUrl), mode: expectedMode));
check(testBinding.takeLaunchUrlCalls()).single.equals((
url: Uri.parse(testUrl),
mode: LaunchMode.platformDefault,
));
});

testWidgets('page builds; external link type navigates away', (WidgetTester tester) async {
Expand All @@ -194,10 +193,10 @@ void main() {
);

await tester.tap(find.text('externalValue'));
final expectedMode = defaultTargetPlatform == TargetPlatform.android ?
LaunchMode.externalApplication : LaunchMode.platformDefault;
check(testBinding.takeLaunchUrlCalls())
.single.equals((url: Uri.parse('http://example/externalValue'), mode: expectedMode));
check(testBinding.takeLaunchUrlCalls()).single.equals((
url: Uri.parse('http://example/externalValue'),
mode: LaunchMode.platformDefault,
));
});

testWidgets('page builds; user links to profile', (WidgetTester tester) async {
Expand Down

0 comments on commit 806a81a

Please sign in to comment.