-
Notifications
You must be signed in to change notification settings - Fork 68
feat: add reusable tag selector for Add/Edit Task dialogs #390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for opening this PR! Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools. Please take a moment to:
More information on how to conduct a self review: This helps make the review process smoother and gives us a clearer understanding of your thought process. Once you've added your self-review, we'll continue from our side. Thank you! |
Hell1213
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done with self review , PR adds a reusable tag component that makes tag management easier for users. Instead of typing tags manually , users can now select from existing tags or create new ones through a dropdown. The component is used in both Add Task and Edit Task dialogs, removing code duplication.
its-me-abhishek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, do checkout the suggestions
| setTasksPerPage(parseInt(storedTasksPerPage, 10)); | ||
| } | ||
| }, [props.email]); | ||
| useEffect(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this useEffect could be removed, completely
| @@ -0,0 +1,203 @@ | |||
| import { useState, useRef, useEffect } from 'react'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we move to a more generic name for this? considering it will be used for both Tags, and Annotations
| handleNewTagCreate(); | ||
| } | ||
| } | ||
| // Do nothing if search is empty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do remove the comment
| } | ||
| }; | ||
|
|
||
| const handleKeyDown = (e: React.KeyboardEvent) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe extract this and other functions to the utils file
|
|
||
| expect(screen.queryByText('tag2')).not.toBeInTheDocument(); | ||
| await waitFor(() => { | ||
| // Check that tag1 is not in the selected tags badges area |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kindly remove the comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and other such comments
| @@ -0,0 +1,556 @@ | |||
| import { render, screen, fireEvent, waitFor } from '@testing-library/react'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests would need to be updated, upon changing the file name
|
sure @its-me-abhishek , i have implemented all changes as requested will push soon . |
cd58387 to
4937543
Compare
Replaced manual tag input with a dropdown selector to fix typos and inconsistent tagging.
Changes:
Added new TagMultiSelect component with dropdown for tag selection
Replaced manual tag input in Add Task dialog with TagMultiSelect
Replaced manual tag input in Edit Task dialog with TagMultiSelect
Save/cancel buttons now appear inline with selected tags
Fixes: #210
Checklist
Additional Notes
The new component makes tag management easier:
Click dropdown to see all existing tags
Search or create new tags by typing
Press Enter or create tag to add a tag
Click X on tag badges to remove them
Save/cancel buttons show up next to the tags
Demo
Screencast.from.2026-01-10.14-38-35.mp4