Conversation
Implemented a comprehensive redesign of the Task Panel, creating several new React components for a more modern and feature-rich user experience.
Key changes and components:
1. **`NewTaskView.tsx`**: Main container for the redesigned task detail view. Manages selected task data and orchestrates sub-components. Includes basic keyboard navigation (J/K scrolling, Escape).
2. **`TaskHeader.tsx`**: Displays task title (editable), custom checkbox for completion, collapsible section, and 'Link Task' quick action.
3. **`TaskDescription.tsx`**: Handles display and editing of task descriptions with markdown-like styling and 'Attach File' quick action.
4. **`SubTasksList.tsx`**: Manages sub-tasks (CRUD operations), including a completion pie chart and visibility toggle for completed items.
5. **`TaskComments.tsx`**: Provides a chat-like interface for task comments, with user avatars, timestamps, sticky input with auto-expansion, and Ctrl+Enter submission. (Unit tests for this component are currently facing issues with async operations and timers).
6. **`TaskAttributesSidebar.tsx`**: Dedicated sidebar for task metadata (Assignee, Due Date, Priority, Status, Category, Tags) with interactive editing.
7. **`TaskAssignment.tsx`**: Component for selecting a user to assign to a task, used within the attributes sidebar.
8. **Integration with `TaskManager.tsx`**: Replaced the old `TaskDetailPanel` with `NewTaskView.tsx`. Implemented a slide-in animation for the new view and dynamic resizing of the main task list area.
9. **Styling**: Applied consistent styling using TailwindCSS and global theme settings, focusing on visual clarity, modern aesthetics, and improved user interaction cues.
10. **Unit Testing Setup**:
* Installed Jest, React Testing Library, `jest-axe`, and other necessary testing dependencies.
* Configured Jest (`jest.config.js`, `jest.setup.js`, `tsconfig.jest.json`) with mocks for timers, UUID, and assets.
* Added test scripts to `package.json`.
11. **Unit Tests Written**:
* `TaskHeader.test.tsx`: Comprehensive tests for rendering, interactions, and accessibility (all passing).
* `TaskDescription.test.tsx`: Tests for rendering, editing, and accessibility (all passing).
* `SubTasksList.test.tsx`: Tests for sub-task management and display (all passing).
* `TaskComments.test.tsx`: Tests initiated, but encountering issues with Jest's fake timers and `act()` not correctly handling asynchronous state updates within `useEffect` and event handlers using `setTimeout`. This results in elements not appearing as expected and React warnings.
**Current Status & Blockers:**
The majority of the new components are implemented and styled. Unit tests for several key components are passing.
The primary blocker is resolving the asynchronous testing issues in `TaskComments.test.tsx`. The interaction between Jest's fake timers, RTL's `act`, and React's `useEffect` with `setTimeout` is proving difficult to get right, leading to failing tests and React warnings about state updates not being wrapped in `act`.
Further work is needed to:
* Fix the `TaskComments.test.tsx` unit tests.
* Write unit tests for `TaskAttributesSidebar.tsx` and `TaskAssignment.tsx`.
* Write integration tests for `NewTaskView.tsx`.
* Add snapshot tests as planned.
* Conduct thorough manual testing and address any remaining UI/UX refinements.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @birdup000, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant architectural and functional upgrade to the application's task management feature. By refactoring the task detail view into a set of specialized, interactive components, it aims to improve user experience with richer features and a more dynamic interface. Concurrently, it establishes a comprehensive testing framework to support ongoing development and ensure high code quality.
Highlights
- Task Detail Panel Redesign: The core of this PR is a complete overhaul of the task detail panel. The previous monolithic
TaskDetailPanelcomponent has been replaced with a new, modularNewTaskViewcomponent, providing a more structured and maintainable approach to displaying and editing task information. - Modular UI Components: Several new, dedicated React components have been introduced to handle specific aspects of task details. These include
TaskHeader(for title and completion),TaskDescription(for editable task descriptions),SubTasksList(for managing sub-tasks),TaskComments(for activity and comments),TaskAssignment(for assigning tasks), andTaskAttributesSidebar(for managing various task metadata like due date, priority, and tags). - Enhanced Task Functionality & UX: The redesign brings enhanced functionality such as support for sub-tasks, task assignment, and inline editing for key fields like title and description. Updates are often debounced for a smoother user experience, and the task view now includes basic keyboard navigation and a responsive layout that dynamically adjusts based on panel visibility.
- Comprehensive Testing Infrastructure: This PR sets up a robust testing environment by integrating Jest and React Testing Library. It includes new unit tests for
SubTasksList,TaskComments,TaskDescription, andTaskHeader, along withjest-axefor accessibility testing, ensuring the quality and maintainability of the new UI components.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a major redesign of the task panel, replacing the old TaskDetailPanel with a more modular NewTaskView composed of several new sub-components. The new design is responsive and includes features like sub-tasks, task assignment, and inline editing. A comprehensive suite of tests using Jest, React Testing Library, and jest-axe has also been added, which is a fantastic addition to ensure quality and accessibility.
My feedback focuses on removing some remaining mock data and placeholder functionality, cleaning up some dead code from the old implementation, and refining a few implementation details for better performance and maintainability. Overall, this is a very strong contribution.
| > | ||
| <TaskDescription task={task} onUpdateTask={handleUpdateTask} /> | ||
| <SubTasksList parentTask={task} onUpdateParentTask={handleUpdateTask} /> | ||
| <TaskComments taskId={task.id} currentUser={{id: 'user-1', name: 'Current User', avatarUrl: 'https://i.pravatar.cc/150?u=current'}} /> |
There was a problem hiding this comment.
The currentUser prop is hardcoded. This is acceptable for development and testing, but it must be replaced with a mechanism to fetch the actual authenticated user's data before this feature is considered complete.
| const mockUsers: User[] = [ | ||
| { id: 'user-1', name: 'John Smith', avatarUrl: 'https://i.pravatar.cc/150?u=john' }, | ||
| { id: 'user-2', name: 'Jane Doe', avatarUrl: 'https://i.pravatar.cc/150?u=jane' }, | ||
| { id: 'user-3', name: 'Alice Wonderland', avatarUrl: 'https://i.pravatar.cc/150?u=alice' }, | ||
| { id: 'user-4', name: 'Bob The Builder', avatarUrl: 'https://i.pravatar.cc/150?u=bob' }, | ||
| { id: 'user-unassigned', name: 'Unassigned', avatarUrl: '' }, // For unassigning | ||
| ]; |
There was a problem hiding this comment.
The mockUsers array is hardcoded. This is fine for development, but it should be replaced with a mechanism to fetch real user data from an API or a central state management store before this feature is considered complete.
| <div | ||
| className={`fixed inset-y-0 right-0 md:relative md:inset-auto bg-bg-secondary border-l border-border-default shadow-xl md:shadow-none | ||
| transform transition-transform duration-300 ease-in-out z-20 | ||
| ${selectedTaskId ? 'translate-x-0' : 'translate-x-full md:translate-x-0 md:hidden'}`} // Control visibility and slide-in |
There was a problem hiding this comment.
The class md:hidden seems to make the md:translate-x-0 redundant when selectedTaskId is falsy on medium and larger screens. While this works, the combination of translate-x-full md:translate-x-0 md:hidden could potentially be simplified. For instance, you could control visibility primarily with md:flex and md:hidden and let the transform handle the mobile animation. This is a minor point on code clarity, and the current implementation is functional.
| setLoading(true); | ||
| setError(null); | ||
| const currentTask = tasks.find(t => t.id === taskId); | ||
| if (currentTask) { | ||
| setTask(currentTask); | ||
| // Focus the main content area when a new task is loaded for keyboard navigation | ||
| mainContentRef.current?.focus(); | ||
| } else { | ||
| setError('Task not found.'); | ||
| } | ||
| setLoading(false); |
There was a problem hiding this comment.
The setLoading(true) and setLoading(false) calls wrap a synchronous operation (tasks.find()). This is unnecessary and can cause a brief, pointless flicker of a loading indicator if one were present. The component should rely on the loading state from the useTasks hook for asynchronous data fetching, and local state should be used for component-specific async operations, not synchronous ones.
| }; | ||
|
|
||
| const deleteSubTask = (subTaskId: string) => { | ||
| if (window.confirm("Are you sure you want to delete this sub-task?")) { |
There was a problem hiding this comment.
window.confirm() is a blocking browser API that provides a poor user experience and doesn't match the application's modern design. Consider replacing this with a custom, non-blocking confirmation modal component for a more integrated and user-friendly experience.
| onDuplicate={() => { | ||
| // Implement duplicate logic here or call a prop | ||
| alert(`Duplicate task: ${contextMenuTaskId}`); | ||
| handleMenuClose(); | ||
| }} | ||
| onCopyLink={() => { | ||
| // Implement copy link logic | ||
| navigator.clipboard.writeText(`${window.location.origin}/task/${contextMenuTaskId}`); | ||
| alert(`Link copied for task: ${contextMenuTaskId}`); | ||
| handleMenuClose(); | ||
| }} |
There was a problem hiding this comment.
The onDuplicate and onCopyLink handlers use alert() as a placeholder. This should be replaced with a proper implementation. For example, "Copy Link" should provide user feedback via a non-blocking toast notification (e.g., "Link copied to clipboard!"), and the duplication logic needs to be fully implemented.
1 participant
No description provided.