Added tag UI placeholder for future search and filtering #8396
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
Contributor
Reviewer's GuideIntroduces initial, UI-only tag support in the workspace home view by adding a tag search bar, a basic tag editor widget, and a simple Tag model plus filtering helper, without yet wiring them into actual workspace filtering logic. Sequence diagram for tag search bar interaction in HomeStacksequenceDiagram
actor User
participant HomeStackState
participant TagSearchBar
User->>HomeStackState: Open workspace home
HomeStackState->>TagSearchBar: Build TagSearchBar(onChanged)
User->>TagSearchBar: Type characters in search field
TagSearchBar-->>HomeStackState: onChanged(query)
HomeStackState->>HomeStackState: debugPrint(Search query: query)
HomeStackState-->>User: No filtering applied yet
Class diagram for new tag-related UI and modelclassDiagram
class Tag {
+String id
+String name
+int color
+Tag(String id, String name, int color)
}
class TagFilter {
+bool match(List~Tag~ tags, String query)
}
class TagEditor {
+List~Tag~ tags
+void onChanged(List~Tag~ tags)
+TagEditor(List~Tag~ tags, void onChanged(List~Tag~ tags))
}
class _TagEditorState {
-TextEditingController controller
+void _addTag(String value)
+Widget build(BuildContext context)
}
class TagSearchBar {
+void onChanged(String query)
+TagSearchBar(void onChanged(String query))
+Widget build(BuildContext context)
}
class HomeStackState {
+int selectedIndex
+Widget build(BuildContext context)
}
TagFilter ..> Tag : uses
TagEditor --> Tag : manages
TagEditor <|-- _TagEditorState : state
TagSearchBar <-- HomeStackState : contains
TagEditor ..> HomeStackState : future_integration
Flow diagram for TagFilter.match query evaluationflowchart TD
A_start["Start TagFilter.match"] --> B_check_empty["Is query empty?"]
B_check_empty -->|Yes| C_return_true["Return true (no filtering)"]
B_check_empty -->|No| D_lower_query["Convert query to lowercase"]
D_lower_query --> E_any_match["Does any tag.name contain query?"]
E_any_match -->|Yes| F_return_true["Return true (tags match query)"]
E_any_match -->|No| G_return_false["Return false (no tags match)"]
C_return_true --> H_end["End"]
F_return_true --> H_end
G_return_false --> H_end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
_HomeStackState, the temporarydebugPrint('Search query: $query');should either be gated behind an assert/debug flag or replaced with a TODO so it doesn’t accidentally ship in production. TagEditorcreates aTextEditingControllerbut does not dispose it; overridedispose()in_TagEditorStateand callcontroller.dispose()to avoid potential memory leaks.- The
.vs/AppFlowy/v17/DocumentLayout.jsonand.vs/VSWorkspaceState.jsonfiles look like local IDE artifacts and should generally be excluded from version control.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_HomeStackState`, the temporary `debugPrint('Search query: $query');` should either be gated behind an assert/debug flag or replaced with a TODO so it doesn’t accidentally ship in production.
- `TagEditor` creates a `TextEditingController` but does not dispose it; override `dispose()` in `_TagEditorState` and call `controller.dispose()` to avoid potential memory leaks.
- The `.vs/AppFlowy/v17/DocumentLayout.json` and `.vs/VSWorkspaceState.json` files look like local IDE artifacts and should generally be excluded from version control.
## Individual Comments
### Comment 1
<location> `frontend/appflowy_flutter/lib/workspace/presentation/widgets/tags/tag_editor.dart:18-19` </location>
<code_context>
+ State<TagEditor> createState() => _TagEditorState();
+}
+
+class _TagEditorState extends State<TagEditor> {
+ final controller = TextEditingController();
+
+ void _addTag(String value) {
</code_context>
<issue_to_address>
**issue (bug_risk):** TextEditingController should be disposed to avoid leaks.
Because `_TagEditorState` creates the `TextEditingController`, override `dispose` and dispose the controller before calling `super.dispose()`:
```dart
@override
void dispose() {
controller.dispose();
super.dispose();
}
```
</issue_to_address>
### Comment 2
<location> `frontend/appflowy_flutter/lib/workspace/application/tag/tag_filter.dart:4-11` </location>
<code_context>
+import 'tag.dart';
+
+class TagFilter {
+ static bool match({
+ required List<Tag> tags,
+ required String query,
+ }) {
+ if (query.isEmpty) return true;
+
+ final lower = query.toLowerCase();
+ return tags.any((tag) => tag.name.toLowerCase().contains(lower));
+ }
+}
</code_context>
<issue_to_address>
**suggestion:** Trimming and normalizing the query would make matching more robust.
Because the emptiness check runs before lowercasing, a query like `' '` is treated as non-empty and never matches anything. Trim and lowercase once (e.g. `final normalized = query.trim().toLowerCase();`) and use `normalized.isEmpty` and `normalized` for matching to handle whitespace-only queries correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+4
to
+11
| static bool match({ | ||
| required List<Tag> tags, | ||
| required String query, | ||
| }) { | ||
| if (query.isEmpty) return true; | ||
|
|
||
| final lower = query.toLowerCase(); | ||
| return tags.any((tag) => tag.name.toLowerCase().contains(lower)); |
Contributor
There was a problem hiding this comment.
suggestion: Trimming and normalizing the query would make matching more robust.
Because the emptiness check runs before lowercasing, a query like ' ' is treated as non-empty and never matches anything. Trim and lowercase once (e.g. final normalized = query.trim().toLowerCase();) and use normalized.isEmpty and normalized for matching to handle whitespace-only queries correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds initial UI groundwork for a tag section in the workspace.
The feature is not wired to filtering logic yet and is intended as a
foundation for future tag-based search and filtering.
I’m opening this early to get feedback on placement and approach
before implementing full behavior.
Summary by Sourcery
Introduce initial workspace tag UI components and basic tag model to prepare for future tag-based search and filtering.
New Features:
Enhancements: