fix(tags): add tag creation in select dialog#230
Conversation
…management - Add INotificationService dependency and widgets binding observer - Implement app lifecycle state handling for permission checks - Add permission status checking on dialog initialization and resume - Update button styling based on permission status - Allow back navigation when on first page only - Refresh permissions after dialog closes
…in select dialog Adds a new create button translation key for multiple languages and implements the functionality to create new tags directly from the tag selection dialog. The dialog now shows a create option when searching for non-existent tags, allowing users to create and select tags in one flow.
Reviewer's GuideEnables creating tags directly from the tag selection dialog, improves onboarding permissions handling/UX, ensures system tray initialization on mobile, and adds localized copy for the new tag-create action in all supported languages. Sequence diagram for creating a new tag from TagSelectDialogsequenceDiagram
actor User
participant TagSelectDialogState
participant AsyncErrorHandler
participant Mediator
participant SaveTagCommand
participant TagStore
User->>TagSelectDialogState: Type search text
TagSelectDialogState->>TagSelectDialogState: Evaluate _shouldShowCreateOption
TagSelectDialogState-->>User: Show Create_tag_option
User->>TagSelectDialogState: Tap Create_tag_option
TagSelectDialogState->>AsyncErrorHandler: execute(operation: _createAndSelectTag)
AsyncErrorHandler->>TagSelectDialogState: Invoke operation callback
TagSelectDialogState->>SaveTagCommand: Create instance(name: searchText)
TagSelectDialogState->>Mediator: send(command: SaveTagCommand)
Mediator->>TagStore: Handle SaveTagCommand
TagStore-->>Mediator: SaveTagCommandResponse(id)
Mediator-->>AsyncErrorHandler: SaveTagCommandResponse
AsyncErrorHandler-->>TagSelectDialogState: onSuccess(result)
TagSelectDialogState->>TagSelectDialogState: setState(add result.id to _tempSelectedTags)
TagSelectDialogState->>TagSelectDialogState: Clear _searchController
TagSelectDialogState->>TagSelectDialogState: Set _tags = null
TagSelectDialogState->>TagSelectDialogState: _getTags(pageIndex: 0)
TagSelectDialogState-->>User: Updated tag list with new tag selected
Updated class diagram for tag selection, onboarding, and platform initializationclassDiagram
class TagSelectDialogState {
- ScrollController _scrollController
- TextEditingController _searchController
- dynamic _tags
- Set~String~ _tempSelectedTags
- bool _hasExplicitlySelectedNone
+ Widget build(BuildContext context)
+ Future _getTags(int pageIndex)
+ bool get _shouldShowCreateOption
+ Future _createAndSelectTag()
}
class AsyncErrorHandler {
+ Future execute(BuildContext context, String errorMessage, Future operation(), void onSuccess(dynamic result))
}
class Mediator {
+ Future send(SaveTagCommand command)
}
class SaveTagCommand {
+ String name
+ SaveTagCommand(String name)
}
class SaveTagCommandResponse {
+ String id
}
TagSelectDialogState --> AsyncErrorHandler : uses
TagSelectDialogState --> Mediator : sends_commands
TagSelectDialogState --> SaveTagCommand : creates
Mediator --> SaveTagCommandResponse : returns
class OnboardingDialogState {
- ITranslationService _translationService
- IThemeService _themeService
- Mediator _mediator
- INotificationService _notificationService
- PageController _pageController
- int _currentPage
- String _selectedLanguageCode
- bool _permissionsReviewed
- List~_OnboardingStep~ _steps
+ void initState()
+ void dispose()
+ void didChangeAppLifecycleState(AppLifecycleState state)
+ Future _checkPermissions()
+ List~_OnboardingStep~ _buildSteps()
+ Widget build(BuildContext context)
+ Future _showPermissionsPage(BuildContext context)
+ Future _onLanguageChanged(String languageCode)
}
class INotificationService {
+ Future<bool> checkPermissionStatus()
}
OnboardingDialogState ..|> WidgetsBindingObserver : implements
OnboardingDialogState --> INotificationService : uses
class PlatformInitializationService {
+ Future initializeMobile(dynamic container)
+ Future _initializeAndroidServerMode(dynamic container)
+ Future _initializeSystemTray(dynamic container)
}
PlatformInitializationService --> SystemTrayComponent : initializes
class SystemTrayComponent {
+ void setupNotificationChannels()
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @ahmet-cetinkaya, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant quality-of-life improvement by integrating tag creation directly into the tag selection dialog. This change streamlines the user workflow, allowing users to create and select new tags without interrupting their current task, habit, or note editing process, thereby enhancing efficiency and reducing friction. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
TagTranslationKeys.createTagButtonkey is defined astags.actions.create_buttonbut the locale files addcreate_buttondirectly undertags, so either the key path or the YAML nesting should be adjusted to match or the translations won’t resolve at runtime. - In
_OnboardingDialogState._checkPermissionsyou rebuild_stepsand callsetStatewhenever the permission status changes; if only the button style/icon depend on_permissionsReviewed, consider deriving that directly in the build method to avoid regenerating the step list on each permission check.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `TagTranslationKeys.createTagButton` key is defined as `tags.actions.create_button` but the locale files add `create_button` directly under `tags`, so either the key path or the YAML nesting should be adjusted to match or the translations won’t resolve at runtime.
- In `_OnboardingDialogState._checkPermissions` you rebuild `_steps` and call `setState` whenever the permission status changes; if only the button style/icon depend on `_permissionsReviewed`, consider deriving that directly in the build method to avoid regenerating the step list on each permission check.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature allowing users to create tags directly from the selection dialog, which significantly improves the workflow. The implementation is well-structured, especially the refactoring in TagSelectDialog to handle dynamic list items. The accompanying enhancements to the onboarding dialog, such as responsive height and more robust permission checking, are also great additions.
I've found one functional issue in the new tag creation logic that doesn't respect the single-selection mode or the 'None' option state, which I've detailed in a comment. Overall, this is a solid contribution with thoughtful improvements to the user experience.
| setState(() { | ||
| _tempSelectedTags.add(result.id); | ||
| _searchController.clear(); | ||
| _tags = null; // Refresh list to include new tag | ||
| }); |
There was a problem hiding this comment.
When creating a new tag, there are a couple of state-handling issues that should be addressed to ensure consistent behavior with the rest of the dialog:
- Single-select mode: If
isMultiSelectisfalse, creating a new tag should clear any previously selected tag. The current code just adds the new tag, which could lead to multiple selections. - 'None' option state: If the 'None' option was selected (
_hasExplicitlySelectedNoneis true), creating a new tag should set this tofalse.
I've suggested a change to the setState block to handle both of these cases correctly, aligning it with the logic used when selecting existing tags from the list.
setState(() {
if (_hasExplicitlySelectedNone) {
_hasExplicitlySelectedNone = false;
}
if (!widget.isMultiSelect) {
_tempSelectedTags.clear();
}
_tempSelectedTags.add(result.id);
_searchController.clear();
_tags = null; // Refresh list to include new tag
});
ahmet-cetinkaya
left a comment
There was a problem hiding this comment.
PR Review: #230 "fix(tags): add tag creation in select dialog"
Overall Assessment
This PR adds useful functionality for creating tags directly in the selection dialog, which is a great UX improvement. The code generally follows project patterns and all tests pass. However, there are some error handling and code clarity issues that should be addressed.
Medium Issues (Should Fix)
1. Potential silent failure after tag creation
Location: tag_select_dialog.dart:398
The _getTags(pageIndex: 0) call inside the onSuccess callback is not wrapped in error handling. If the list refresh fails, users will be in an inconsistent state (tag created but list empty).
Suggestion: Move _getTags inside the main operation callback for atomicity, or add explicit error handling.
2. Missing error context
Location: tag_select_dialog.dart:384-401
Uses generic unexpectedError message instead of a specific tag creation error. This makes debugging production issues difficult.
Suggestion: Create a specific translation key like TagTranslationKeys.createTagError.
3. Potential race condition with mounted check
Location: tag_select_dialog.dart:398
The _getTags call is outside the mounted check scope:
onSuccess: (result) {
if (mounted) {
setState(() { ... });
_getTags(pageIndex: 0); // Called outside mounted check
}
}Suggestion: Move _getTags inside the mounted check or add a separate check.
4. Complex itemBuilder logic
Location: tag_select_dialog.dart:238-275
The currentIndex tracking approach creates confusing control flow that's hard to follow and maintain.
Suggestion: Consider extracting to separate _build*Tile() methods for better clarity.
Low Issues (Consider Fixing)
5. No client-side tag name validation
Location: tag_select_dialog.dart:382
Only checks for empty string. Adding client-side validation (min/max length, invalid characters) would provide immediate feedback.
6. Case-insensitive matching consistency
Location: tag_select_dialog.dart:377
The toLowerCase() comparison for duplicate detection should be verified to match backend behavior.
Positive Notes ✅
- Proper use of
currentIndexpattern for extensibility - Good separation of concerns with dedicated getter and method
- Consistent use of
AsyncErrorHandlerfollowing project patterns - Proper
mountedchecks beforesetState - Comprehensive translations for all 22 languages
- All 1,745 tests pass
Recommendation
Request Changes - Please address issues #1, #2, and #3 before merging for better robustness. Issues #4-6 are optional improvements that could be handled as follow-up.
Review generated via PR Review Toolkit
Fixes state management issues when creating a new tag from the select dialog: - Clear 'None' option state (_hasExplicitlySelectedNone) when creating a tag - In single-select mode, clear previously selected tags before adding new one - Move _getTags call inside mounted check for better lifecycle safety These changes align the tag creation behavior with the existing tag selection logic, ensuring consistent state management across all selection modes. Addresses review comments from PR #230.
Adds a specific error translation key (createTagError) for tag creation failures in the select dialog, providing better debugging information than the generic unexpectedError message. Changes: - Added createTagError translation key to tag_translation_keys.dart - Added translated error messages to all 22 locale files - Updated AsyncErrorHandler to use the specific error message This addresses review feedback requesting more context-specific error messages for better user experience and production debugging. Addresses review comments from PR #230.
🚀 Motivation and Context
This PR adds the ability to create new tags directly from the tag selection dialog, improving the user workflow by eliminating the need to navigate away from the current context. Previously, users had to leave their task/habit/note editing flow to create a new tag in the tags section.
⚙️ Implementation Details
createTagButtontranslation key with translations for all 22 supported languagesTagSelectDialogto show a "Create tag" option when search text doesn't match existing tags_createAndSelectTag()method that creates the tag and automatically selects it_shouldShowCreateOptiongetter to conditionally show the create option📋 Checklist for Reviewer
🔗 Related
N/A
Summary by Sourcery
Allow creating and selecting new tags directly from the tag selection dialog while improving onboarding permissions handling and mobile initialization.
New Features:
Enhancements: