diff --git a/integration_test/management_test.dart b/integration_test/management_test.dart index 6b16a3be..f152c640 100644 --- a/integration_test/management_test.dart +++ b/integration_test/management_test.dart @@ -37,7 +37,8 @@ void main() { group('Management UI tests', () { appTest('Drawer items exist', (WidgetTester tester) async { await tester.openDrawer(); - expect(find.byKey(app_keys.managementAppDrawer), findsOneWidget); + expect(find.byKey(app_keys.managementAppDrawer).hitTestable(), + findsOneWidget); }); }); diff --git a/integration_test/utils/android/util.dart b/integration_test/utils/android/util.dart index 8a0f6963..a7078731 100644 --- a/integration_test/utils/android/util.dart +++ b/integration_test/utils/android/util.dart @@ -30,9 +30,10 @@ Future startUp(WidgetTester tester, // only wait for yubikey connection when needed // needs_yubikey defaults to true if (startUpParams['needs_yubikey'] != false) { + await tester.openDrawer(); // wait for a YubiKey connection await tester.waitForFinder(find.descendant( - of: tester.findDeviceButton(), + of: find.byKey(app_keys.deviceInfoListTile), matching: find.byWidgetPredicate((widget) => widget is DeviceAvatar && widget.key != app_keys.noDeviceAvatar))); } diff --git a/integration_test/utils/oath_test_util.dart b/integration_test/utils/oath_test_util.dart index 4847524b..aee4f5c8 100644 --- a/integration_test/utils/oath_test_util.dart +++ b/integration_test/utils/oath_test_util.dart @@ -17,6 +17,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:yubico_authenticator/core/state.dart'; +import 'package:yubico_authenticator/app/views/keys.dart' as app_keys; import 'package:yubico_authenticator/oath/keys.dart' as keys; import 'package:yubico_authenticator/oath/views/account_list.dart'; import 'package:yubico_authenticator/oath/views/account_view.dart'; @@ -235,8 +236,12 @@ extension OathFunctions on WidgetTester { /// now the account dialog is shown /// TODO verify it shows correct issuer and name - /// close the account dialog by tapping out of it - await tapAt(const Offset(10, 10)); + /// close the account dialog by tapping the close button + var closeButton = find.byKey(app_keys.closeButton).hitTestable(); + // Wait for toast to clear + await waitForFinder(closeButton); + + await tap(closeButton); await longWait(); /// verify accounts in the list diff --git a/integration_test/utils/test_util.dart b/integration_test/utils/test_util.dart index 714f79ff..275475c7 100644 --- a/integration_test/utils/test_util.dart +++ b/integration_test/utils/test_util.dart @@ -17,7 +17,6 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:freezed_annotation/freezed_annotation.dart'; -import 'package:yubico_authenticator/app/views/device_button.dart'; import 'package:yubico_authenticator/app/views/keys.dart' as app_keys; import 'package:yubico_authenticator/app/views/keys.dart'; import 'package:yubico_authenticator/core/state.dart'; @@ -65,16 +64,6 @@ extension AppWidgetTester on WidgetTester { return f; } - Finder findDeviceButton() { - return find.byType(DeviceButton).hitTestable(); - } - - /// Taps the device button - Future tapDeviceButton() async { - await tap(findDeviceButton()); - await pump(const Duration(milliseconds: 500)); - } - Finder findActionIconButton() { return find.byKey(actionsIconButtonKey).hitTestable(); } @@ -119,7 +108,7 @@ extension AppWidgetTester on WidgetTester { await openDrawer(); } - await tap(find.byKey(managementAppDrawer)); + await tap(find.byKey(managementAppDrawer).hitTestable()); await pump(const Duration(milliseconds: 500)); expect(find.byKey(screenKey), findsOneWidget); @@ -153,17 +142,22 @@ extension AppWidgetTester on WidgetTester { return; } - await tapDeviceButton(); + await openDrawer(); var deviceInfo = find.byKey(app_keys.deviceInfoListTile); if (deviceInfo.evaluate().isNotEmpty) { - ListTile lt = deviceInfo.evaluate().single.widget as ListTile; + ListTile lt = find + .descendant(of: deviceInfo, matching: find.byType(ListTile)) + .evaluate() + .single + .widget as ListTile; + //ListTile lt = deviceInfo.evaluate().single.widget as ListTile; yubiKeyName = (lt.title as Text).data; var subtitle = (lt.subtitle as Text?)?.data; if (subtitle != null) { - RegExpMatch? match = RegExp(r'S/N: (\d.*) F/W: (\d\.\d\.\d)') - .firstMatch(subtitle); + RegExpMatch? match = + RegExp(r'S/N: (\d.*) F/W: (\d\.\d\.\d)').firstMatch(subtitle); if (match != null) { yubiKeySerialNumber = match.group(1); yubiKeyFirmware = match.group(2); @@ -177,7 +171,7 @@ extension AppWidgetTester on WidgetTester { } // close the opened menu - await tapTopLeftCorner(); + await closeDrawer(); testLog(false, 'Connected YubiKey: $yubiKeySerialNumber/$yubiKeyFirmware - $yubiKeyName'); diff --git a/lib/app/views/device_button.dart b/lib/app/views/device_button.dart deleted file mode 100755 index 50772ce4..00000000 --- a/lib/app/views/device_button.dart +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright (C) 2022 Yubico. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import 'package:flutter/material.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:flutter_gen/gen_l10n/app_localizations.dart'; - -import '../../core/state.dart'; -import '../message.dart'; -import 'device_avatar.dart'; -import 'device_picker_dialog.dart'; - -class _CircledDeviceAvatar extends ConsumerWidget { - final double radius; - const _CircledDeviceAvatar(this.radius); - - @override - Widget build(BuildContext context, WidgetRef ref) => CircleAvatar( - radius: radius, - backgroundColor: Theme.of(context).colorScheme.primary, - child: IconTheme( - // Force the standard icon theme - data: IconTheme.of(context), - child: DeviceAvatar.currentDevice(ref, radius: radius - 1), - ), - ); -} - -class DeviceButton extends ConsumerWidget { - final double radius; - const DeviceButton({super.key, this.radius = 16}); - - @override - Widget build(BuildContext context, WidgetRef ref) { - return IconButton( - tooltip: isAndroid - ? AppLocalizations.of(context)!.s_yk_information - : AppLocalizations.of(context)!.s_select_yk, - icon: _CircledDeviceAvatar(radius), - onPressed: () async { - await showBlurDialog( - context: context, - builder: (context) => const DevicePickerDialog(), - routeSettings: const RouteSettings(name: 'device_picker'), - ); - }, - ); - } -} diff --git a/lib/app/views/device_picker.dart b/lib/app/views/device_picker.dart index 2a01b491..6a7a2974 100644 --- a/lib/app/views/device_picker.dart +++ b/lib/app/views/device_picker.dart @@ -24,6 +24,7 @@ import '../../management/models.dart'; import '../models.dart'; import '../state.dart'; import 'device_avatar.dart'; +import 'keys.dart' as keys; final _hiddenDevicesProvider = StateNotifierProvider<_HiddenDevicesNotifier, List>( @@ -337,6 +338,7 @@ _DeviceRow _buildCurrentDeviceRow( final subtitle = messages.join('\n'); return _DeviceRow( + key: keys.deviceInfoListTile, leading: data.maybeWhen( data: (data) => DeviceAvatar.yubiKeyData(data, radius: extended ? null : 16), diff --git a/lib/app/views/device_picker_dialog.dart b/lib/app/views/device_picker_dialog.dart deleted file mode 100755 index 454d97c7..00000000 --- a/lib/app/views/device_picker_dialog.dart +++ /dev/null @@ -1,362 +0,0 @@ -/* - * Copyright (C) 2022 Yubico. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import 'package:flutter/material.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:shared_preferences/shared_preferences.dart'; -import 'package:flutter_gen/gen_l10n/app_localizations.dart'; - -import '../../core/state.dart'; -import '../../management/models.dart'; -import '../models.dart'; -import '../state.dart'; -import 'device_avatar.dart'; -import 'keys.dart'; - -final _hiddenDevicesProvider = - StateNotifierProvider<_HiddenDevicesNotifier, List>( - (ref) => _HiddenDevicesNotifier(ref.watch(prefProvider))); - -class _HiddenDevicesNotifier extends StateNotifier> { - static const String _key = 'DEVICE_PICKER_HIDDEN'; - final SharedPreferences _prefs; - _HiddenDevicesNotifier(this._prefs) : super(_prefs.getStringList(_key) ?? []); - - void showAll() { - state = []; - _prefs.setStringList(_key, state); - } - - void hideDevice(DevicePath devicePath) { - state = [...state, devicePath.key]; - _prefs.setStringList(_key, state); - } -} - -class DevicePickerDialog extends StatefulWidget { - const DevicePickerDialog({super.key}); - - @override - State createState() => _DevicePickerDialogState(); -} - -class _DevicePickerDialogState extends State { - late FocusScopeNode _focus; - - @override - void initState() { - super.initState(); - _focus = FocusScopeNode(); - } - - @override - void dispose() { - _focus.dispose(); - super.dispose(); - } - - @override - Widget build(BuildContext context) { - // This keeps the focus in the dialog, even if the underlying page - // changes as it does when a new device is selected. - return FocusScope( - node: _focus, - autofocus: true, - onFocusChange: (focused) { - if (!focused) { - _focus.requestFocus(); - } - }, - child: const _DevicePickerContent(), - ); - } -} - -class _DevicePickerContent extends ConsumerWidget { - const _DevicePickerContent(); - - @override - Widget build(BuildContext context, WidgetRef ref) { - final l10n = AppLocalizations.of(context)!; - final hidden = ref.watch(_hiddenDevicesProvider); - final devices = ref - .watch(attachedDevicesProvider) - .where((e) => !hidden.contains(e.path.key)) - .toList(); - final currentNode = ref.watch(currentDeviceProvider); - - final Widget hero; - final bool showUsb; - if (currentNode != null) { - showUsb = isDesktop && devices.whereType().isEmpty; - devices.removeWhere((e) => e.path == currentNode.path); - hero = _CurrentDeviceRow( - currentNode, - ref.watch(currentDeviceDataProvider), - ); - } else { - hero = Column( - children: [ - _HeroAvatar( - child: DeviceAvatar( - radius: 64, - child: Icon(isAndroid ? Icons.no_cell : Icons.usb), - ), - ), - ListTile( - title: Center(child: Text(l10n.l_no_yk_present)), - subtitle: Center( - child: Text(isAndroid ? l10n.l_insert_or_tap_yk : l10n.s_usb)), - ), - ], - ); - showUsb = false; - } - - List others = [ - if (showUsb) - ListTile( - leading: const Padding( - padding: EdgeInsets.symmetric(horizontal: 4), - child: DeviceAvatar(child: Icon(Icons.usb)), - ), - title: Text(l10n.s_usb), - subtitle: Text(l10n.l_no_yk_present), - onTap: () { - ref.read(currentDeviceProvider.notifier).setCurrentDevice(null); - }, - ), - ...devices.map( - (e) => e.map( - usbYubiKey: (node) => _DeviceRow(node, info: node.info), - nfcReader: (node) => _NfcDeviceRow(node), - ), - ), - ]; - - return GestureDetector( - onSecondaryTapDown: hidden.isEmpty - ? null - : (details) { - showMenu( - context: context, - position: RelativeRect.fromLTRB( - details.globalPosition.dx, - details.globalPosition.dy, - details.globalPosition.dx, - 0, - ), - items: [ - PopupMenuItem( - onTap: () { - ref.read(_hiddenDevicesProvider.notifier).showAll(); - }, - child: ListTile( - title: Text(l10n.s_show_hidden_devices), - dense: true, - contentPadding: EdgeInsets.zero, - ), - ), - ], - ); - }, - child: SimpleDialog( - children: [ - hero, - if (others.isNotEmpty) - const Padding( - padding: EdgeInsets.symmetric(horizontal: 24), - child: Divider(), - ), - ...others, - ], - ), - ); - } -} - -String _getDeviceInfoString(BuildContext context, DeviceInfo info) { - final l10n = AppLocalizations.of(context)!; - final serial = info.serial; - return [ - if (serial != null) l10n.s_sn_serial(serial), - if (info.version.isAtLeast(1)) - l10n.s_fw_version(info.version) - else - l10n.s_unknown_type, - ].join(' '); -} - -List _getDeviceStrings( - BuildContext context, DeviceNode node, AsyncValue data) { - final l10n = AppLocalizations.of(context)!; - final messages = data.whenOrNull( - data: (data) => [data.name, _getDeviceInfoString(context, data.info)], - error: (error, _) => switch (error) { - 'device-inaccessible' => [node.name, l10n.s_yk_inaccessible], - 'unknown-device' => [l10n.s_unknown_device], - _ => null, - }, - ) ?? - [l10n.l_no_yk_present]; - - // Add the NFC reader name, unless it's already included (as device name, like on Android) - if (node is NfcReaderNode && !messages.contains(node.name)) { - messages.add(node.name); - } - - return messages; -} - -class _HeroAvatar extends StatelessWidget { - final Widget child; - const _HeroAvatar({required this.child}); - - @override - Widget build(BuildContext context) { - final theme = Theme.of(context); - return Container( - decoration: BoxDecoration( - shape: BoxShape.circle, - gradient: RadialGradient( - colors: [ - theme.colorScheme.inverseSurface.withOpacity(0.6), - theme.colorScheme.inverseSurface.withOpacity(0.25), - (DialogTheme.of(context).backgroundColor ?? - theme.dialogBackgroundColor) - .withOpacity(0), - ], - ), - ), - padding: const EdgeInsets.all(12), - child: Theme( - // Give the avatar a transparent background - data: theme.copyWith( - colorScheme: - theme.colorScheme.copyWith(surfaceVariant: Colors.transparent)), - child: child, - ), - ); - } -} - -class _CurrentDeviceRow extends StatelessWidget { - final DeviceNode node; - final AsyncValue data; - - const _CurrentDeviceRow(this.node, this.data); - - @override - Widget build(BuildContext context) { - final hero = data.maybeWhen( - data: (data) => DeviceAvatar.yubiKeyData(data, radius: 64), - orElse: () => DeviceAvatar.deviceNode(node, radius: 64), - ); - final messages = _getDeviceStrings(context, node, data); - - return Column( - children: [ - _HeroAvatar(child: hero), - ListTile( - key: deviceInfoListTile, - title: Text(messages.removeAt(0), textAlign: TextAlign.center), - isThreeLine: messages.length > 1, - subtitle: Text(messages.join('\n'), textAlign: TextAlign.center), - ) - ], - ); - } -} - -class _DeviceRow extends ConsumerWidget { - final DeviceNode node; - final DeviceInfo? info; - - const _DeviceRow(this.node, {this.info}); - - @override - Widget build(BuildContext context, WidgetRef ref) { - final l10n = AppLocalizations.of(context)!; - return ListTile( - leading: Padding( - padding: const EdgeInsets.symmetric(horizontal: 4), - child: DeviceAvatar.deviceNode(node), - ), - title: Text(node.name), - subtitle: Text( - node.when( - usbYubiKey: (_, __, ___, info) => info == null - ? l10n.s_yk_inaccessible - : _getDeviceInfoString(context, info), - nfcReader: (_, __) => l10n.s_select_to_scan, - ), - ), - onTap: () { - ref.read(currentDeviceProvider.notifier).setCurrentDevice(node); - }, - ); - } -} - -class _NfcDeviceRow extends ConsumerWidget { - final DeviceNode node; - - const _NfcDeviceRow(this.node); - - @override - Widget build(BuildContext context, WidgetRef ref) { - final l10n = AppLocalizations.of(context)!; - final hidden = ref.watch(_hiddenDevicesProvider); - return GestureDetector( - onSecondaryTapDown: (details) { - showMenu( - context: context, - position: RelativeRect.fromLTRB( - details.globalPosition.dx, - details.globalPosition.dy, - details.globalPosition.dx, - 0, - ), - items: [ - PopupMenuItem( - enabled: hidden.isNotEmpty, - onTap: () { - ref.read(_hiddenDevicesProvider.notifier).showAll(); - }, - child: ListTile( - title: Text(l10n.s_show_hidden_devices), - dense: true, - contentPadding: EdgeInsets.zero, - enabled: hidden.isNotEmpty, - ), - ), - PopupMenuItem( - onTap: () { - ref.read(_hiddenDevicesProvider.notifier).hideDevice(node.path); - }, - child: ListTile( - title: Text(l10n.s_hide_device), - dense: true, - contentPadding: EdgeInsets.zero, - ), - ), - ], - ); - }, - child: _DeviceRow(node), - ); - } -} diff --git a/lib/app/views/fs_dialog.dart b/lib/app/views/fs_dialog.dart index 0aaa45de..30d24928 100644 --- a/lib/app/views/fs_dialog.dart +++ b/lib/app/views/fs_dialog.dart @@ -16,6 +16,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_gen/gen_l10n/app_localizations.dart'; +import 'keys.dart' as keys; class FsDialog extends StatelessWidget { final Widget child; @@ -35,6 +36,7 @@ class FsDialog extends StatelessWidget { Padding( padding: const EdgeInsets.only(bottom: 16.0), child: TextButton.icon( + key: keys.closeButton, icon: const Icon(Icons.close), label: Text(l10n.s_close), onPressed: () { diff --git a/lib/app/views/keys.dart b/lib/app/views/keys.dart index b69b9d2b..06c29fb1 100644 --- a/lib/app/views/keys.dart +++ b/lib/app/views/keys.dart @@ -30,3 +30,6 @@ const managementAppDrawer = Key('$_prefix.drawer.management'); // settings page const themeModeSetting = Key('$_prefix.settings.theme_mode'); Key themeModeOption(ThemeMode mode) => Key('$_prefix.theme_mode.${mode.name}'); + +// misc buttons +const closeButton = Key('$_prefix.close_button');