fix: Inline validation for empty and duplicate tags in Tag Editor #586
Conversation
…d persist tags on task creation
There was a problem hiding this comment.
Pull request overview
Adds inline validation to the task tag editor dialog to prevent empty/duplicate tags and allow comma-separated input, aligning the UX with issue #518.
Changes:
- Added comma-separated parsing and multi-tag add support (
_parseTags,_addTags). - Added inline form validation for empty input, spaces, and duplicates.
- Updated tag selection chips to use the new multi-add path.
Comments suppressed due to low confidence (1)
lib/app/modules/detailRoute/views/tags_widget.dart:293
- After moving validation into the
TextFormField.validator, thetry { ... } on FormatExceptionblock no longer appears to protect any code that can throw (and will likely leavevalidate.dart/loggyimports unused). Consider removing thetry/catchhere and cleaning up any now-unused imports to keepflutter analyzeclean.
if (formKey.currentState!.validate()) {
try {
final tags = _parseTags(controller.text);
_addTags(tags);
Get.back();
} on FormatException catch (e, trace) {
logError(e, trace);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| validateTaskTags(controller.text); | ||
| _addTag(controller.text); | ||
| // Navigator.of(context).pop(); | ||
| final tags = _parseTags(controller.text); | ||
| _addTags(tags); | ||
| Get.back(); | ||
| } on FormatException catch (e, trace) { |
There was a problem hiding this comment.
This change introduces new validation behavior (empty input, duplicates, comma-separated submission, and keeping the dialog open on invalid submit), but there are no widget tests covering it. Please add a widget test for TagsRoute that asserts inline errors are shown and that the dialog only closes when a valid tag (or tag list) is submitted.
| draftTags ??= ListBuilder<String>(); | ||
|
|
||
| for (final tag in tags) { | ||
| if (!draftTags!.build().contains(tag)) { |
There was a problem hiding this comment.
In _addTags, calling draftTags!.build() inside the loop allocates a new immutable list on every iteration. Use draftTags!.contains(tag) (ListBuilder supports contains) or build once before the loop / keep a Set for membership checks to avoid repeated allocations and quadratic behavior as tags grow.
| if (!draftTags!.build().contains(tag)) { | |
| if (!draftTags!.contains(tag)) { |
| validator: (value) { | ||
| if (value != null) { | ||
| if (value.isNotEmpty && value.contains(" ")) { | ||
| final tags = _parseTags(value ?? ''); | ||
|
|
||
| if (tags.isEmpty) { | ||
| return "Please enter a tag"; | ||
| } | ||
|
|
||
| for (final tag in tags) { | ||
| if (tag.contains(' ')) { | ||
| return "Tags cannot contain spaces"; | ||
| } | ||
|
|
||
| if (draftTags?.build().contains(tag) ?? false) { | ||
| return "Tag already exists"; | ||
| } | ||
| } |
There was a problem hiding this comment.
The validator introduces hard-coded English error strings ("Please enter a tag", "Tags cannot contain spaces", "Tag already exists"), which bypasses the app’s localization approach via SentenceManager (e.g., sentences.tagAlreadyExists, sentences.tagShouldNotContainSpaces). Please switch these to localized SentenceManager(...).sentences entries, and add a new sentences key for the empty-input message so it’s translated consistently.
| return "Tags cannot contain spaces"; | ||
| } | ||
|
|
||
| if (draftTags?.build().contains(tag) ?? false) { |
There was a problem hiding this comment.
draftTags?.build().contains(tag) in the validator has the same repeated-allocation issue as in _addTags. Prefer draftTags?.contains(tag) (or build once outside the loop) to avoid creating a new BuiltList during every validation pass (which can be triggered frequently while typing).
| if (draftTags?.build().contains(tag) ?? false) { | |
| if (draftTags?.contains(tag) ?? false) { |
Screen.Recording.2026-02-10.at.11.20.49.AM.movIt's still there |
@mulikruchi07 , Also check it with the Taskchampion type of Server Tag editor is not used there so you may need to implement it. |
|
I was thinking to replace this tags editor with bottom sheet with only tag editor which is in add task bottom sheet |
3 participants
Description
This PR introduces validation and displays clear inline error messages:
The dialog now remains open until a valid tag is submitted.
Fixes #518
Screenshots
TAG.mp4
Checklist