From 0bb41ae3541762be7baab2ded3c66fd4c6732b53 Mon Sep 17 00:00:00 2001 From: kyy Date: Fri, 20 Mar 2026 09:57:39 +0900 Subject: [PATCH] =?UTF-8?q?=ED=94=84=EB=A1=9C=ED=95=84=20=EC=9E=90?= =?UTF-8?q?=EB=8F=99=EC=A0=80=EC=9E=A5=20=EC=A0=9C=EA=B1=B0=20=EB=B0=8F=20?= =?UTF-8?q?=EB=AA=85=EC=8B=9C=EC=A0=81=20=EC=A0=80=EC=9E=A5=20UX=20?= =?UTF-8?q?=EA=B0=9C=EC=84=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../presentation/pages/profile_page.dart | 274 ++++++------------ .../test/profile_page_edit_flow_test.dart | 120 ++++++++ 2 files changed, 210 insertions(+), 184 deletions(-) create mode 100644 userfront/test/profile_page_edit_flow_test.dart diff --git a/userfront/lib/features/profile/presentation/pages/profile_page.dart b/userfront/lib/features/profile/presentation/pages/profile_page.dart index dd741004..705b9752 100644 --- a/userfront/lib/features/profile/presentation/pages/profile_page.dart +++ b/userfront/lib/features/profile/presentation/pages/profile_page.dart @@ -38,12 +38,8 @@ class _ProfilePageState extends ConsumerState { final FocusNode _departmentFocus = FocusNode(); final FocusNode _phoneFocus = FocusNode(); final FocusNode _phoneCodeFocus = FocusNode(); - bool _nameTouched = false; - bool _departmentTouched = false; - bool _phoneTouched = false; - bool _phoneCodeTouched = false; bool _isSavingField = false; - String? _skipAutoSaveField; + String? _fieldSaveError; String _initialPhone = ''; bool _isPhoneChanged = false; @@ -61,10 +57,6 @@ class _ProfilePageState extends ConsumerState { @override void initState() { super.initState(); - _nameFocus.addListener(_onNameFocusChange); - _departmentFocus.addListener(_onDepartmentFocusChange); - _phoneFocus.addListener(_onPhoneFocusChange); - _phoneCodeFocus.addListener(_onPhoneCodeFocusChange); } void _debugLog( @@ -83,63 +75,6 @@ class _ProfilePageState extends ConsumerState { _log.fine(parts.join(' ')); } - void _onNameFocusChange() { - if (!mounted) return; - if (!_nameFocus.hasFocus && _nameTouched) { - Future.microtask(() { - if (!mounted) return; - final profile = ref.read(profileProvider).value ?? _cachedProfile; - if (profile != null) _autoSaveIfEditing(profile, 'name'); - }); - } else if (_nameFocus.hasFocus) { - _nameTouched = true; - } - } - - void _onDepartmentFocusChange() { - if (!mounted) return; - _debugLog( - 'department_focus_change', - field: 'department', - hasFocus: _departmentFocus.hasFocus, - ); - if (!_departmentFocus.hasFocus && _departmentTouched) { - Future.microtask(() { - if (!mounted) return; - final profile = ref.read(profileProvider).value ?? _cachedProfile; - if (profile != null) _autoSaveIfEditing(profile, 'department'); - }); - } else if (_departmentFocus.hasFocus) { - _departmentTouched = true; - } - } - - void _onPhoneFocusChange() { - if (!mounted) return; - if (!_phoneFocus.hasFocus && _phoneTouched) { - Future.microtask(() { - if (!mounted) return; - final profile = ref.read(profileProvider).value ?? _cachedProfile; - if (profile != null) _handlePhoneFocusChange(profile); - }); - } else if (_phoneFocus.hasFocus) { - _phoneTouched = true; - } - } - - void _onPhoneCodeFocusChange() { - if (!mounted) return; - if (!_phoneCodeFocus.hasFocus && _phoneCodeTouched) { - Future.microtask(() { - if (!mounted) return; - final profile = ref.read(profileProvider).value ?? _cachedProfile; - if (profile != null) _handlePhoneFocusChange(profile); - }); - } else if (_phoneCodeFocus.hasFocus) { - _phoneCodeTouched = true; - } - } - @override void dispose() { _nameController?.dispose(); @@ -210,14 +145,13 @@ class _ProfilePageState extends ConsumerState { _isCodeSent = false; _isVerifying = false; _codeController?.clear(); - _phoneTouched = false; - _phoneCodeTouched = false; } void _startEditing(String field, UserProfile profile) { _debugLog('start_editing', field: field); setState(() { _editingField = field; + _fieldSaveError = null; if (field == 'name') { _nameController?.text = profile.name; } else if (field == 'department') { @@ -252,8 +186,7 @@ class _ProfilePageState extends ConsumerState { _resetPhoneState(); } _editingField = null; - _nameTouched = false; - _departmentTouched = false; + _fieldSaveError = null; }); } @@ -307,9 +240,6 @@ class _ProfilePageState extends ConsumerState { SnackBar(content: Text(tr('msg.userfront.profile.phone.verified'))), ); } - if (_editingField == 'phone') { - await _saveField(profile); - } } catch (e) { setState(() => _isVerifying = false); if (mounted) { @@ -389,64 +319,6 @@ class _ProfilePageState extends ConsumerState { } } - void _autoSaveIfEditing(UserProfile profile, String field) { - if (_editingField != field) return; - if (_skipAutoSaveField == field) { - _debugLog('autosave_skip', field: field, reason: 'skip_flag'); - _skipAutoSaveField = null; - return; - } - if (_isVerifying) { - _debugLog('autosave_skip', field: field, reason: 'verifying'); - return; - } - if (_isSavingField) { - _debugLog('autosave_skip', field: field, reason: 'saving_in_flight'); - return; - } - if (!_hasFieldChanged(profile, field)) { - _debugLog( - 'autosave_skip', - field: field, - reason: 'unchanged', - changed: false, - ); - setState(() { - if (field == 'phone') { - _resetPhoneState(); - } - _editingField = null; - if (field == 'name') { - _nameTouched = false; - } else if (field == 'department') { - _departmentTouched = false; - } - }); - return; - } - _debugLog('autosave_trigger', field: field, changed: true); - _saveField(profile); - } - - void _handlePhoneFocusChange(UserProfile profile) { - if (_editingField != 'phone') return; - if (_skipAutoSaveField == 'phone') { - _skipAutoSaveField = null; - return; - } - if (_isVerifying) return; - if (_isSavingField) return; - if (_phoneFocus.hasFocus || _phoneCodeFocus.hasFocus) return; - if (!_hasFieldChanged(profile, 'phone')) { - setState(() { - _resetPhoneState(); - _editingField = null; - }); - return; - } - _saveField(profile); - } - bool _hasFieldChanged(UserProfile profile, String field) { if (field == 'name') { return (_nameController?.text.trim() ?? '') != profile.name; @@ -466,6 +338,11 @@ class _ProfilePageState extends ConsumerState { _debugLog('save_skip', reason: 'saving_in_flight'); return; } + + setState(() { + _fieldSaveError = null; + }); + final currentField = _editingField!; final nextName = currentField == 'name' @@ -482,26 +359,24 @@ class _ProfilePageState extends ConsumerState { if (currentField == 'name' && nextName.isEmpty) { _debugLog('save_skip', field: currentField, reason: 'empty_name'); - ScaffoldMessenger.of(context).showSnackBar( - SnackBar(content: Text(tr('msg.userfront.profile.name_required'))), - ); + setState(() { + _fieldSaveError = tr('msg.userfront.profile.name_required'); + }); return; } if (currentField == 'department' && nextDepartment.isEmpty) { _debugLog('save_skip', field: currentField, reason: 'empty_department'); - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text(tr('msg.userfront.profile.department_required')), - ), - ); + setState(() { + _fieldSaveError = tr('msg.userfront.profile.department_required'); + }); return; } if (currentField == 'phone') { if (nextPhone.isEmpty) { _debugLog('save_skip', field: currentField, reason: 'empty_phone'); - ScaffoldMessenger.of(context).showSnackBar( - SnackBar(content: Text(tr('msg.userfront.profile.phone_required'))), - ); + setState(() { + _fieldSaveError = tr('msg.userfront.profile.phone_required'); + }); return; } if (_isPhoneChanged && !_isPhoneVerified) { @@ -510,11 +385,9 @@ class _ProfilePageState extends ConsumerState { field: currentField, reason: 'phone_not_verified', ); - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text(tr('msg.userfront.profile.phone_verify_required')), - ), - ); + setState(() { + _fieldSaveError = tr('msg.userfront.profile.phone_verify_required'); + }); return; } } @@ -531,13 +404,14 @@ class _ProfilePageState extends ConsumerState { _resetPhoneState(); } _editingField = null; - _nameTouched = false; - _departmentTouched = false; }); return; } - _isSavingField = true; + setState(() { + _isSavingField = true; + }); + _debugLog('save_dispatch', field: currentField, changed: true); try { @@ -555,8 +429,6 @@ class _ProfilePageState extends ConsumerState { _resetPhoneState(); } _editingField = null; - _nameTouched = false; - _departmentTouched = false; }); _debugLog('save_success', field: currentField); ScaffoldMessenger.of(context).showSnackBar( @@ -566,19 +438,19 @@ class _ProfilePageState extends ConsumerState { } catch (e) { _debugLog('save_failed', field: currentField, reason: e.toString()); if (mounted) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text( - tr( - 'msg.userfront.profile.update_failed', - params: {'error': e.toString()}, - ), - ), - ), - ); + setState(() { + _fieldSaveError = tr( + 'msg.userfront.profile.update_failed', + params: {'error': e.toString().replaceFirst('Exception: ', '')}, + ); + }); } } finally { - _isSavingField = false; + if (mounted) { + setState(() { + _isSavingField = false; + }); + } } } @@ -793,13 +665,15 @@ class _ProfilePageState extends ConsumerState { ); } + final hasChanged = _hasFieldChanged(profile, field); + return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ Text(label, style: const TextStyle(fontWeight: FontWeight.w600)), const SizedBox(height: 8), Row( - crossAxisAlignment: CrossAxisAlignment.end, + crossAxisAlignment: CrossAxisAlignment.start, children: [ Expanded( child: TextField( @@ -807,23 +681,38 @@ class _ProfilePageState extends ConsumerState { controller: controller, focusNode: field == 'name' ? _nameFocus : _departmentFocus, textInputAction: TextInputAction.done, - onSubmitted: (_) => _autoSaveIfEditing(profile, field), + onSubmitted: (_) => _saveField(profile), + onChanged: (_) { + setState(() { + _fieldSaveError = null; + }); + }, decoration: InputDecoration( border: const OutlineInputBorder(), hintText: label, + errorText: _fieldSaveError, ), ), ), const SizedBox(width: 12), - Listener( - onPointerDown: (_) { - _skipAutoSaveField = field; - }, - child: OutlinedButton( - key: Key('profile-$field-cancel-button'), - onPressed: isUpdating ? null : () => _cancelEditing(profile), - child: Text(tr('ui.common.cancel')), - ), + ElevatedButton( + key: Key('profile-$field-save-button'), + onPressed: isUpdating || !hasChanged || _isSavingField + ? null + : () => _saveField(profile), + child: _isSavingField + ? const SizedBox( + width: 16, + height: 16, + child: CircularProgressIndicator(strokeWidth: 2), + ) + : Text(tr('ui.common.save')), + ), + const SizedBox(width: 8), + OutlinedButton( + key: Key('profile-$field-cancel-button'), + onPressed: isUpdating || _isSavingField ? null : () => _cancelEditing(profile), + child: Text(tr('ui.common.cancel')), ), ], ), @@ -847,6 +736,9 @@ class _ProfilePageState extends ConsumerState { ); } + final hasChanged = _hasFieldChanged(profile, 'phone'); + final canSave = hasChanged && (!_isPhoneChanged || _isPhoneVerified); + return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ @@ -856,7 +748,7 @@ class _ProfilePageState extends ConsumerState { ), const SizedBox(height: 8), Row( - crossAxisAlignment: CrossAxisAlignment.end, + crossAxisAlignment: CrossAxisAlignment.start, children: [ Expanded( child: TextField( @@ -864,10 +756,16 @@ class _ProfilePageState extends ConsumerState { focusNode: _phoneFocus, keyboardType: TextInputType.phone, textInputAction: TextInputAction.done, - onSubmitted: (_) => _autoSaveIfEditing(profile, 'phone'), + onSubmitted: (_) => _saveField(profile), + onChanged: (_) { + setState(() { + _fieldSaveError = null; + }); + }, decoration: InputDecoration( border: const OutlineInputBorder(), hintText: '01012345678', + errorText: _fieldSaveError, suffixIcon: _isPhoneVerified ? const Icon(Icons.check_circle, color: Colors.green) : null, @@ -886,14 +784,22 @@ class _ProfilePageState extends ConsumerState { ), ), const SizedBox(width: 8), - Listener( - onPointerDown: (_) { - _skipAutoSaveField = 'phone'; - }, - child: OutlinedButton( - onPressed: isUpdating ? null : () => _cancelEditing(profile), - child: Text(tr('ui.common.cancel')), - ), + ElevatedButton( + onPressed: isUpdating || !canSave || _isSavingField + ? null + : () => _saveField(profile), + child: _isSavingField + ? const SizedBox( + width: 16, + height: 16, + child: CircularProgressIndicator(strokeWidth: 2), + ) + : Text(tr('ui.common.save')), + ), + const SizedBox(width: 8), + OutlinedButton( + onPressed: isUpdating || _isSavingField ? null : () => _cancelEditing(profile), + child: Text(tr('ui.common.cancel')), ), ], ), diff --git a/userfront/test/profile_page_edit_flow_test.dart b/userfront/test/profile_page_edit_flow_test.dart new file mode 100644 index 00000000..0abfe01f --- /dev/null +++ b/userfront/test/profile_page_edit_flow_test.dart @@ -0,0 +1,120 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:userfront/features/profile/data/models/user_profile_model.dart'; +import 'package:userfront/features/profile/domain/notifiers/profile_notifier.dart'; +import 'package:userfront/features/profile/presentation/pages/profile_page.dart'; + +// Mocking the profile notifier +class MockProfileNotifier extends ProfileNotifier { + UserProfile? _profile; + bool updateCalled = false; + String? updatedName; + + @override + Future build() async { + _profile = UserProfile( + id: 'test-id', + email: 'test@example.com', + name: 'Original Name', + phone: '01012345678', + department: 'Dev', + affiliationType: 'employee', + companyCode: 'C100', + ); + return _profile; + } + + @override + Future loadProfile() async { + state = const AsyncValue.loading(); + state = AsyncValue.data(_profile); + return _profile; + } + + @override + Future updateProfile({String? name, String? phone, String? department}) async { + updateCalled = true; + updatedName = name; + _profile = _profile!.copyWith( + name: name ?? _profile!.name, + phone: phone ?? _profile!.phone, + department: department ?? _profile!.department, + ); + state = AsyncValue.data(_profile); + } +} + +void main() { + testWidgets('ProfilePage explicit save button UX flow (Edit -> Cancel -> Edit -> Save)', (tester) async { + final recordedErrors = []; + final previousOnError = FlutterError.onError; + FlutterError.onError = (details) { + final text = details.exceptionAsString(); + if (text.contains('A RenderFlex overflowed')) { + return; + } + recordedErrors.add(details); + }; + addTearDown(() { + FlutterError.onError = previousOnError; + }); + + tester.view.physicalSize = const Size(1920, 1080); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + + final mockNotifier = MockProfileNotifier(); + + await tester.pumpWidget( + ProviderScope( + overrides: [ + profileProvider.overrideWith(() => mockNotifier), + ], + child: const MaterialApp( + home: Scaffold(body: ProfilePage()), + ), + ), + ); + + await tester.pumpAndSettle(); + + // 1. Entering edit mode + final editButton = find.byKey(const Key('profile-name-edit-button')); + expect(editButton, findsOneWidget); + await tester.tap(editButton); + await tester.pumpAndSettle(); + + final inputField = find.byKey(const Key('profile-name-input')); + expect(inputField, findsOneWidget); + + // 2. Testing cancel flow + await tester.enterText(inputField, 'Changed Name'); + await tester.pumpAndSettle(); + + final cancelButton = find.byKey(const Key('profile-name-cancel-button')); + await tester.tap(cancelButton); + await tester.pumpAndSettle(); + + // After cancellation, the field should be read-only again. + expect(find.byKey(const Key('profile-name-input')), findsNothing); + // Find text could be part of ListTile + expect(find.text('Original Name'), findsWidgets); + + // 3. Re-enter edit mode and explicitly save + await tester.tap(find.byKey(const Key('profile-name-edit-button'))); + await tester.pumpAndSettle(); + + await tester.enterText(find.byKey(const Key('profile-name-input')), 'Saved Name'); + await tester.pumpAndSettle(); + + final saveButton = find.byKey(const Key('profile-name-save-button')); + await tester.tap(saveButton); + await tester.pumpAndSettle(); + + // Verify the mock received the update + expect(mockNotifier.updateCalled, isTrue); + expect(mockNotifier.updatedName, 'Saved Name'); + }); +}