Skip to content

Commit

Permalink
[url_launcher][web] Link should work when triggered by keyboard (#6505)
Browse files Browse the repository at this point in the history
### 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/flutter#97863
  • Loading branch information
mdebbar authored Apr 15, 2024
1 parent 6116ae5 commit 1d0bb4f
Show file tree
Hide file tree
Showing 4 changed files with 331 additions and 11 deletions.
6 changes: 5 additions & 1 deletion packages/url_launcher/url_launcher_web/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.3.1

* Implements correct handling of keyboard events with Link.

## 2.3.0

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

* Adds `launchUrl` implementation.
* Prevents _Tabnabbing_ and disallows `javascript:` URLs on `launch` and `launchUrl`.
* Prevents _Tabnabbing_ and disallows `javascript:` URLs on `launch` and `launchUrl`.

## 2.0.20

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import 'dart:js_interop';
import 'dart:js_interop_unsafe';
import 'dart:ui_web' as ui_web;

import 'package:flutter/widgets.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:integration_test/integration_test.dart';
import 'package:url_launcher_platform_interface/link.dart';
import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart';
import 'package:url_launcher_web/src/link.dart';
import 'package:url_launcher_web/url_launcher_web.dart';
import 'package:web/web.dart' as html;

void main() {
Expand Down Expand Up @@ -171,6 +173,196 @@ void main() {
await tester.pumpAndSettle();
});
});

group('Follows links', () {
late TestUrlLauncherPlugin testPlugin;
late UrlLauncherPlatform originalPlugin;

setUp(() {
originalPlugin = UrlLauncherPlatform.instance;
testPlugin = TestUrlLauncherPlugin();
UrlLauncherPlatform.instance = testPlugin;
});

tearDown(() {
UrlLauncherPlatform.instance = originalPlugin;
});

testWidgets('click to navigate to internal link',
(WidgetTester tester) async {
final TestNavigatorObserver observer = TestNavigatorObserver();
final Uri uri = Uri.parse('/foobar');
FollowLink? followLinkCallback;

await tester.pumpWidget(MaterialApp(
navigatorObservers: <NavigatorObserver>[observer],
routes: <String, WidgetBuilder>{
'/foobar': (BuildContext context) => const Text('Internal route'),
},
home: Directionality(
textDirection: TextDirection.ltr,
child: WebLinkDelegate(TestLinkInfo(
uri: uri,
target: LinkTarget.blank,
builder: (BuildContext context, FollowLink? followLink) {
followLinkCallback = followLink;
return const SizedBox(width: 100, height: 100);
},
)),
),
));
// Platform view creation happens asynchronously.
await tester.pumpAndSettle();

expect(observer.currentRouteName, '/');
expect(testPlugin.launches, isEmpty);

final html.Element anchor = _findSingleAnchor();

await followLinkCallback!();
_simulateClick(anchor);
await tester.pumpAndSettle();

// Internal links should navigate the app to the specified route. There
// should be no calls to `launchUrl`.
expect(observer.currentRouteName, '/foobar');
expect(testPlugin.launches, isEmpty);

// Needed when testing on on Chrome98 headless in CI.
// See https://github.com/flutter/flutter/issues/121161
await tester.pumpAndSettle();
});

testWidgets('keydown to navigate to internal link',
(WidgetTester tester) async {
final TestNavigatorObserver observer = TestNavigatorObserver();
final Uri uri = Uri.parse('/foobar');
FollowLink? followLinkCallback;

await tester.pumpWidget(MaterialApp(
navigatorObservers: <NavigatorObserver>[observer],
routes: <String, WidgetBuilder>{
'/foobar': (BuildContext context) => const Text('Internal route'),
},
home: Directionality(
textDirection: TextDirection.ltr,
child: WebLinkDelegate(TestLinkInfo(
uri: uri,
target: LinkTarget.blank,
builder: (BuildContext context, FollowLink? followLink) {
followLinkCallback = followLink;
return const SizedBox(width: 100, height: 100);
},
)),
),
));
// Platform view creation happens asynchronously.
await tester.pumpAndSettle();

expect(observer.currentRouteName, '/');
expect(testPlugin.launches, isEmpty);

final html.Element anchor = _findSingleAnchor();

await followLinkCallback!();
_simulateKeydown(anchor);
await tester.pumpAndSettle();

// Internal links should navigate the app to the specified route. There
// should be no calls to `launchUrl`.
expect(observer.currentRouteName, '/foobar');
expect(testPlugin.launches, isEmpty);

// Needed when testing on on Chrome98 headless in CI.
// See https://github.com/flutter/flutter/issues/121161
await tester.pumpAndSettle();
});

testWidgets('click to navigate to external link',
(WidgetTester tester) async {
final TestNavigatorObserver observer = TestNavigatorObserver();
final Uri uri = Uri.parse('https://google.com');
FollowLink? followLinkCallback;

await tester.pumpWidget(MaterialApp(
navigatorObservers: <NavigatorObserver>[observer],
home: Directionality(
textDirection: TextDirection.ltr,
child: WebLinkDelegate(TestLinkInfo(
uri: uri,
target: LinkTarget.blank,
builder: (BuildContext context, FollowLink? followLink) {
followLinkCallback = followLink;
return const SizedBox(width: 100, height: 100);
},
)),
),
));
// Platform view creation happens asynchronously.
await tester.pumpAndSettle();

expect(observer.currentRouteName, '/');
expect(testPlugin.launches, isEmpty);

final html.Element anchor = _findSingleAnchor();

await followLinkCallback!();
_simulateClick(anchor);
await tester.pumpAndSettle();

// External links that are triggered by a click are left to be handled by
// the browser, so there should be no change to the app's route name, and
// no calls to `launchUrl`.
expect(observer.currentRouteName, '/');
expect(testPlugin.launches, isEmpty);

// Needed when testing on on Chrome98 headless in CI.
// See https://github.com/flutter/flutter/issues/121161
await tester.pumpAndSettle();
});

testWidgets('keydown to navigate to external link',
(WidgetTester tester) async {
final TestNavigatorObserver observer = TestNavigatorObserver();
final Uri uri = Uri.parse('https://google.com');
FollowLink? followLinkCallback;

await tester.pumpWidget(MaterialApp(
navigatorObservers: <NavigatorObserver>[observer],
home: Directionality(
textDirection: TextDirection.ltr,
child: WebLinkDelegate(TestLinkInfo(
uri: uri,
target: LinkTarget.blank,
builder: (BuildContext context, FollowLink? followLink) {
followLinkCallback = followLink;
return const SizedBox(width: 100, height: 100);
},
)),
),
));
// Platform view creation happens asynchronously.
await tester.pumpAndSettle();

expect(observer.currentRouteName, '/');
expect(testPlugin.launches, isEmpty);

final html.Element anchor = _findSingleAnchor();

await followLinkCallback!();
_simulateKeydown(anchor);
await tester.pumpAndSettle();

// External links that are triggered by keyboard are handled by calling
// `launchUrl`, and there's no change to the app's route name.
expect(observer.currentRouteName, '/');
expect(testPlugin.launches, <String>['https://google.com']);

// Needed when testing on on Chrome98 headless in CI.
// See https://github.com/flutter/flutter/issues/121161
await tester.pumpAndSettle();
});
});
}

html.Element _findSingleAnchor() {
Expand Down Expand Up @@ -199,6 +391,33 @@ html.Element _findSingleAnchor() {
return foundAnchors.single;
}

void _simulateClick(html.Element target) {
target.dispatchEvent(
html.MouseEvent(
'click',
html.MouseEventInit()..bubbles = true,
),
);
}

void _simulateKeydown(html.Element target) {
target.dispatchEvent(
html.KeyboardEvent(
'keydown',
html.KeyboardEventInit()..bubbles = true,
),
);
}

class TestNavigatorObserver extends NavigatorObserver {
String? currentRouteName;

@override
void didPush(Route<dynamic> route, Route<dynamic>? previousRoute) {
currentRouteName = route.settings.name;
}
}

class TestLinkInfo extends LinkInfo {
TestLinkInfo({
required this.uri,
Expand All @@ -218,3 +437,13 @@ class TestLinkInfo extends LinkInfo {
@override
bool get isDisabled => uri == null;
}

class TestUrlLauncherPlugin extends UrlLauncherPlugin {
final List<String> launches = <String>[];

@override
Future<bool> launchUrl(String url, LaunchOptions options) async {
launches.add(url);
return true;
}
}
Loading

0 comments on commit 1d0bb4f

Please sign in to comment.