Settings revamp#3775
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'Custom Vocabulary' feature, allowing users to add and manage custom words for transcription, and refactors several settings pages for improved UI consistency and functionality. The 'Change Name' widget was redesigned with a custom dialog, loading states, and input trimming. The 'Language Settings' page was simplified by moving custom vocabulary management to its own dedicated page and consolidating primary language selection and automatic translation into a single card. The 'Profile' page was updated to use a new chipValue display for items like Name, Email, and Language, and now includes a 'Data & Privacy' section while moving 'Persona', 'Conversation Timeout', and 'Import Data' to 'Developer Settings'. The 'Integrations' and 'Task Integrations' pages now display shimmer loading effects while fetching data. Review comments highlighted the need for robust error handling in the _executeBatchDeletion method within the new custom vocabulary feature to prevent UI lock-ups, and suggested awaiting the updateGivenName call in the 'Change Name' widget to ensure proper state management and user feedback.
| Future<void> _executeBatchDeletion(UserProvider userProvider) async { | ||
| if (_pendingDeletions.isEmpty) return; | ||
|
|
||
| setState(() { | ||
| _isDeletingBatch = true; | ||
| }); | ||
|
|
||
| final wordsToDelete = List<String>.from(_pendingDeletions); | ||
| bool anySuccess = false; | ||
|
|
||
| for (final word in wordsToDelete) { | ||
| final success = await userProvider.removeVocabularyWord(word); | ||
| if (success) anySuccess = true; | ||
| } | ||
|
|
||
| if (mounted) { | ||
| setState(() { | ||
| _pendingDeletions.clear(); | ||
| _isDeletingBatch = false; | ||
| }); | ||
|
|
||
| if (anySuccess) { | ||
| context.read<CaptureProvider>().onTranscriptionSettingsChanged(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The _executeBatchDeletion method lacks error handling. If any of the userProvider.removeVocabularyWord(word) calls throw an exception, the _isDeletingBatch flag will remain true, and _pendingDeletions will not be cleared. This will leave the UI in a permanently disabled/loading state for the vocabulary section. A try...finally block should be used to ensure the state is reset even if an error occurs.
Future<void> _executeBatchDeletion(UserProvider userProvider) async {
if (_pendingDeletions.isEmpty) return;
setState(() {
_isDeletingBatch = true;
});
final wordsToDelete = List<String>.from(_pendingDeletions);
bool anySuccess = false;
try {
for (final word in wordsToDelete) {
final success = await userProvider.removeVocabularyWord(word);
if (success) anySuccess = true;
}
} finally {
if (mounted) {
setState(() {
_pendingDeletions.clear();
_isDeletingBatch = false;
});
if (anySuccess) {
context.read<CaptureProvider>().onTranscriptionSettingsChanged();
}
}
}
}| : () { | ||
| if (nameController.text.isEmpty || nameController.text.trim().isEmpty) { | ||
| AppSnackbar.showSnackbarError('Name cannot be empty'); | ||
| return; | ||
| } | ||
| setState(() => isSaving = true); | ||
| SharedPreferencesUtil().givenName = nameController.text.trim(); | ||
| AuthService.instance.updateGivenName(nameController.text.trim()); | ||
| AppSnackbar.showSnackbar('Name updated successfully!'); | ||
| Navigator.of(context).pop(); | ||
| }, |
There was a problem hiding this comment.
The updateGivenName call is not awaited. This means the dialog will close and a success message will be shown to the user before the update operation is actually completed on the backend. If the update fails, the user will be misled into thinking it was successful. The onTap callback should be async and await the updateGivenName call. Also, error handling should be added, and isSaving should be reset to false in case of an error.
| : () { | |
| if (nameController.text.isEmpty || nameController.text.trim().isEmpty) { | |
| AppSnackbar.showSnackbarError('Name cannot be empty'); | |
| return; | |
| } | |
| setState(() => isSaving = true); | |
| SharedPreferencesUtil().givenName = nameController.text.trim(); | |
| AuthService.instance.updateGivenName(nameController.text.trim()); | |
| AppSnackbar.showSnackbar('Name updated successfully!'); | |
| Navigator.of(context).pop(); | |
| }, | |
| : () async { | |
| if (nameController.text.trim().isEmpty) { | |
| AppSnackbar.showSnackbarError('Name cannot be empty'); | |
| return; | |
| } | |
| setState(() => isSaving = true); | |
| try { | |
| final newName = nameController.text.trim(); | |
| await AuthService.instance.updateGivenName(newName); | |
| SharedPreferencesUtil().givenName = newName; | |
| AppSnackbar.showSnackbar('Name updated successfully!'); | |
| if (mounted) Navigator.of(context).pop(); | |
| } catch (e) { | |
| AppSnackbar.showSnackbarError('Failed to update name. Please try again.'); | |
| if (mounted) { | |
| setState(() => isSaving = false); | |
| } | |
| } | |
| }, |
No description provided.