feat: implement completed features management in BoardView and Kanban…#97
feat: implement completed features management in BoardView and Kanban…#97webdevcody merged 6 commits intomainfrom
Conversation
…Card - Added functionality to complete and unarchive features, allowing users to manage feature statuses effectively. - Introduced a modal to display completed features, enhancing user experience by providing a dedicated view for archived items. - Updated KanbanCard to include buttons for completing features and managing their states, improving interactivity and workflow. - Modified the Feature interface to include a new "completed" status, ensuring comprehensive state management across the application.
Summary of ChangesHello @webdevcody, 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 robust system for managing the lifecycle of features, specifically focusing on the completion and archiving process. It provides users with the ability to mark features as done, view them in a centralized archive, and restore them if necessary, significantly improving task organization and workflow flexibility within the application. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
There was a problem hiding this comment.
Code Review
This pull request successfully implements the management of completed features, including archiving, restoring, and deleting them through a new modal. The Kanban card has been updated with new actions to support this workflow. The code is generally well-structured, but I've identified a couple of areas with code duplication that could be refactored for better maintainability. I've also suggested a small improvement to the user experience in the delete confirmation dialog. Overall, this is a great addition to the application.
| {!isCurrentAutoTask && feature.status === "waiting_approval" && ( | ||
| <div className="absolute top-2 right-2 flex items-center gap-1"> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="h-6 w-6 p-0 hover:bg-white/10 text-muted-foreground hover:text-foreground" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onEdit(); | ||
| }} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| data-testid={`edit-waiting-${feature.id}`} | ||
| title="Edit" | ||
| > | ||
| <Edit className="w-4 h-4" /> | ||
| </Button> | ||
| {onViewOutput && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="h-6 w-6 p-0 hover:bg-white/10 text-muted-foreground hover:text-foreground" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onViewOutput(); | ||
| }} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| data-testid={`logs-waiting-${feature.id}`} | ||
| title="Logs" | ||
| > | ||
| <FileText className="w-4 h-4" /> | ||
| </Button> | ||
| )} | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="h-6 w-6 p-0 hover:bg-white/10 text-muted-foreground hover:text-destructive" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| handleDeleteClick(e); | ||
| }} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| data-testid={`delete-waiting-${feature.id}`} | ||
| title="Delete" | ||
| > | ||
| <Trash2 className="w-4 h-4" /> | ||
| </Button> | ||
| </div> | ||
| )} | ||
| {!isCurrentAutoTask && feature.status === "verified" && ( | ||
| <div className="absolute top-2 right-2 flex items-center gap-1"> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="h-6 w-6 p-0 hover:bg-white/10 text-muted-foreground hover:text-foreground" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onEdit(); | ||
| }} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| data-testid={`edit-verified-${feature.id}`} | ||
| title="Edit" | ||
| > | ||
| <Edit className="w-4 h-4" /> | ||
| </Button> | ||
| {onViewOutput && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="h-6 w-6 p-0 hover:bg-white/10 text-muted-foreground hover:text-foreground" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onViewOutput(); | ||
| }} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| data-testid={`logs-verified-${feature.id}`} | ||
| title="Logs" | ||
| > | ||
| <FileText className="w-4 h-4" /> | ||
| </Button> | ||
| )} | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="h-6 w-6 p-0 hover:bg-white/10 text-muted-foreground hover:text-destructive" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| handleDeleteClick(e); | ||
| }} | ||
| onPointerDown={(e) => e.stopPropagation()} | ||
| data-testid={`delete-verified-${feature.id}`} | ||
| title="Delete" | ||
| > | ||
| <Trash2 className="w-4 h-4" /> | ||
| </Button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
There's significant code duplication here for handling actions on waiting_approval and verified cards. The JSX for the action buttons is identical in both blocks. You can combine these into a single conditional block using || to improve maintainability and reduce redundancy.
{!isCurrentAutoTask && (feature.status === "waiting_approval" || feature.status === "verified") && (
<div className="absolute top-2 right-2 flex items-center gap-1">
<Button
variant="ghost"
size="sm"
className="h-6 w-6 p-0 hover:bg-white/10 text-muted-foreground hover:text-foreground"
onClick={(e) => {
e.stopPropagation();
onEdit();
}}
onPointerDown={(e) => e.stopPropagation()}
data-testid={`edit-${feature.status === "waiting_approval" ? "waiting" : "verified"}-${feature.id}`}
title="Edit"
>
<Edit className="w-4 h-4" />
</Button>
{onViewOutput && (
<Button
variant="ghost"
size="sm"
className="h-6 w-6 p-0 hover:bg-white/10 text-muted-foreground hover:text-foreground"
onClick={(e) => {
e.stopPropagation();
onViewOutput();
}}
onPointerDown={(e) => e.stopPropagation()}
data-testid={`logs-${feature.status === "waiting_approval" ? "waiting" : "verified"}-${feature.id}`}
title="Logs"
>
<FileText className="w-4 h-4" />
</Button>
)}
<Button
variant="ghost"
size="sm"
className="h-6 w-6 p-0 hover:bg-white/10 text-muted-foreground hover:text-destructive"
onClick={(e) => {
e.stopPropagation();
handleDeleteClick(e);
}}
onPointerDown={(e) => e.stopPropagation()}
data-testid={`delete-${feature.status === "waiting_approval" ? "waiting" : "verified"}-${feature.id}`}
title="Delete"
>
<Trash2 className="w-4 h-4" />
</Button>
</div>
)}
| onImplement={async () => { | ||
| // Check concurrency limit | ||
| if (!autoMode.canStartNewTask) { | ||
| toast.error("Concurrency limit reached", { | ||
| description: `You can only have ${ | ||
| autoMode.maxConcurrency | ||
| } task${ | ||
| autoMode.maxConcurrency > 1 ? "s" : "" | ||
| } running at a time. Wait for a task to complete or increase the limit.`, | ||
| }); | ||
| return; | ||
| } | ||
| // Update with startedAt timestamp | ||
| const updates = { | ||
| status: "in_progress" as const, | ||
| startedAt: new Date().toISOString(), | ||
| }; | ||
| updateFeature(feature.id, updates); | ||
| persistFeatureUpdate(feature.id, updates); | ||
| console.log( | ||
| "[Board] Feature moved to in_progress via Implement button, starting agent..." | ||
| ); | ||
| await handleRunFeature(feature); | ||
| }} |
There was a problem hiding this comment.
The logic for implementing a feature is duplicated here and in handleDragEnd. To improve maintainability and reduce code duplication, you could extract this logic into a new helper function, for example handleStartImplementation(feature).
This new function could look like this:
const handleStartImplementation = async (feature: Feature) => {
if (!autoMode.canStartNewTask) {
toast.error("Concurrency limit reached", {
description: `You can only have ${
autoMode.maxConcurrency
} task${
autoMode.maxConcurrency > 1 ? "s" : ""
} running at a time. Wait for a task to complete or increase the limit.`,
});
return;
}
const updates = {
status: "in_progress" as const,
startedAt: new Date().toISOString(),
};
updateFeature(feature.id, updates);
persistFeatureUpdate(feature.id, updates);
console.log(
"[Board] Feature moved to in_progress, starting agent..."
);
await handleRunFeature(feature);
};This new function could then be called from both onImplement and handleDragEnd.
| onClick={() => { | ||
| if (deleteCompletedFeature) { | ||
| handleDeleteFeature(deleteCompletedFeature.id); | ||
| setDeleteCompletedFeature(null); | ||
| } | ||
| }} |
There was a problem hiding this comment.
The handleDeleteFeature function is async and can be slow if it needs to stop a running agent. However, it's not being awaited here. This can lead to a confusing user experience where the confirmation dialog closes instantly, but the feature item might linger in the UI until the async operation completes. It would be better to await the function call so the dialog remains open until the deletion is fully processed. You might also consider adding a loading state to the button for better user feedback.
| onClick={() => { | |
| if (deleteCompletedFeature) { | |
| handleDeleteFeature(deleteCompletedFeature.id); | |
| setDeleteCompletedFeature(null); | |
| } | |
| }} | |
| onClick={async () => { | |
| if (deleteCompletedFeature) { | |
| await handleDeleteFeature(deleteCompletedFeature.id); | |
| setDeleteCompletedFeature(null); | |
| } | |
| }} |
… KanbanCard - Introduced a helper function, handleStartImplementation, to manage concurrency checks and feature status updates when moving features from backlog to in_progress. - Simplified the onImplement callback in KanbanCard to utilize the new helper function, enhancing code readability and maintainability. - Removed redundant concurrency checks from multiple locations, centralizing the logic for better consistency and reducing code duplication.
- Added a new ProjectSetupDialog component to facilitate project specification generation, enhancing user experience by guiding users through project setup. - Refactored the Sidebar component to integrate the new ProjectSetupDialog, replacing the previous inline dialog implementation for improved code organization and maintainability. - Updated the sidebar to handle project overview and feature generation options, streamlining the project setup process. - Removed the old dialog implementation from the Sidebar, reducing code duplication and improving clarity.
…rove BoardView layout - Added missing onOpenChange call in ProjectSetupDialog to ensure proper state management. - Reformatted the COLUMNS array in BoardView for improved readability and consistency. - Adjusted DragOverlay component's formatting for better code clarity.
- Simplified the sidebar button's class structure by removing unnecessary overflow styling. - Enhanced the visual representation of the trashed projects count with updated styling for better visibility. - Wrapped the dropdown menu's subcontent in a portal for improved rendering and performance.
…Card
resolve