-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Add task pinning functionality #6365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add pinnedTasks field to global settings schema - Implement toggleTaskPin message handler for backend state management - Update ExtensionStateContext to include pinned tasks state - Add pin/unpin button to TaskItemHeader with consistent codicon styling - Update task sorting logic to prioritize pinned tasks first - Prevent deletion of pinned tasks with user-friendly alerts - Add translation keys for pin/unpin functionality - Integrate pinned tasks state throughout the application Resolves #6244
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review: Task Pinning Functionality
This PR implements task pinning functionality with excellent consistency to existing patterns. The implementation follows the codebase conventions well and provides a complete data flow from UI to backend persistence.
Critical Issues (Must Fix)
1. Missing icon differentiation in TaskItemHeader.tsx
- Location: https://github.com/RooCodeInc/Roo-Code/blob/feature/task-pinning/webview-ui/src/components/history/TaskItemHeader.tsx#L55
- Issue: The pin button uses
codicon-pinfor both pinned and unpinned states - Fix: Should use
codicon-pinnedwhenisPinnedis true to provide clear visual feedback - Suggestion:
<span className={codicon ${isPinned ? 'codicon-pinned' : 'codicon-pin'} text-xs opacity-50} />
2. Inconsistent button styling in TaskItemHeader.tsx
- Location: https://github.com/RooCodeInc/Roo-Code/blob/feature/task-pinning/webview-ui/src/components/history/TaskItemHeader.tsx#L52-L54
- Issue: Pin button uses
size-5class instead ofsize="icon"prop - Fix: Should match DeleteButton styling for consistency
- Suggestion: Change
className="size-5 flex items-center justify-center"tosize="icon"
Important Suggestions (Should Consider)
3. User experience improvement in DeleteButton.tsx
- Location: https://github.com/RooCodeInc/Roo-Code/blob/feature/task-pinning/webview-ui/src/components/history/DeleteButton.tsx#L25
- Issue: Using
alert()for pinned task deletion prevention is not ideal UX - Suggestion: Consider using a toast notification or modal dialog for better consistency with the rest of the application
4. Missing test coverage
- Areas: Sorting logic in useTaskSearch.ts and state management in ExtensionStateContext.tsx
- Suggestion: Add unit tests for the new pinning functionality, particularly:
- Pinned task sorting logic (lines 61-86 in useTaskSearch.ts)
- State management functions (lines 489-503 in ExtensionStateContext.tsx)
Minor Improvements (Nice to Have)
5. Code organization in useTaskSearch.ts
- Location: https://github.com/RooCodeInc/Roo-Code/blob/feature/task-pinning/webview-ui/src/components/history/useTaskSearch.ts#L61-L68
- Suggestion: Extract pinned task sorting logic into a separate utility function for better readability and testability
6. Translation completeness
- Location: https://github.com/RooCodeInc/Roo-Code/blob/feature/task-pinning/webview-ui/src/i18n/locales/en/history.json#L45-L47
- Suggestion: Consider adding translations for other supported locales beyond English
Positive Aspects
✅ Excellent consistency with existing API config pinning patterns
✅ Proper TypeScript typing throughout the implementation
✅ Complete data flow from UI interactions to backend persistence
✅ Backward compatibility maintained
✅ ESLint compliant code
✅ Follows existing codebase patterns (similar to API config pinning)
✅ Proper state management with context and global state integration
Summary
This is a solid implementation that addresses the user's request effectively. The critical issues are minor and easily fixable. The functionality works as intended and integrates well with the existing codebase architecture.
Recommendation: Approve after addressing the critical icon differentiation and button styling issues.
|
Closing, this doesn't align with out current roadmap |
Implements task pinning in history section as requested in #6244
Summary
This PR adds the ability to pin/unpin tasks in the task history section to keep important tasks at the top and prevent accidental deletion.
Changes Made
pinnedTasksfield to global settings schema with proper type definitionstoggleTaskPinmessage handler for state managementTechnical Implementation
codicon-pinandcodicon-pinned)Testing
Closes #6244
@daniel-lxs This implements the task pinning functionality you requested with a consistent pin icon as mentioned.
Important
Add task pinning functionality to keep important tasks at the top of the history and prevent their deletion.
pinnedTasksfield toglobalSettingsSchemainglobal-settings.ts.ClineProviderto handlepinnedTasksingetState()andpostStateToWebview().toggleTaskPininwebviewMessageHandlerto updatepinnedTasks.TaskItemHeader.tsxwithtogglePinnedTasklogic.DeleteButton.tsxwith alert message.useTaskSearch.tsto sort pinned tasks first.ExtensionStateContext.tsxwithtogglePinnedTaskfunction.history.json.This description was created by
for a8e9b8f. You can customize this summary. It will automatically update as commits are pushed.