Skip to content

feat: Add task details modal with view-only functionality #199

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.

Sign up for GitHub

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

Open
vibhor121 wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat: Add task details modal with view-only functionality #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
232 changes: 232 additions & 0 deletions components/task-details-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
'use client'

import { TTask } from '@/api/tasks/tasks.types'
import { Button } from '@/components/ui/button'
import {
Dialog,
DialogContent,
DialogHeader,
DialogTitle,
DialogTrigger,
} from '@/components/ui/dialog'
import { Label } from '@/components/ui/label'
import { Separator } from '@/components/ui/separator'
import { Textarea } from '@/components/ui/textarea'
import { DateFormats, DateUtil } from '@/lib/date-util'
import { CalendarIcon, ClockIcon, TagIcon, UserIcon, UserPlusIcon } from 'lucide-react'
import { useState } from 'react'
import { TaskPriorityLabel } from './task-priority-label'
import { TodoLabelsList } from './todo-labels-list'
import { TodoStatusTable } from './todo-status-table'

type TaskDetailsModalProps = {
task: TTask
children: React.ReactNode
}
Comment on lines +22 to +25
Copy link

@coderabbitai coderabbitai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Comment on lines +22 to +25
Copy link

@korbit-ai korbit-ai bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Props Documentation category 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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


export function TaskDetailsModal({ task, children }: TaskDetailsModalProps) {
Copy link

@korbit-ai korbit-ai bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Component Documentation category 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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

const [isOpen, setIsOpen] = useState(false)
Copy link

@korbit-ai korbit-ai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modal State Control Limitation category Design

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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


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"
Comment on lines +60 to +62
Copy link

@korbit-ai korbit-ai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsanitized Task Description Rendering category Security

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) => ({
    '<': '&lt;',
    '>': '&gt;',
    '"': '&quot;',
    "'": '&#39;',
    '&': '&amp;'
  }[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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

/>
</div>
Comment on lines +59 to +64
Copy link

@coderabbitai coderabbitai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
<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.

</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'}
Copy link
Contributor

@graphite-app graphite-app bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
{task.assignee.assignee_name?.charAt(0).toUpperCase() || 'U'}
{task.assignee?.assignee_name?.charAt(0).toUpperCase() || 'U'}

Spotted by Diamond

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.

Copy link

@korbit-ai korbit-ai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repeated Complex User Initial Logic category Readability

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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

</div>
<span>{task.assignee.assignee_name || 'Unknown User'}</span>
</div>
) : (
'Unassigned'
)}
</div>
</div>
Comment on lines +95 to +113
Copy link

@korbit-ai korbit-ai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated User Avatar Logic category Design

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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


{/* 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'}
Copy link

@korbit-ai korbit-ai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant String Operations category Performance

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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

</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)
Copy link

@korbit-ai korbit-ai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inefficient Date Formatting category Performance

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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

) : (
'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
Comment on lines +153 to +154
Copy link

@korbit-ai korbit-ai bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect icon for Task ID field category Design

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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

</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"
>
Comment on lines +182 to +186
Copy link

@korbit-ai korbit-ai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inefficient Array Key Usage category Performance

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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

{tag}
</span>
))}
Comment on lines +176 to +189
Copy link

@coderabbitai coderabbitai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Comment on lines +182 to +189
Copy link

@korbit-ai korbit-ai bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstable React keys using array index category Functionality

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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

</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)}
Copy link
Contributor

@graphite-app graphite-app bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
{new DateUtil(task.deferredDetails.deferredTill).format(DateFormats.D_MMM_YYYY)}
{new DateUtil(task.deferredDetails?.deferredTill).format(DateFormats.D_MMM_YYYY)}

Spotted by Diamond

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.

</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>
)
Comment on lines +27 to +231
Copy link

@korbit-ai korbit-ai bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Component with Multiple Responsibilities category Design

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

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

}
9 changes: 9 additions & 0 deletions components/todo-list-table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ import { TTask } from '@/api/tasks/tasks.types'
import { useAuth } from '@/hooks/useAuth'
import { DateFormats, DateUtil } from '@/lib/date-util'
import { DashboardTasksTableTabs } from '@/modules/dashboard/constants'
import { EyeIcon } from 'lucide-react'
import { usePathname, useRouter, useSearchParams } from 'next/navigation'
import { EditTodoButton } from './edit-task-button'
import { IncludeDoneSwitch } from './include-done-switch'
import { Searchbar } from './searchbar'
import { Shimmer } from './Shimmer'
import { TaskDetailsModal } from './task-details-modal'
import { TaskPriorityLabel } from './task-priority-label'
import { TodoLabelsList } from './todo-labels-list'
import { TodoStatusTable } from './todo-status-table'
import { Button } from './ui/button'
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from './ui/table'
import { WatchListButton } from './watchlist-button'

Expand Down Expand Up @@ -96,6 +99,12 @@ const TodoListTableRow = ({
<TableCell>
{showActions ? (
<div className="flex items-center gap-0.5">
<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>
Comment on lines +102 to +107
Copy link

@coderabbitai coderabbitai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
<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.

{isEditTodoVisible && <EditTodoButton todo={todo} />}
<WatchListButton taskId={todo.id} isInWatchlist={todo.in_watchlist} />
</div>
Expand Down
13 changes: 11 additions & 2 deletions modules/teams/team-tasks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,27 @@

import { USER_TYPE_ENUM } from '@/api/common/common-enum'
import { TasksApi } from '@/api/tasks/tasks.api'
import { TASK_STATUS_ENUM } from '@/api/tasks/tasks.enum'
import { GetTaskReqDto, TTask } from '@/api/tasks/tasks.types'
import { TeamsApi } from '@/api/teams/teams.api'
import { TTeam } from '@/api/teams/teams.type'
import { EditTodoButton } from '@/components/edit-task-button'
import { IncludeDoneSwitch } from '@/components/include-done-switch'
import { ReassignUser } from '@/components/reassign-user'
import { Searchbar } from '@/components/searchbar'
import { TaskDetailsModal } from '@/components/task-details-modal'
import { TaskPriorityLabel } from '@/components/task-priority-label'
import { TodoLabelsList } from '@/components/todo-labels-list'
import { TodoListTableHeader, TodoListTableRowShimmer } from '@/components/todo-list-table'
import { TodoStatusTable } from '@/components/todo-status-table'
import { IncludeDoneSwitch } from '@/components/include-done-switch'
import { Button } from '@/components/ui/button'
import { Table, TableBody, TableCell, TableRow } from '@/components/ui/table'
import { WatchListButton } from '@/components/watchlist-button'
import { useAuth } from '@/hooks/useAuth'
import { DateFormats, DateUtil } from '@/lib/date-util'
import { useQuery } from '@tanstack/react-query'
import { EyeIcon } from 'lucide-react'
import { usePathname, useRouter, useSearchParams } from 'next/navigation'
import { TASK_STATUS_ENUM } from '@/api/tasks/tasks.enum'

const QUERY_PARAMS_KEYS = {
search: 'search',
Expand Down Expand Up @@ -61,6 +64,12 @@ const TodoListTableRow = ({ todo, team }: TodoListTableRowProps) => {
</TableCell>

<TableCell className="flex items-center gap-0.5">
<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>
Comment on lines +67 to +72
Copy link

@coderabbitai coderabbitai bot Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
<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.

{isRessignTodoCtaVisible && <ReassignUser taskId={todo.id} teamId={team.id} />}
{isEditTodoVisible && <EditTodoButton todo={todo} teamId={team?.id} />}
{!isRessignTodoCtaVisible && (
Expand Down