-
Notifications
You must be signed in to change notification settings - Fork 68
feat(tags): implement tags dropdown in Add/Edit Task dialogs #351
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?
feat(tags): implement tags dropdown in Add/Edit Task dialogs #351
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! |
frontend/src/components/HomeComponents/Tasks/__tests__/AddTaskDialog.test.tsx
Show resolved
Hide resolved
ShivaGupta-14
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.
self review done! ready for review
|
Hi @its-me-abhishek! I’ve added dropdown tests in AddTaskDialog.test.tsx since the logic is identical in both dialogs, for TaskDialog.test.tsx, I only kept the remove/save tests to avoid duplication, should I also add the same dropdown interaction tests there, or is covering it once sufficient? Let me know your preference. |
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.
Please look into the above comments
frontend/src/components/HomeComponents/Tasks/__tests__/AddTaskDialog.test.tsx
Show resolved
Hide resolved
frontend/src/components/HomeComponents/Tasks/__tests__/AddTaskDialog.test.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/HomeComponents/Tasks/__tests__/AddTaskDialog.test.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/HomeComponents/Tasks/__tests__/AddTaskDialog.test.tsx
Outdated
Show resolved
Hide resolved
|
on it |
06dcaa3 to
d255e26
Compare
|
done with all changes, ready for review |
frontend/src/components/HomeComponents/Tasks/__tests__/AddTaskDialog.test.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/HomeComponents/Tasks/__tests__/AddTaskDialog.test.tsx
Show resolved
Hide resolved
frontend/src/components/HomeComponents/Tasks/__tests__/TaskDialog.test.tsx
Show resolved
Hide resolved
frontend/src/components/HomeComponents/Tasks/__tests__/tasks-utils.test.ts
Outdated
Show resolved
Hide resolved
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.
Suggested some changes and fixes
|
@its-me-abhishek, I understand your suggestions, but before proceeding, I’d genuinely like to get your input. I’ve also opened a PR for the same issue where I implemented a TagSelector component with search and multi-select functionality that shows the selected items. As a user, I personally prefer this approach, but I’d really appreciate your suggestion. I’m happy to proceed with whatever you decide. |
d255e26 to
20f9279
Compare
|
I implemented the current solution with considering all things in mind(your review and user experience), but i faced a issue with this -> Issue faced:the Popover inside Dialog had a click-through issue. radix UI's Dialog overlay blocks pointer events to Popover content because both use Portal and render at level with the same z-index. Approaches explored:
References:Stackover link: Link Current StatusImplemented Container Solution. All tests passing. |
|
also we can do one more thing if going with option 1st -> to maintain two popover file (one with portal and one without portal) |
frontend/src/components/HomeComponents/Tasks/__tests__/SearchAndAddSelector.test.tsx
Show resolved
Hide resolved
frontend/src/components/HomeComponents/Tasks/__tests__/AddTaskDialog.test.tsx
Show resolved
Hide resolved
frontend/src/components/HomeComponents/Tasks/__tests__/TaskDialog.test.tsx
Show resolved
Hide resolved
20f9279 to
f96c5db
Compare
ShivaGupta-14
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.
@its-me-abhishek self review done, ready for review
f96c5db to
a57865a
Compare
…alogs - Create SearchAndAddSelector component with search, select, and create functionality - Use local Popover (no Portal) to fix click-through issue in Dialog without modifying global popover.tsx - Display selected tags as removable chips with X buttons - Add search filtering for existing tags - Add inline "Create new tag" option for non-existing tags - Integrate SearchAndAddSelector into TaskDialog and AddTaskDialog Bug fix: - Fix tag removal not persisting: now sends tags with "-" prefix to backend for removals - Updated handleSaveTags to calculate tag additions and removals correctly Tests: - Add comprehensive tests for SearchAndAddSelector component - Update AddTaskDialog tests for new tag selection flow - Update TaskDialog tests for tag editing with SearchAndAddSelector - Mock SearchAndAddSelector in tests Fixes: CCExtractor#210
a57865a to
46cd01d
Compare
|
@its-me-abhishek thank you for the thoughtful guidance on the channel! Following your suggestion about keeping Shadcn components untouched (for long-term maintainability):
I've refactored the implementation:
All tests passing, happy to make any adjustments based on your review. thanks again for the clear direction, learned a lot from this! Also, if @Hell1213's PR #390 looks good for the project, happy to go with that approach instead! :) |
Description
SearchAndAddSelectorcomponent with search, select, and create functionalitylocal Popover(no Portal) to fix click-through issue in Dialog without modifying global popover.tsxPopover-in-Dialog fix (local component approach):
LocalPopoverContentinsideSearchAndAddSelectorthat renders without PortalTests:
Add comprehensive tests for SearchAndAddSelector component
Update AddTaskDialog tests for new tag selection flow
Update TaskDialog tests for tag editing with SearchAndAddSelector
Mock SearchAndAddSelector in tests
Fixes: Reusable tags for Editing or Adding a task #210
Bug: Tag removal not persisting (Fixed in this PR)
Screen.Recording.2026-01-12.at.12.50.58.AM.mov
Bug fix:
Checklist
npx prettier --write .(for formatting)gofmt -w .(for Go backend)npm test(for JS/TS testing)Additional Notes
Demo Video:
Screen.Recording.2026-01-12.at.12.31.45.AM.mov
Screenshots: