Skip to content

Commit 1d0bb4f

Browse files
authored
[url_launcher][web] Link should work when triggered by keyboard (flutter#6505)
### Background You can think of the `Link` widget (on the web) as two components working together: 1. The `<a>` element created by the `Link` widget. This is essential to make all browser interactions feel natural (e.g. context menu, cmd+click, etc). 2. The children of `Link` widget. These are the widgets visible to the user (e.g. a button or a hyperlink text) and the user can interact with them the same way they would interact with any Flutter widgets (focus, pointer click, etc). In order for the Link widget to navigate to a URI, the two components from above have to indicate their intent of navigation: 1. Some widget has to call `followLink` to indicate that the click successfully landed (i.e. hit tested) on it. E.g. if it's a button, then the `onPressed` callback should lead to a call to the Link's `followLink`. 2. The `<a>` element also has to receive an event to initiate the navigation. ### The PR We used to only handle click events on the `<a>` element, and no handling for keyboard events was present. So when a user tabs their way to the Link, then hits "Enter", the following happens: 1. The focused widget (e.g. button) that received the "Enter" will correctly indicate its intent to navigate by calling `followLink`. 2. The intent from the `<a>` element is lost because we were only handling clicks and not keyboard events. This PR adds handling of keyboard events so that it works similar to clicks. Fixes flutter#97863
1 parent 6116ae5 commit 1d0bb4f

File tree

4 files changed

+331
-11
lines changed

4 files changed

+331
-11
lines changed

packages/url_launcher/url_launcher_web/CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 2.3.1
2+
3+
* Implements correct handling of keyboard events with Link.
4+
15
## 2.3.0
26

37
* Updates web code to package `web: ^0.5.0`.
@@ -24,7 +28,7 @@
2428
## 2.1.0
2529

2630
* Adds `launchUrl` implementation.
27-
* Prevents _Tabnabbing_ and disallows `javascript:` URLs on `launch` and `launchUrl`.
31+
* Prevents _Tabnabbing_ and disallows `javascript:` URLs on `launch` and `launchUrl`.
2832

2933
## 2.0.20
3034

packages/url_launcher/url_launcher_web/example/integration_test/link_widget_test.dart

Lines changed: 230 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ import 'dart:js_interop';
66
import 'dart:js_interop_unsafe';
77
import 'dart:ui_web' as ui_web;
88

9-
import 'package:flutter/widgets.dart';
9+
import 'package:flutter/material.dart';
1010
import 'package:flutter_test/flutter_test.dart';
1111
import 'package:integration_test/integration_test.dart';
1212
import 'package:url_launcher_platform_interface/link.dart';
13+
import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart';
1314
import 'package:url_launcher_web/src/link.dart';
15+
import 'package:url_launcher_web/url_launcher_web.dart';
1416
import 'package:web/web.dart' as html;
1517

1618
void main() {
@@ -171,6 +173,196 @@ void main() {
171173
await tester.pumpAndSettle();
172174
});
173175
});
176+
177+
group('Follows links', () {
178+
late TestUrlLauncherPlugin testPlugin;
179+
late UrlLauncherPlatform originalPlugin;
180+
181+
setUp(() {
182+
originalPlugin = UrlLauncherPlatform.instance;
183+
testPlugin = TestUrlLauncherPlugin();
184+
UrlLauncherPlatform.instance = testPlugin;
185+
});
186+
187+
tearDown(() {
188+
UrlLauncherPlatform.instance = originalPlugin;
189+
});
190+
191+
testWidgets('click to navigate to internal link',
192+
(WidgetTester tester) async {
193+
final TestNavigatorObserver observer = TestNavigatorObserver();
194+
final Uri uri = Uri.parse('/foobar');
195+
FollowLink? followLinkCallback;
196+
197+
await tester.pumpWidget(MaterialApp(
198+
navigatorObservers: <NavigatorObserver>[observer],
199+
routes: <String, WidgetBuilder>{
200+
'/foobar': (BuildContext context) => const Text('Internal route'),
201+
},
202+
home: Directionality(
203+
textDirection: TextDirection.ltr,
204+
child: WebLinkDelegate(TestLinkInfo(
205+
uri: uri,
206+
target: LinkTarget.blank,
207+
builder: (BuildContext context, FollowLink? followLink) {
208+
followLinkCallback = followLink;
209+
return const SizedBox(width: 100, height: 100);
210+
},
211+
)),
212+
),
213+
));
214+
// Platform view creation happens asynchronously.
215+
await tester.pumpAndSettle();
216+
217+
expect(observer.currentRouteName, '/');
218+
expect(testPlugin.launches, isEmpty);
219+
220+
final html.Element anchor = _findSingleAnchor();
221+
222+
await followLinkCallback!();
223+
_simulateClick(anchor);
224+
await tester.pumpAndSettle();
225+
226+
// Internal links should navigate the app to the specified route. There
227+
// should be no calls to `launchUrl`.
228+
expect(observer.currentRouteName, '/foobar');
229+
expect(testPlugin.launches, isEmpty);
230+
231+
// Needed when testing on on Chrome98 headless in CI.
232+
// See https://github.com/flutter/flutter/issues/121161
233+
await tester.pumpAndSettle();
234+
});
235+
236+
testWidgets('keydown to navigate to internal link',
237+
(WidgetTester tester) async {
238+
final TestNavigatorObserver observer = TestNavigatorObserver();
239+
final Uri uri = Uri.parse('/foobar');
240+
FollowLink? followLinkCallback;
241+
242+
await tester.pumpWidget(MaterialApp(
243+
navigatorObservers: <NavigatorObserver>[observer],
244+
routes: <String, WidgetBuilder>{
245+
'/foobar': (BuildContext context) => const Text('Internal route'),
246+
},
247+
home: Directionality(
248+
textDirection: TextDirection.ltr,
249+
child: WebLinkDelegate(TestLinkInfo(
250+
uri: uri,
251+
target: LinkTarget.blank,
252+
builder: (BuildContext context, FollowLink? followLink) {
253+
followLinkCallback = followLink;
254+
return const SizedBox(width: 100, height: 100);
255+
},
256+
)),
257+
),
258+
));
259+
// Platform view creation happens asynchronously.
260+
await tester.pumpAndSettle();
261+
262+
expect(observer.currentRouteName, '/');
263+
expect(testPlugin.launches, isEmpty);
264+
265+
final html.Element anchor = _findSingleAnchor();
266+
267+
await followLinkCallback!();
268+
_simulateKeydown(anchor);
269+
await tester.pumpAndSettle();
270+
271+
// Internal links should navigate the app to the specified route. There
272+
// should be no calls to `launchUrl`.
273+
expect(observer.currentRouteName, '/foobar');
274+
expect(testPlugin.launches, isEmpty);
275+
276+
// Needed when testing on on Chrome98 headless in CI.
277+
// See https://github.com/flutter/flutter/issues/121161
278+
await tester.pumpAndSettle();
279+
});
280+
281+
testWidgets('click to navigate to external link',
282+
(WidgetTester tester) async {
283+
final TestNavigatorObserver observer = TestNavigatorObserver();
284+
final Uri uri = Uri.parse('https://google.com');
285+
FollowLink? followLinkCallback;
286+
287+
await tester.pumpWidget(MaterialApp(
288+
navigatorObservers: <NavigatorObserver>[observer],
289+
home: Directionality(
290+
textDirection: TextDirection.ltr,
291+
child: WebLinkDelegate(TestLinkInfo(
292+
uri: uri,
293+
target: LinkTarget.blank,
294+
builder: (BuildContext context, FollowLink? followLink) {
295+
followLinkCallback = followLink;
296+
return const SizedBox(width: 100, height: 100);
297+
},
298+
)),
299+
),
300+
));
301+
// Platform view creation happens asynchronously.
302+
await tester.pumpAndSettle();
303+
304+
expect(observer.currentRouteName, '/');
305+
expect(testPlugin.launches, isEmpty);
306+
307+
final html.Element anchor = _findSingleAnchor();
308+
309+
await followLinkCallback!();
310+
_simulateClick(anchor);
311+
await tester.pumpAndSettle();
312+
313+
// External links that are triggered by a click are left to be handled by
314+
// the browser, so there should be no change to the app's route name, and
315+
// no calls to `launchUrl`.
316+
expect(observer.currentRouteName, '/');
317+
expect(testPlugin.launches, isEmpty);
318+
319+
// Needed when testing on on Chrome98 headless in CI.
320+
// See https://github.com/flutter/flutter/issues/121161
321+
await tester.pumpAndSettle();
322+
});
323+
324+
testWidgets('keydown to navigate to external link',
325+
(WidgetTester tester) async {
326+
final TestNavigatorObserver observer = TestNavigatorObserver();
327+
final Uri uri = Uri.parse('https://google.com');
328+
FollowLink? followLinkCallback;
329+
330+
await tester.pumpWidget(MaterialApp(
331+
navigatorObservers: <NavigatorObserver>[observer],
332+
home: Directionality(
333+
textDirection: TextDirection.ltr,
334+
child: WebLinkDelegate(TestLinkInfo(
335+
uri: uri,
336+
target: LinkTarget.blank,
337+
builder: (BuildContext context, FollowLink? followLink) {
338+
followLinkCallback = followLink;
339+
return const SizedBox(width: 100, height: 100);
340+
},
341+
)),
342+
),
343+
));
344+
// Platform view creation happens asynchronously.
345+
await tester.pumpAndSettle();
346+
347+
expect(observer.currentRouteName, '/');
348+
expect(testPlugin.launches, isEmpty);
349+
350+
final html.Element anchor = _findSingleAnchor();
351+
352+
await followLinkCallback!();
353+
_simulateKeydown(anchor);
354+
await tester.pumpAndSettle();
355+
356+
// External links that are triggered by keyboard are handled by calling
357+
// `launchUrl`, and there's no change to the app's route name.
358+
expect(observer.currentRouteName, '/');
359+
expect(testPlugin.launches, <String>['https://google.com']);
360+
361+
// Needed when testing on on Chrome98 headless in CI.
362+
// See https://github.com/flutter/flutter/issues/121161
363+
await tester.pumpAndSettle();
364+
});
365+
});
174366
}
175367

176368
html.Element _findSingleAnchor() {
@@ -199,6 +391,33 @@ html.Element _findSingleAnchor() {
199391
return foundAnchors.single;
200392
}
201393

394+
void _simulateClick(html.Element target) {
395+
target.dispatchEvent(
396+
html.MouseEvent(
397+
'click',
398+
html.MouseEventInit()..bubbles = true,
399+
),
400+
);
401+
}
402+
403+
void _simulateKeydown(html.Element target) {
404+
target.dispatchEvent(
405+
html.KeyboardEvent(
406+
'keydown',
407+
html.KeyboardEventInit()..bubbles = true,
408+
),
409+
);
410+
}
411+
412+
class TestNavigatorObserver extends NavigatorObserver {
413+
String? currentRouteName;
414+
415+
@override
416+
void didPush(Route<dynamic> route, Route<dynamic>? previousRoute) {
417+
currentRouteName = route.settings.name;
418+
}
419+
}
420+
202421
class TestLinkInfo extends LinkInfo {
203422
TestLinkInfo({
204423
required this.uri,
@@ -218,3 +437,13 @@ class TestLinkInfo extends LinkInfo {
218437
@override
219438
bool get isDisabled => uri == null;
220439
}
440+
441+
class TestUrlLauncherPlugin extends UrlLauncherPlugin {
442+
final List<String> launches = <String>[];
443+
444+
@override
445+
Future<bool> launchUrl(String url, LaunchOptions options) async {
446+
launches.add(url);
447+
return true;
448+
}
449+
}

0 commit comments

Comments
 (0)