From 5ad56952791ccc4a6d6102b4428a615d0c6d9122 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Fri, 18 Aug 2023 14:54:55 +0200 Subject: [PATCH] Combine OATH rename dialogs. --- lib/oath/views/account_dialog.dart | 17 +- lib/oath/views/account_view.dart | 16 +- lib/oath/views/add_multi_account_page.dart | 47 +++-- lib/oath/views/rename_account_dialog.dart | 159 +++++++++------- lib/oath/views/rename_list_account.dart | 207 --------------------- 5 files changed, 144 insertions(+), 302 deletions(-) delete mode 100644 lib/oath/views/rename_list_account.dart diff --git a/lib/oath/views/account_dialog.dart b/lib/oath/views/account_dialog.dart index 963994d0..3446a55b 100755 --- a/lib/oath/views/account_dialog.dart +++ b/lib/oath/views/account_dialog.dart @@ -59,15 +59,16 @@ class AccountDialog extends ConsumerWidget { EditIntent: CallbackAction(onInvoke: (_) async { final credentials = ref.read(credentialsProvider); final withContext = ref.read(withContextProvider); - final OathCredential? renamed = + final renamed = await withContext((context) async => await showBlurDialog( - context: context, - builder: (context) => RenameAccountDialog( - node, - credential, - credentials, - ), - )); + context: context, + builder: (context) => RenameAccountDialog.forOathCredential( + ref, + node, + credential, + credentials?.map((e) => (e.issuer, e.name)).toList() ?? + [], + ))); if (renamed != null) { // Replace the dialog with the renamed credential await withContext((context) async { diff --git a/lib/oath/views/account_view.dart b/lib/oath/views/account_view.dart index 6801e168..8beed7dc 100755 --- a/lib/oath/views/account_view.dart +++ b/lib/oath/views/account_view.dart @@ -96,12 +96,16 @@ class _AccountViewState extends ConsumerState { EditIntent: CallbackAction(onInvoke: (_) async { final node = ref.read(currentDeviceProvider)!; final credentials = ref.read(credentialsProvider); - return await ref.read(withContextProvider)( - (context) async => await showBlurDialog( - context: context, - builder: (context) => - RenameAccountDialog(node, credential, credentials), - )); + final withContext = ref.read(withContextProvider); + return await withContext((context) async => await showBlurDialog( + context: context, + builder: (context) => RenameAccountDialog.forOathCredential( + ref, + node, + credential, + credentials?.map((e) => (e.issuer, e.name)).toList() ?? [], + ), + )); }), DeleteIntent: CallbackAction(onInvoke: (_) async { final node = ref.read(currentDeviceProvider)!; diff --git a/lib/oath/views/add_multi_account_page.dart b/lib/oath/views/add_multi_account_page.dart index 12577b49..08280c34 100644 --- a/lib/oath/views/add_multi_account_page.dart +++ b/lib/oath/views/add_multi_account_page.dart @@ -18,7 +18,7 @@ import '../state.dart'; import '../../app/message.dart'; import '../../exception/cancellation_exception.dart'; -import 'rename_list_account.dart'; +import 'rename_account_dialog.dart'; final _log = Logger('oath.views.list_screen'); @@ -91,6 +91,7 @@ class _OathAddMultiAccountPageState secondary: Row(mainAxisSize: MainAxisSize.min, children: [ if (isTouchSupported()) IconButton( + tooltip: l10n.s_require_touch, color: touch ? colorScheme.primary : null, onPressed: unique ? () { @@ -100,30 +101,46 @@ class _OathAddMultiAccountPageState }); } : null, - icon: const Icon(Icons.touch_app_outlined)), + icon: Icon(touch + ? Icons.touch_app + : Icons.touch_app_outlined)), IconButton( + tooltip: l10n.s_rename_account, onPressed: () async { final node = ref .read(currentDeviceDataProvider) .valueOrNull ?.node; final withContext = ref.read(withContextProvider); - CredentialData renamed = await withContext( + CredentialData? renamed = await withContext( (context) async => await showBlurDialog( context: context, - builder: (context) => RenameList(node!, cred, - widget.credentialsFromUri, _credentials), + builder: (context) => RenameAccountDialog( + device: node!, + issuer: cred.issuer, + name: cred.name, + oathType: cred.oathType, + period: cred.period, + existing: (widget.credentialsFromUri ?? []) + .map((e) => (e.issuer, e.name)) + .followedBy((_credentials ?? []) + .map((e) => (e.issuer, e.name))) + .toList(), + rename: (issuer, name) async => cred + .copyWith(issuer: issuer, name: name), + ), )); - - setState(() { - int index = widget.credentialsFromUri!.indexWhere( - (element) => - element.name == cred.name && - (element.issuer == cred.issuer)); - widget.credentialsFromUri![index] = renamed; - _credStates.remove(cred); - _credStates[renamed] = (true, touch, true); - }); + if (renamed != null) { + setState(() { + int index = widget.credentialsFromUri!.indexWhere( + (element) => + element.name == cred.name && + (element.issuer == cred.issuer)); + widget.credentialsFromUri![index] = renamed; + _credStates.remove(cred); + _credStates[renamed] = (true, touch, true); + }); + } }, icon: IconTheme( data: IconTheme.of(context), diff --git a/lib/oath/views/rename_account_dialog.dart b/lib/oath/views/rename_account_dialog.dart index 17cb6432..9d0fede4 100755 --- a/lib/oath/views/rename_account_dialog.dart +++ b/lib/oath/views/rename_account_dialog.dart @@ -22,6 +22,7 @@ import 'package:logging/logging.dart'; import '../../app/logging.dart'; import '../../app/message.dart'; import '../../app/models.dart'; +import '../../app/state.dart'; import '../../exception/cancellation_exception.dart'; import '../../desktop/models.dart'; import '../../widgets/focus_utils.dart'; @@ -36,97 +37,121 @@ final _log = Logger('oath.view.rename_account_dialog'); class RenameAccountDialog extends ConsumerStatefulWidget { final DeviceNode device; - final OathCredential credential; - final List? credentials; + final String? issuer; + final String name; + final OathType oathType; + final int period; + final List<(String? issuer, String name)> existing; + final Future Function(String? issuer, String name) rename; - const RenameAccountDialog(this.device, this.credential, this.credentials, - {super.key}); + const RenameAccountDialog({ + required this.device, + required this.issuer, + required this.name, + required this.oathType, + this.period = defaultPeriod, + this.existing = const [], + required this.rename, + super.key, + }); @override ConsumerState createState() => _RenameAccountDialogState(); + + factory RenameAccountDialog.forOathCredential( + WidgetRef ref, + DeviceNode device, + OathCredential credential, + List<(String? issuer, String name)> existing) { + return RenameAccountDialog( + device: device, + issuer: credential.issuer, + name: credential.name, + oathType: credential.oathType, + period: credential.period, + existing: existing, + rename: (issuer, name) async { + final withContext = ref.read(withContextProvider); + try { + // Rename credentials + final renamed = await ref + .read(credentialListProvider(device.path).notifier) + .renameAccount(credential, issuer, name); + + // Update favorite + ref + .read(favoritesProvider.notifier) + .renameCredential(credential.id, renamed.id); + + await withContext((context) async => showMessage( + context, AppLocalizations.of(context)!.s_account_renamed)); + return renamed; + } on CancellationException catch (_) { + // ignored + } catch (e) { + _log.error('Failed to add account', e); + final String errorMessage; + // TODO: Make this cleaner than importing desktop specific RpcError. + if (e is RpcError) { + errorMessage = e.message; + } else { + errorMessage = e.toString(); + } + await withContext((context) async => showMessage( + context, + AppLocalizations.of(context)! + .l_account_add_failed(errorMessage), + duration: const Duration(seconds: 4), + )); + return null; + } + }, + ); + } } class _RenameAccountDialogState extends ConsumerState { late String _issuer; - late String _account; + late String _name; @override void initState() { super.initState(); - _issuer = widget.credential.issuer?.trim() ?? ''; - _account = widget.credential.name.trim(); + _issuer = widget.issuer?.trim() ?? ''; + _name = widget.name.trim(); } void _submit() async { - final l10n = AppLocalizations.of(context)!; - try { - - FocusUtils.unfocus(context); - - // Rename credentials - final renamed = await ref - .read(credentialListProvider(widget.device.path).notifier) - .renameAccount( - widget.credential, _issuer.isNotEmpty ? _issuer : null, _account); - - // Update favorite - ref - .read(favoritesProvider.notifier) - .renameCredential(widget.credential.id, renamed.id); - - if (!mounted) return; - Navigator.of(context).pop(renamed); - showMessage(context, l10n.s_account_renamed); - } on CancellationException catch (_) { - // ignored - } catch (e) { - _log.error('Failed to add account', e); - final String errorMessage; - // TODO: Make this cleaner than importing desktop specific RpcError. - if (e is RpcError) { - errorMessage = e.message; - } else { - errorMessage = e.toString(); - } - showMessage( - context, - l10n.l_account_add_failed(errorMessage), - duration: const Duration(seconds: 4), - ); - } + FocusUtils.unfocus(context); + final nav = Navigator.of(context); + final renamed = + await widget.rename(_issuer.isNotEmpty ? _issuer : null, _name); + nav.pop(renamed); } @override Widget build(BuildContext context) { final l10n = AppLocalizations.of(context)!; - final credential = widget.credential; final (issuerRemaining, nameRemaining) = getRemainingKeySpace( - oathType: credential.oathType, - period: credential.period, + oathType: widget.oathType, + period: widget.period, issuer: _issuer, - name: _account, + name: _name, ); - // is this credentials name/issuer pair different from all other? - final isUnique = widget.credentials - ?.where((element) => - element != credential && - element.name == _account && - (element.issuer ?? '') == _issuer) - .isEmpty ?? - false; + // are the name/issuer values different from original + final didChange = (widget.issuer ?? '') != _issuer || widget.name != _name; + + // is this credentials name/issuer pair different from all other, or initial value? + final isUnique = !widget.existing.contains((_issuer, _name)) || !didChange; // is this credential name/issuer of valid format - final isValidFormat = _account.isNotEmpty; - - // are the name/issuer values different from original - final didChange = (widget.credential.issuer ?? '') != _issuer || - widget.credential.name != _account; + final nameNotEmpty = _name.isNotEmpty; // can we rename with the new values - final isValid = isUnique && isValidFormat; + final isValid = isUnique && nameNotEmpty; return ResponsiveDialog( title: Text(l10n.s_rename_account), @@ -142,7 +167,9 @@ class _RenameAccountDialogState extends ConsumerState { child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - Text(l10n.q_rename_target(getTextName(credential))), + Text(l10n.q_rename_target(widget.issuer != null + ? '${widget.issuer} (${widget.name})' + : widget.name)), Text(l10n.p_rename_will_change_account_displayed), TextFormField( initialValue: _issuer, @@ -165,16 +192,16 @@ class _RenameAccountDialogState extends ConsumerState { }, ), TextFormField( - initialValue: _account, + initialValue: _name, maxLength: nameRemaining, inputFormatters: [limitBytesLength(nameRemaining)], - buildCounter: buildByteCounterFor(_account), + buildCounter: buildByteCounterFor(_name), key: keys.nameField, decoration: InputDecoration( border: const OutlineInputBorder(), labelText: l10n.s_account_name, helperText: '', // Prevents dialog resizing when disabled - errorText: !isValidFormat + errorText: !nameNotEmpty ? l10n.l_account_name_required : !isUnique ? l10n.l_name_already_exists @@ -184,7 +211,7 @@ class _RenameAccountDialogState extends ConsumerState { textInputAction: TextInputAction.done, onChanged: (value) { setState(() { - _account = value.trim(); + _name = value.trim(); }); }, onFieldSubmitted: (_) { diff --git a/lib/oath/views/rename_list_account.dart b/lib/oath/views/rename_list_account.dart deleted file mode 100644 index 123c3484..00000000 --- a/lib/oath/views/rename_list_account.dart +++ /dev/null @@ -1,207 +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_gen/gen_l10n/app_localizations.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:logging/logging.dart'; - -import '../../app/logging.dart'; -import '../../app/message.dart'; -import '../../app/models.dart'; -import '../../exception/cancellation_exception.dart'; -import '../../desktop/models.dart'; -import '../../widgets/responsive_dialog.dart'; -import '../../widgets/utf8_utils.dart'; -import '../models.dart'; -import '../keys.dart' as keys; -import 'utils.dart'; - -final _log = Logger('oath.view.rename_account_dialog'); - -class RenameList extends ConsumerStatefulWidget { - final DeviceNode device; - final CredentialData credential; - final List? credentialsFromUri; - final List? credentials; - - const RenameList( - this.device, this.credential, this.credentialsFromUri, this.credentials, - {super.key}); - - @override - ConsumerState createState() => _RenameListState(); -} - -class _RenameListState extends ConsumerState { - late String _issuer; - late String _account; - - @override - void initState() { - super.initState(); - _issuer = widget.credential.issuer?.trim() ?? ''; - _account = widget.credential.name.trim(); - } - - void _submit() { - if (!mounted) return; - - final l10n = AppLocalizations.of(context)!; - try { - // Rename credentials - final credential = widget.credential.copyWith( - issuer: _issuer == '' ? null : _issuer, - name: _account, - ); - - Navigator.of(context).pop(credential); - showMessage(context, l10n.s_account_renamed); - } on CancellationException catch (_) { - // ignored - } catch (e) { - _log.error('Failed to add account', e); - final String errorMessage; - // TODO: Make this cleaner than importing desktop specific RpcError. - if (e is RpcError) { - errorMessage = e.message; - } else { - errorMessage = e.toString(); - } - showMessage( - context, - l10n.l_account_add_failed(errorMessage), - duration: const Duration(seconds: 4), - ); - } - } - - @override - Widget build(BuildContext context) { - final l10n = AppLocalizations.of(context)!; - final credential = widget.credential; - - final (issuerRemaining, nameRemaining) = getRemainingKeySpace( - oathType: credential.oathType, - period: credential.period, - issuer: _issuer, - name: _account, - ); - - // is this credential's name/issuer pair different from all other? - final isUniqueFromUri = widget.credentialsFromUri - ?.where((element) => - element != credential && - element.name == _account && - (element.issuer ?? '') == _issuer) - .isEmpty ?? - false; - - final isUniqueFromDevice = widget.credentials - ?.where((element) => - element.name == _account && (element.issuer ?? '') == _issuer) - .isEmpty ?? - false; - - // is this credential's name/issuer of valid format - final isValidFormat = _account.isNotEmpty; - - // are the name/issuer values different from original - final didChange = (widget.credential.issuer ?? '') != _issuer || - widget.credential.name != _account; - - // can we rename with the new values - final isValid = isUniqueFromUri && isUniqueFromDevice && isValidFormat; - - return ResponsiveDialog( - title: Text(l10n.s_rename_account), - actions: [ - TextButton( - onPressed: didChange && isValid ? _submit : null, - key: keys.saveButton, - child: Text(l10n.s_save), - ), - ], - child: Padding( - padding: const EdgeInsets.symmetric(horizontal: 18.0), - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - credential.issuer != null - ? Text(l10n.q_rename_target( - '${credential.issuer} (${credential.name})')) - : Text(l10n.q_rename_target(credential.name)), - Text(l10n.p_rename_will_change_account_displayed), - TextFormField( - initialValue: _issuer, - enabled: issuerRemaining > 0, - maxLength: issuerRemaining > 0 ? issuerRemaining : null, - buildCounter: buildByteCounterFor(_issuer), - inputFormatters: [limitBytesLength(issuerRemaining)], - key: keys.issuerField, - decoration: InputDecoration( - border: const OutlineInputBorder(), - labelText: l10n.s_issuer_optional, - helperText: '', // Prevents dialog resizing when disabled - prefixIcon: const Icon(Icons.business_outlined), - ), - textInputAction: TextInputAction.next, - onChanged: (value) { - setState(() { - _issuer = value.trim(); - }); - }, - ), - TextFormField( - initialValue: _account, - maxLength: nameRemaining, - inputFormatters: [limitBytesLength(nameRemaining)], - buildCounter: buildByteCounterFor(_account), - key: keys.nameField, - decoration: InputDecoration( - border: const OutlineInputBorder(), - labelText: l10n.s_account_name, - helperText: '', // Prevents dialog resizing when disabled - errorText: !isValidFormat - ? l10n.l_account_name_required - : (!isUniqueFromUri || !isUniqueFromDevice) - ? l10n.l_name_already_exists - : null, - prefixIcon: const Icon(Icons.people_alt_outlined), - ), - textInputAction: TextInputAction.done, - onChanged: (value) { - setState(() { - _account = value.trim(); - }); - }, - onFieldSubmitted: (_) { - if (didChange && isValid) { - _submit(); - } - }, - ), - ] - .map((e) => Padding( - padding: const EdgeInsets.symmetric(vertical: 8.0), - child: e, - )) - .toList(), - ), - ), - ); - } -}