feat(activity-feed-v2): swap in TaskModalV2 and scroll to new tasks#4671
Conversation
- Replace v1 TaskModal with TaskModalV2 in ActivityFeedV2 - Adapt loosely typed createTask/onTaskUpdate props to strict callbacks - Extend post-mutation scroll snapshot to include newly created tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughActivityFeedV2 replaces the legacy TaskModal with TaskModalV2, updates task state and error handling, extends scroll targeting to authored task items, and wires new create/edit adapters plus matching test coverage. ChangesTaskModalV2 Migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx (1)
505-533: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deduplicating the shared
TaskModalV2props.The
edit/createbranches repeatcreateTask,error,fetchAvatarUrls,fetchUsers,isOpen,onClose,onSubmitError,onSubmitSuccess, andtaskType. Extracting these into a shared object and spreading it into each branch would reduce duplication while still satisfying the discriminated union.♻️ Proposed refactor
+ {(() => { + const sharedModalProps = { + createTask: handleCreateTask, + error: taskError, + fetchAvatarUrls, + fetchUsers, + isOpen: isTaskFormOpen, + onClose: handleTaskModalClose, + onSubmitError: setTaskError, + onSubmitSuccess: handleTaskModalClose, + taskType, + }; + return editingTask ? ( + <TaskModalV2 + {...sharedModalProps} + editingAssignees={editingAssignees?.entries ?? []} + editingTask={editingTask} + editTask={handleEditTask} + mode="edit" + /> + ) : ( + <TaskModalV2 {...sharedModalProps} /> + ); + })()} - {editingTask ? ( - <TaskModalV2 - createTask={handleCreateTask} - editingAssignees={editingAssignees?.entries ?? []} - editingTask={editingTask} - editTask={handleEditTask} - error={taskError} - fetchAvatarUrls={fetchAvatarUrls} - fetchUsers={fetchUsers} - isOpen={isTaskFormOpen} - mode="edit" - onClose={handleTaskModalClose} - onSubmitError={setTaskError} - onSubmitSuccess={handleTaskModalClose} - taskType={taskType} - /> - ) : ( - <TaskModalV2 - createTask={handleCreateTask} - error={taskError} - fetchAvatarUrls={fetchAvatarUrls} - fetchUsers={fetchUsers} - isOpen={isTaskFormOpen} - onClose={handleTaskModalClose} - onSubmitError={setTaskError} - onSubmitSuccess={handleTaskModalClose} - taskType={taskType} - /> - )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx` around lines 505 - 533, The `TaskModalV2` render branches in `ActivityFeedV2` duplicate most props between edit and create modes. Extract the shared props into a common object near `handleTaskModalClose`/`handleCreateTask`, then spread that object into both `TaskModalV2` branches while keeping the edit-only `editingTask`, `editingAssignees`, `editTask`, and `mode` props only in the edit branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx`:
- Around line 505-533: The `TaskModalV2` render branches in `ActivityFeedV2`
duplicate most props between edit and create modes. Extract the shared props
into a common object near `handleTaskModalClose`/`handleCreateTask`, then spread
that object into both `TaskModalV2` branches while keeping the edit-only
`editingTask`, `editingAssignees`, `editTask`, and `mode` props only in the edit
branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0f2ecbf2-aec5-4a70-bd98-666dc586707c
📒 Files selected for processing (1)
src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx
- Mock TaskModalV2 to capture props via Partial<TaskModalV2Props> - Assert create-mode props, adapter arg forwarding, and unavailable-callback path - Assert scroll behavior after creating tasks (own vs stranger, missing target)
Merge Queue Status
This pull request spent 9 seconds in the queue, including 2 seconds running CI. Required conditions to merge
|
Summary
TaskModalforTaskModalV2insideActivityFeedV2, using the flat prop shape and discriminatedmodeunion.ActivityFeedV2Props.createTask/onTaskUpdateto the strictCreateTaskCallback/EditTaskCallbackvia small adapters; return a syntheticElementsXhrErrorwhen either is unwired so the modal never gets stuck in a submitting state.taskType: string->TaskType;taskError: Error | null->ElementsXhrError | undefined.Test plan
yarn test --watchAll=false --testPathPattern="activity-feed-v2"(11 suites, 328 tests) pass.createTaskprop: modal surfaces an error and returns to an interactive state instead of hanging.onTaskUpdateprop: same as above for edit path.TaskModalreferences remain inactivity-feed-v2/.Summary by CodeRabbit
New Features
Bug Fixes
Tests