feat: Add task details modal with view-only functionality #199
Conversation
- Create TaskDetailsModal component for viewing complete task information - Add View Details button (eye icon) to task tables in dashboard and team pages - Display task title, description, status, priority, assignee, created by, due date, labels, tags, and watchlist status - Use large textarea for easy reading of task descriptions - Remove edit functionality to keep it as view-only feature - Integrate modal into both main task list and team task list components
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
WalkthroughAdds a new TaskDetailsModal component and integrates it with an Eye-button in task rows to view detailed task info. Updates team tasks view to use TASK_STATUS_ENUM for including done tasks and adds an IncludeDoneSwitch in the header. Minor import reorganizations; no API signature changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant T as TodoListTableRow
participant M as TaskDetailsModal
participant UI as UI Primitives
U->>T: Click "View details" (Eye)
T->>M: Open modal with task props
activate M
M->>UI: Render dialog, text, grid, badges, avatars
note right of M: Fields shown with fallbacks<br/>Dates formatted via DateUtil
U-->>M: Close
M->>UI: Dismiss dialog
deactivate M
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| {task.assignee ? ( | ||
| <div className="flex items-center space-x-2"> | ||
| <div className="w-6 h-6 bg-blue-500 rounded-full flex items-center justify-center text-white text-xs font-medium"> | ||
| {task.assignee.assignee_name?.charAt(0).toUpperCase() || 'U'} |
There was a problem hiding this comment.
The optional chaining operator is applied too late in the expression. If assignee_name is null or undefined, the code will still attempt to access it through task.assignee.assignee_name, which could cause a runtime error if assignee exists but assignee_name doesn't. Consider using optional chaining earlier in the chain:
{task.assignee?.assignee_name?.charAt(0).toUpperCase() || 'U'}This ensures safe property access throughout the entire expression.
| {task.assignee.assignee_name?.charAt(0).toUpperCase() || 'U'} | |
| {task.assignee?.assignee_name?.charAt(0).toUpperCase() || 'U'} |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| Deferred Until | ||
| </Label> | ||
| <div className="text-gray-900"> | ||
| {new DateUtil(task.deferredDetails.deferredTill).format(DateFormats.D_MMM_YYYY)} |
There was a problem hiding this comment.
There's a potential null reference issue in this line. While the condition correctly uses optional chaining with task.deferredDetails?.deferredTill, the actual usage below doesn't maintain this pattern. To prevent runtime errors, please use optional chaining consistently:
new DateUtil(task.deferredDetails?.deferredTill).format(DateFormats.D_MMM_YYYY)This ensures the code handles cases where deferredDetails might be null or undefined.
| {new DateUtil(task.deferredDetails.deferredTill).format(DateFormats.D_MMM_YYYY)} | |
| {new DateUtil(task.deferredDetails?.deferredTill).format(DateFormats.D_MMM_YYYY)} |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Repeated Complex User Initial Logic ▹ view | |
 |
Unsanitized Task Description Rendering ▹ view | |
 |
Modal State Control Limitation ▹ view | |
 |
Redundant String Operations ▹ view | |
|
Inefficient Date Formatting ▹ view | |
|
Inefficient Array Key Usage ▹ view | |
|
Duplicated User Avatar Logic ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| modules/teams/team-tasks.tsx | ✅ |
| components/todo-list-table.tsx | ✅ |
| components/task-details-modal.tsx | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
| value={task.description || 'No description provided'} | ||
| readOnly | ||
| className="min-h-[100px] resize-none border-none bg-transparent p-0 focus:ring-0" |
There was a problem hiding this comment.
Unsanitized Task Description Rendering 
Tell me more
What is the issue?
Task description is rendered directly in a textarea without sanitization, potentially allowing XSS attacks if the description contains malicious content.
Why this matters
If the task description contains JavaScript code or HTML tags, it could execute in the user's browser when rendered, leading to cross-site scripting (XSS) vulnerabilities.
Suggested change ∙ Feature Preview
// Option 1: Use a sanitization library
import DOMPurify from 'dompurify';
<Textarea
value={DOMPurify.sanitize(task.description) || 'No description provided'}
readOnly
className="min-h-[100px] resize-none border-none bg-transparent p-0 focus:ring-0"
/>
// Option 2: If HTML is not needed, escape special characters
<Textarea
value={task.description?.replace(/[<>"'&]/g, (char) => ({
'<': '<',
'>': '>',
'"': '"',
"'": ''',
'&': '&'
}[char])) || 'No description provided'}
readOnly
className="min-h-[100px] resize-none border-none bg-transparent p-0 focus:ring-0"
/>Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| } | ||
|
|
||
| export function TaskDetailsModal({ task, children }: TaskDetailsModalProps) { | ||
| const [isOpen, setIsOpen] = useState(false) |
There was a problem hiding this comment.
Modal State Control Limitation 
Tell me more
What is the issue?
The modal's open state is managed internally, which prevents parent components from controlling the modal's visibility or reacting to its state changes.
Why this matters
Parent components may need to programmatically open/close the modal or synchronize its state with other UI elements. The current implementation prevents this flexibility.
Suggested change ∙ Feature Preview
Add isOpen and onOpenChange props to allow parent components to control the modal's state:
type TaskDetailsModalProps = {
task: TTask
children: React.ReactNode
isOpen?: boolean
onOpenChange?: (open: boolean) => void
}
export function TaskDetailsModal({
task,
children,
isOpen: controlledIsOpen,
onOpenChange
}: TaskDetailsModalProps) {
const [uncontrolledIsOpen, setUncontrolledIsOpen] = useState(false)
const isOpen = controlledIsOpen ?? uncontrolledIsOpen
const handleOpenChange = (open: boolean) => {
setUncontrolledIsOpen(open)
onOpenChange?.(open)
}
return (
<Dialog open={isOpen} onOpenChange={handleOpenChange}>
{/* rest of the code... */}
</Dialog>
)
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| {task.createdBy ? ( | ||
| <div className="flex items-center space-x-2"> | ||
| <div className="w-6 h-6 bg-green-500 rounded-full flex items-center justify-center text-white text-xs font-medium"> | ||
| {task.createdBy.name?.charAt(0).toUpperCase() || 'U'} |
There was a problem hiding this comment.
Redundant String Operations 
Tell me more
What is the issue?
Multiple unnecessary string operations are being performed on every render for both assignee and creator initials.
Why this matters
Each render triggers charAt(), toUpperCase(), and optional chaining operations. For a list of tasks, this can impact performance, especially during scrolling or frequent re-renders.
Suggested change ∙ Feature Preview
Memoize the initials or compute them when the task data changes:
const getInitials = useCallback((name?: string) => {
return name?.charAt(0).toUpperCase() || 'U'
}, [])
// Then use:
{getInitials(task.createdBy.name)}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| </Label> | ||
| <div className="text-gray-900"> | ||
| {task.dueAt ? ( | ||
| new DateUtil(task.dueAt).format(DateFormats.D_MMM_YYYY) |
There was a problem hiding this comment.
Inefficient Date Formatting
Tell me more
What is the issue?
Date formatting operations are being created and executed on every render.
Why this matters
Creating new DateUtil instances and formatting dates on each render is computationally expensive, especially when dealing with lists of tasks.
Suggested change ∙ Feature Preview
Memoize the formatted date or move the formatting logic outside the render cycle:
const formattedDate = useMemo(() => {
return task.dueAt ? new DateUtil(task.dueAt).format(DateFormats.D_MMM_YYYY) : 'No due date'
}, [task.dueAt])
// Then use:
{formattedDate}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| {task.tags.map((tag, index) => ( | ||
| <span | ||
| key={index} | ||
| className="px-2 py-1 bg-gray-200 text-gray-700 text-xs rounded-full" | ||
| > |
There was a problem hiding this comment.
Inefficient Array Key Usage
Tell me more
What is the issue?
Using array index as key in the tags mapping can lead to unnecessary re-renders when the array changes.
Why this matters
When tags are added, removed, or reordered, React may unnecessarily re-render components because index-based keys don't provide stable identification.
Suggested change ∙ Feature Preview
Use the tag value as the key if tags are unique, or generate stable IDs:
task.tags.map((tag) => (
<span
key={tag} // Assuming tags are unique strings
className="px-2 py-1 bg-gray-200 text-gray-700 text-xs rounded-full"
>Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| {task.assignee ? ( | ||
| <div className="flex items-center space-x-2"> | ||
| <div className="w-6 h-6 bg-blue-500 rounded-full flex items-center justify-center text-white text-xs font-medium"> | ||
| {task.assignee.assignee_name?.charAt(0).toUpperCase() || 'U'} |
There was a problem hiding this comment.
Repeated Complex User Initial Logic 
Tell me more
What is the issue?
The complex fallback logic for user initials appears multiple times with slight variations.
Why this matters
Duplicated complex logic makes the code harder to maintain and increases the chance of inconsistencies.
Suggested change ∙ Feature Preview
const getUserInitial = (name?: string) => name?.charAt(0).toUpperCase() || 'U';
// Then use it like:
{getUserInitial(task.assignee.assignee_name)}
{getUserInitial(task.createdBy.name)}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| {/* Assignee */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| <UserIcon className="inline w-4 h-4 mr-1" /> | ||
| Assignee | ||
| </Label> | ||
| <div className="text-gray-900"> | ||
| {task.assignee ? ( | ||
| <div className="flex items-center space-x-2"> | ||
| <div className="w-6 h-6 bg-blue-500 rounded-full flex items-center justify-center text-white text-xs font-medium"> | ||
| {task.assignee.assignee_name?.charAt(0).toUpperCase() || 'U'} | ||
| </div> | ||
| <span>{task.assignee.assignee_name || 'Unknown User'}</span> | ||
| </div> | ||
| ) : ( | ||
| 'Unassigned' | ||
| )} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Duplicated User Avatar Logic
Tell me more
What is the issue?
The user avatar display logic is duplicated between Assignee and Created By sections with nearly identical markup and styling.
Why this matters
Violates the DRY principle and makes maintenance harder as changes to the avatar display need to be made in multiple places.
Suggested change ∙ Feature Preview
Extract the avatar display into a reusable component:
const UserAvatar = ({ name, bgColor }: { name: string; bgColor: string }) => (
<div className="flex items-center space-x-2">
<div className={`w-6 h-6 ${bgColor} rounded-full flex items-center justify-center text-white text-xs font-medium`}>
{name?.charAt(0).toUpperCase() || 'U'}
</div>
<span>{name || 'Unknown User'}</span>
</div>
);Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
components/todo-list-table.tsx (1)
150-151: Fix colSpan mismatch when Deferred column is shown.Empty and shimmer rows don’t account for
showDeferredColumn, causing misaligned table cells.- <TableCell colSpan={showActions ? 7 : 6}> + <TableCell colSpan={(showActions ? 8 : 7) + (showDeferredColumn ? 1 : 0) - 1}>And similarly in
TodoListTableRowShimmer:- <TableCell colSpan={showActions ? 7 : 6}> + <TableCell colSpan={(showActions ? 8 : 7) + (showDeferredColumn ? 1 : 0) - 1}>If
showDeferredColumnisn’t available in shimmer’s props, extend its props to accept it.Also applies to: 176-177
modules/teams/team-tasks.tsx (2)
105-107: Correct colSpan for team tasks table.Header shows 7 base columns + Actions => 8. Current
colSpan={showActions ? 7 : 6}is off by one.- <TableCell colSpan={showActions ? 7 : 6}> + <TableCell colSpan={showActions ? 8 : 7}>
38-41: Rename typo: isRessignTodoCtaVisible → isReassignTodoCtaVisible.Improves readability and consistency.
- const isRessignTodoCtaVisible = + const isReassignTodoCtaVisible = todo.assignee?.user_type === USER_TYPE_ENUM.TEAM && team?.poc_id === user.id ... - {isRessignTodoCtaVisible && <ReassignUser taskId={todo.id} teamId={team.id} />} + {isReassignTodoCtaVisible && <ReassignUser taskId={todo.id} teamId={team.id} />} ... - {!isRessignTodoCtaVisible && ( + {!isReassignTodoCtaVisible && (Also applies to: 73-77
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
components/task-details-modal.tsx(1 hunks)components/todo-list-table.tsx(2 hunks)modules/teams/team-tasks.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T11:21:22.274Z
Learnt from: Hariom01010
PR: Real-Dev-Squad/todo-frontend#192
File: api/teams/teams.type.ts:56-58
Timestamp: 2025-08-29T11:21:22.274Z
Learning: In the todo-frontend repository, UI mappings for TeamActivityActions are handled in lib/team-utils.ts in the getActivityUIData function using a switch statement that maps each action to an icon, title, and description for rendering audit logs.
Applied to files:
modules/teams/team-tasks.tsx
🧬 Code graph analysis (3)
components/todo-list-table.tsx (1)
components/task-details-modal.tsx (1)
TaskDetailsModal(27-232)
components/task-details-modal.tsx (5)
api/tasks/tasks.types.ts (1)
TTask(11-24)components/todo-status-table.tsx (1)
TodoStatusTable(15-32)components/task-priority-label.tsx (1)
TaskPriorityLabel(14-29)lib/date-util.ts (1)
DateUtil(11-21)components/todo-labels-list.tsx (1)
TodoLabelsList(92-109)
modules/teams/team-tasks.tsx (2)
components/task-details-modal.tsx (1)
TaskDetailsModal(27-232)components/ui/button.tsx (1)
Button(59-59)
🔇 Additional comments (5)
components/todo-list-table.tsx (2)
8-8: Imports for modal trigger look good.No issues with added imports.
Also applies to: 14-14, 18-18
160-168: Confirm UX: details hidden for TEAM-assigned tasks.
showActions={showActions && task.assignee?.user_type !== USER_TYPE_ENUM.TEAM}will also hide the View Details button. If details should be available to everyone, gate edit/watchlist only.modules/teams/team-tasks.tsx (2)
10-11: Imports and integration look consistent.All new imports align with usage.
Also applies to: 13-13, 18-18, 24-24
131-135: Verify “Include done” behavior — this filters to only DONE tasks.
includeDoneTasksis true only whenstatus === TASK_STATUS_ENUM.DONE, and thenqueryParamssetsstatus: DONE. That shows only DONE, not “include” in results. Confirm intended copy/behavior.components/task-details-modal.tsx (1)
30-41: LGTM – controlled Dialog with trigger-as-child is correct.State handling and modal structure are sound.
| type TaskDetailsModalProps = { | ||
| task: TTask | ||
| children: React.ReactNode | ||
| } |
There was a problem hiding this comment.
Fix React type reference for children.
Using React.ReactNode without importing the React namespace can fail TS checks. Prefer ReactNode type import.
-import { useState } from 'react'
+import { useState } from 'react'
+import type { ReactNode } from 'react'
...
-type TaskDetailsModalProps = {
+type TaskDetailsModalProps = {
task: TTask
- children: React.ReactNode
+ children: ReactNode
}Also applies to: 17-17
🤖 Prompt for AI Agents
In components/task-details-modal.tsx around lines 17 and 22-25, replace the use
of the namespaced type React.ReactNode with the directly imported ReactNode and
add the import: import { ReactNode } from 'react'; update the
TaskDetailsModalProps children type to use ReactNode and update any other
occurrences in the file to use the imported ReactNode to satisfy TypeScript
checks.
| <Textarea | ||
| value={task.description || 'No description provided'} | ||
| readOnly | ||
| className="min-h-[100px] resize-none border-none bg-transparent p-0 focus:ring-0" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Associate description with an accessible name.
Add aria-label to the read-only textarea.
<Textarea
value={task.description || 'No description provided'}
readOnly
+ aria-label="Task description"
className="min-h-[100px] resize-none border-none bg-transparent p-0 focus:ring-0"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Textarea | |
| value={task.description || 'No description provided'} | |
| readOnly | |
| className="min-h-[100px] resize-none border-none bg-transparent p-0 focus:ring-0" | |
| /> | |
| </div> | |
| <Textarea | |
| value={task.description || 'No description provided'} | |
| readOnly | |
| aria-label="Task description" | |
| className="min-h-[100px] resize-none border-none bg-transparent p-0 focus:ring-0" | |
| /> |
🤖 Prompt for AI Agents
In components/task-details-modal.tsx around lines 59 to 64, the read-only
Textarea lacks an accessible name; add an aria-label prop to the Textarea (for
example "Task description" or a dynamic value like `Description for ${task.title
|| 'task'}`) so screen readers can identify the field; ensure the aria-label
conveys that this is the task description and keep the Textarea readOnly
behavior unchanged.
| {task.tags && task.tags.length > 0 && ( | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| Tags | ||
| </Label> | ||
| <div className="flex flex-wrap gap-2"> | ||
| {task.tags.map((tag, index) => ( | ||
| <span | ||
| key={index} | ||
| className="px-2 py-1 bg-gray-200 text-gray-700 text-xs rounded-full" | ||
| > | ||
| {tag} | ||
| </span> | ||
| ))} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider stable keys for tags if unique.
If tag strings are unique, prefer key={tag} over index to avoid reconciliation issues on reordering.
🤖 Prompt for AI Agents
In components/task-details-modal.tsx around lines 176 to 189, the tag list uses
the array index as the React key which can cause reconciliation bugs on reorder;
if tag strings are guaranteed unique, change the key to the tag value
(key={tag}); if they may not be unique, use a stable key such as a unique id on
each tag or a composite key like `${tag}-${index}` to ensure stability.
| <TaskDetailsModal task={todo}> | ||
| <Button variant="ghost" size="sm" className="h-8 w-8 p-0"> | ||
| <EyeIcon className="h-4 w-4" /> | ||
| <span className="sr-only">View details</span> | ||
| </Button> | ||
| </TaskDetailsModal> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Good addition of view-only TaskDetails trigger; add explicit aria-label for better SR output.
Current sr-only text works; an aria-label with task context improves accessibility.
- <Button variant="ghost" size="sm" className="h-8 w-8 p-0">
+ <Button
+ variant="ghost"
+ size="sm"
+ className="h-8 w-8 p-0"
+ aria-label={`View details: ${todo.title}`}
+ title="View details"
+ >
<EyeIcon className="h-4 w-4" />
- <span className="sr-only">View details</span>
+ <span className="sr-only">View details</span>
</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <TaskDetailsModal task={todo}> | |
| <Button variant="ghost" size="sm" className="h-8 w-8 p-0"> | |
| <EyeIcon className="h-4 w-4" /> | |
| <span className="sr-only">View details</span> | |
| </Button> | |
| </TaskDetailsModal> | |
| <TaskDetailsModal task={todo}> | |
| <Button | |
| variant="ghost" | |
| size="sm" | |
| className="h-8 w-8 p-0" | |
| aria-label={`View details: ${todo.title}`} | |
| title="View details" | |
| > | |
| <EyeIcon className="h-4 w-4" /> | |
| <span className="sr-only">View details</span> | |
| </Button> | |
| </TaskDetailsModal> |
🤖 Prompt for AI Agents
In components/todo-list-table.tsx around lines 102 to 107, the view-only
TaskDetails trigger uses an sr-only span but lacks an explicit aria-label; add
an aria-label attribute to the Button (e.g., `aria-label={`View details for
${todo.title}`}` or similar using the task context) so screen readers receive a
concise, contextual label, keeping the existing sr-only span if desired for
redundancy.
| <TaskDetailsModal task={todo}> | ||
| <Button variant="ghost" size="sm" className="h-8 w-8 p-0"> | ||
| <EyeIcon className="h-4 w-4" /> | ||
| <span className="sr-only">View details</span> | ||
| </Button> | ||
| </TaskDetailsModal> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Nice modal trigger; add aria-label with task title.
Matches pattern in dashboard table; include contextual label.
- <Button variant="ghost" size="sm" className="h-8 w-8 p-0">
+ <Button
+ variant="ghost"
+ size="sm"
+ className="h-8 w-8 p-0"
+ aria-label={`View details: ${todo.title}`}
+ title="View details"
+ >
<EyeIcon className="h-4 w-4" />
<span className="sr-only">View details</span>
</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <TaskDetailsModal task={todo}> | |
| <Button variant="ghost" size="sm" className="h-8 w-8 p-0"> | |
| <EyeIcon className="h-4 w-4" /> | |
| <span className="sr-only">View details</span> | |
| </Button> | |
| </TaskDetailsModal> | |
| <TaskDetailsModal task={todo}> | |
| <Button | |
| variant="ghost" | |
| size="sm" | |
| className="h-8 w-8 p-0" | |
| aria-label={`View details: ${todo.title}`} | |
| title="View details" | |
| > | |
| <EyeIcon className="h-4 w-4" /> | |
| <span className="sr-only">View details</span> | |
| </Button> | |
| </TaskDetailsModal> |
🤖 Prompt for AI Agents
In modules/teams/team-tasks.tsx around lines 67 to 72, the modal trigger Button
lacks an accessible aria-label that includes the task title; add an aria-label
prop to the Button that contains contextual text including the todo.title (for
example "View details for {todo.title}") so screen readers get the task-specific
label, keeping the existing visually-hidden span as needed.
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
|
Incorrect icon for Task ID field ▹ view | |
 |
Missing Component Documentation ▹ view | |
 |
Unstable React keys using array index ▹ view | |
|
Missing Props Documentation ▹ view | |
|
Component with Multiple Responsibilities ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| modules/teams/team-tasks.tsx | ✅ |
| components/todo-list-table.tsx | ✅ |
| components/task-details-modal.tsx | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
| <ClockIcon className="inline w-4 h-4 mr-1" /> | ||
| Task ID |
There was a problem hiding this comment.
Incorrect icon for Task ID field
Tell me more
What is the issue?
Clock icon is semantically incorrect for representing a Task ID field.
Why this matters
Users may be confused by the visual representation as clock icons typically represent time-related information, not identifiers, reducing the UI's intuitiveness.
Suggested change ∙ Feature Preview
Use a more appropriate icon like HashIcon or remove the icon entirely:
<Label className="text-sm font-medium text-gray-700 mb-2 block">
Task ID
</Label>Or import and use HashIcon:
import { HashIcon } from 'lucide-react'
<Label className="text-sm font-medium text-gray-700 mb-2 block">
<HashIcon className="inline w-4 h-4 mr-1" />
Task ID
</Label>Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| type TaskDetailsModalProps = { | ||
| task: TTask | ||
| children: React.ReactNode | ||
| } |
There was a problem hiding this comment.
Missing Props Documentation 
Tell me more
What is the issue?
The TaskDetailsModalProps type lacks documentation explaining the purpose of each prop and their expected values.
Why this matters
Without clear prop documentation, other developers may misuse the component or spend unnecessary time investigating the expected prop values.
Suggested change ∙ Feature Preview
/** Props for the TaskDetailsModal component */
type TaskDetailsModalProps = {
/** The task object containing all task details to be displayed */
task: TTask
/** The trigger element that opens the modal when clicked */
children: React.ReactNode
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| children: React.ReactNode | ||
| } | ||
|
|
||
| export function TaskDetailsModal({ task, children }: TaskDetailsModalProps) { |
There was a problem hiding this comment.
Missing Component Documentation
Tell me more
What is the issue?
The TaskDetailsModal component lacks a component-level JSDoc comment explaining its purpose and usage.
Why this matters
Without component documentation, developers need to read through the implementation to understand the component's purpose and how to use it correctly.
Suggested change ∙ Feature Preview
/**
* A modal component that displays detailed information about a task,
* including its title, description, status, priority, assignee, and other metadata.
*/
export function TaskDetailsModal({ task, children }: TaskDetailsModalProps) {Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| {task.tags.map((tag, index) => ( | ||
| <span | ||
| key={index} | ||
| className="px-2 py-1 bg-gray-200 text-gray-700 text-xs rounded-full" | ||
| > | ||
| {tag} | ||
| </span> | ||
| ))} |
There was a problem hiding this comment.
Unstable React keys using array index 
Tell me more
What is the issue?
Using array index as React key when rendering tags list creates unstable keys that can cause rendering issues.
Why this matters
React may incorrectly reconcile components when tags are reordered, added, or removed, leading to potential state corruption or performance issues in the UI.
Suggested change ∙ Feature Preview
Use the tag value itself as the key since tags should be unique strings:
{task.tags.map((tag) => (
<span
key={tag}
className="px-2 py-1 bg-gray-200 text-gray-700 text-xs rounded-full"
>
{tag}
</span>
))}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| export function TaskDetailsModal({ task, children }: TaskDetailsModalProps) { | ||
| const [isOpen, setIsOpen] = useState(false) | ||
|
|
||
| return ( | ||
| <Dialog open={isOpen} onOpenChange={setIsOpen}> | ||
| <DialogTrigger asChild> | ||
| {children} | ||
| </DialogTrigger> | ||
| <DialogContent className="max-w-2xl max-h-[80vh] overflow-y-auto"> | ||
| <DialogHeader> | ||
| <DialogTitle className="text-xl font-semibold text-gray-900"> | ||
| Task Details | ||
| </DialogTitle> | ||
| </DialogHeader> | ||
|
|
||
| <div className="space-y-6"> | ||
| {/* Task Title */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| Task Title | ||
| </Label> | ||
| <div className="text-lg font-medium text-gray-900"> | ||
| {task.title} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Description */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| Description | ||
| </Label> | ||
| <div className="min-h-[120px] p-3 bg-gray-50 rounded-md border"> | ||
| <Textarea | ||
| value={task.description || 'No description provided'} | ||
| readOnly | ||
| className="min-h-[100px] resize-none border-none bg-transparent p-0 focus:ring-0" | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| <Separator /> | ||
|
|
||
| {/* Task Information Grid */} | ||
| <div className="grid grid-cols-1 md:grid-cols-2 gap-4"> | ||
| {/* Status */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| Status | ||
| </Label> | ||
| <div className="flex items-center"> | ||
| <TodoStatusTable status={task.status} /> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Priority */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| Priority | ||
| </Label> | ||
| <div className="flex items-center"> | ||
| {task.priority ? ( | ||
| <TaskPriorityLabel priority={task.priority} /> | ||
| ) : ( | ||
| <span className="text-gray-500">No priority set</span> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Assignee */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| <UserIcon className="inline w-4 h-4 mr-1" /> | ||
| Assignee | ||
| </Label> | ||
| <div className="text-gray-900"> | ||
| {task.assignee ? ( | ||
| <div className="flex items-center space-x-2"> | ||
| <div className="w-6 h-6 bg-blue-500 rounded-full flex items-center justify-center text-white text-xs font-medium"> | ||
| {task.assignee.assignee_name?.charAt(0).toUpperCase() || 'U'} | ||
| </div> | ||
| <span>{task.assignee.assignee_name || 'Unknown User'}</span> | ||
| </div> | ||
| ) : ( | ||
| 'Unassigned' | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Created By */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| <UserPlusIcon className="inline w-4 h-4 mr-1" /> | ||
| Created By | ||
| </Label> | ||
| <div className="text-gray-900"> | ||
| {task.createdBy ? ( | ||
| <div className="flex items-center space-x-2"> | ||
| <div className="w-6 h-6 bg-green-500 rounded-full flex items-center justify-center text-white text-xs font-medium"> | ||
| {task.createdBy.name?.charAt(0).toUpperCase() || 'U'} | ||
| </div> | ||
| <span>{task.createdBy.name || 'Unknown User'}</span> | ||
| </div> | ||
| ) : ( | ||
| 'Unknown' | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Due Date */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| <CalendarIcon className="inline w-4 h-4 mr-1" /> | ||
| Due Date | ||
| </Label> | ||
| <div className="text-gray-900"> | ||
| {task.dueAt ? ( | ||
| new DateUtil(task.dueAt).format(DateFormats.D_MMM_YYYY) | ||
| ) : ( | ||
| 'No due date' | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Task ID */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| <ClockIcon className="inline w-4 h-4 mr-1" /> | ||
| Task ID | ||
| </Label> | ||
| <div className="text-gray-900 font-mono text-sm"> | ||
| {task.id} | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Labels */} | ||
| {task.labels && task.labels.length > 0 && ( | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| <TagIcon className="inline w-4 h-4 mr-1" /> | ||
| Labels | ||
| </Label> | ||
| <div className="flex flex-wrap gap-2"> | ||
| <TodoLabelsList labels={task.labels} /> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Tags */} | ||
| {task.tags && task.tags.length > 0 && ( | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| Tags | ||
| </Label> | ||
| <div className="flex flex-wrap gap-2"> | ||
| {task.tags.map((tag, index) => ( | ||
| <span | ||
| key={index} | ||
| className="px-2 py-1 bg-gray-200 text-gray-700 text-xs rounded-full" | ||
| > | ||
| {tag} | ||
| </span> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Watchlist Status */} | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| Watchlist | ||
| </Label> | ||
| <div className="text-gray-900"> | ||
| {task.in_watchlist ? ( | ||
| <span className="inline-flex items-center px-2 py-1 rounded-full text-xs font-medium bg-yellow-100 text-yellow-800"> | ||
| ⭐ In Watchlist | ||
| </span> | ||
| ) : ( | ||
| <span className="text-gray-500">Not in watchlist</span> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Deferred Details */} | ||
| {task.deferredDetails?.deferredTill && ( | ||
| <div> | ||
| <Label className="text-sm font-medium text-gray-700 mb-2 block"> | ||
| Deferred Until | ||
| </Label> | ||
| <div className="text-gray-900"> | ||
| {new DateUtil(task.deferredDetails.deferredTill).format(DateFormats.D_MMM_YYYY)} | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
| {/* Action Buttons */} | ||
| <div className="flex justify-end space-x-2 pt-4 border-t"> | ||
| <Button variant="outline" onClick={() => setIsOpen(false)}> | ||
| Close | ||
| </Button> | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
| ) |
There was a problem hiding this comment.
Component with Multiple Responsibilities
Tell me more
What is the issue?
The TaskDetailsModal component has too many responsibilities, handling both the modal dialog logic and the rendering of multiple different types of task information.
Why this matters
This violates the Single Responsibility Principle and makes the component harder to maintain, test, and reuse. Future changes to either the modal behavior or task display logic will require modifying this large component.
Suggested change ∙ Feature Preview
Split the component into smaller, focused components:
function TaskDetails({ task }: { task: TTask }) {
return (
<div className="space-y-6">
<TaskHeader task={task} />
<TaskDescription description={task.description} />
<TaskMetadata task={task} />
<TaskLabelsSection labels={task.labels} />
<TaskTagsSection tags={task.tags} />
<TaskWatchlistStatus inWatchlist={task.in_watchlist} />
<TaskDeferredDetails deferredDetails={task.deferredDetails} />
</div>
)
}
export function TaskDetailsModal({ task, children }: TaskDetailsModalProps) {
const [isOpen, setIsOpen] = useState(false)
return (
<Dialog open={isOpen} onOpenChange={setIsOpen}>
<DialogContent>
<TaskDetails task={task} />
</DialogContent>
</Dialog>
)
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
1 participant
Date: 10 septem 2025
Developer Name:
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
No
Breaking Changes
Development Tested?
Screenshots
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Add a new TaskDetailsModal component for viewing task details in a read-only modal, and integrate it into the Todo list and team tasks UIs with an eye-button trigger.
Why are these changes being made?
Provide an accessible, view-only details dialog to quickly inspect task information from lists without editing. (If any broader UX decisions or limitations exist, they are not reflected in the code; this PR focuses on read-only detail viewing via a modal.)