-
Notifications
You must be signed in to change notification settings - Fork 52
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
[Feat] Team's tasks | Assigned / Unassigned option #3532
[Feat] Team's tasks | Assigned / Unassigned option #3532
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (14)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/components/pages/team/tasks/DropdownMenuTask.tsx (1)
69-69
: Fix inconsistent className spacing.While the addition of
cursor-pointer
improves UX, there's inconsistent spacing in the className values:
- Line 69:
" cursor-pointer"
- Line 76:
"relative cursor-pointer"
- Line 80:
" cursor-pointer"
- Line 84:
" cursor-pointer"
- className=" cursor-pointer" + className="cursor-pointer"Also applies to: 76-76, 80-80, 84-85
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/components/pages/team/tasks/DropdownMenuTask.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
apps/web/components/pages/team/tasks/DropdownMenuTask.tsx (2)
29-34
: Clean implementation of assignment toggle logic!The updated
handleAssignment
function is more straightforward and properly memoized with the correct dependencies.
20-20
: Verify integration with task filtering feature.The
isAssigned
flag correctly identifies task assignment status, but ensure that this status is properly integrated with the task filtering feature mentioned in the PR objectives.✅ Verification successful
Assignment status properly integrated with filtering system
The
isAssigned
flag is correctly integrated with the task filtering feature. It's used to populate the assigned/unassigned task groups, which are then filtered through the tab-based navigation system and can be further refined using additional filters.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for task filtering implementation rg -l "isAssigned.*filter|filter.*isAssigned" --type tsxLength of output: 86
Script:
#!/bin/bash # Search with correct file extensions and broader patterns rg -t ts -t tsx "filter.*task|task.*filter" -l rg -t ts -t tsx "(assigned|unassigned).*\b(filter|status)" -lLength of output: 168
Script:
#!/bin/bash # Find all potential TypeScript/React files first fd "\.(tsx?|jsx?)$" | grep -i "task" # Search for filtering logic without file type restrictions rg "filter.*task|task.*filter" --type-not test --type-not spec rg "(assigned|unassigned).*(filter|status)" --type-not test --type-not specLength of output: 17648
Script:
#!/bin/bash # Check content of potential filtering-related files echo "=== Task Filters Component ===" cat apps/web/lib/features/task/task-filters.tsx || true echo -e "\n=== Filter Button Component ===" cat apps/web/components/pages/team/tasks/FilterButton.tsx || true echo -e "\n=== Task Filter Store ===" cat apps/web/app/stores/task-filter.ts || trueLength of output: 20996
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/components/pages/team/tasks/DropdownMenuTask.tsx (4)
20-21
: Remove unused member variable.The
member
variable on line 21 appears to be unused after the introduction ofisAssigned
. Consider removing it to improve code clarity.const isAssigned = task?.members?.some((m) => m?.user?.id === user?.id); - const member = activeTeam?.members?.find((m) => m?.employee?.user?.id === user?.id);
29-34
: Optimize useCallback dependencies.The assignment logic looks good, but if you remove the unused
member
variable as suggested above, you should also consider creating a custom hook for the assignment logic to avoid thememberInfo
dependency.Consider extracting the assignment logic into a custom hook:
const useTaskAssignment = (task: ITeamTask) => { const { user } = useAuthenticateUser(); const isAssigned = task?.members?.some((m) => m?.user?.id === user?.id); const { assignTask, unassignTask } = useTaskActions(); // New hook for task actions const handleAssignment = useCallback(() => { if (isAssigned) { unassignTask(task); } else { assignTask(task); } }, [isAssigned, task, assignTask, unassignTask]); return { isAssigned, handleAssignment }; };
69-69
: Fix inconsistent className spacing.The
className
values have inconsistent spacing beforecursor-pointer
. Some have a leading space, while others don't.- className=" cursor-pointer" + className="cursor-pointer"Also applies to: 76-76, 80-80, 84-84
69-73
: Add error handling and user feedback for clipboard operation.The clipboard operation should include error handling and provide feedback to the user when the task ID is copied.
<DropdownMenuItem className="cursor-pointer" - onClick={() => taskEdition?.task?.id && navigator.clipboard.writeText(taskEdition.task.id)} + onClick={async () => { + if (taskEdition?.task?.id) { + try { + await navigator.clipboard.writeText(taskEdition.task.id); + // Add toast or notification: "Task ID copied to clipboard" + } catch (error) { + // Add error notification + console.error('Failed to copy task ID:', error); + } + } + }} > Copy Task ID </DropdownMenuItem>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/components/pages/team/tasks/DropdownMenuTask.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
Report too large to display inline |
Description
Closes #3454
Type of Change
Checklist
Loom
Summary by CodeRabbit
New Features
Style
cursor-pointer
class to dropdown menu items