feat(activity-feed-v2): add TaskFormV2 with blueprint primitives and user-selector#4662
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds ChangesTaskFormV2 Component
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.tsx`:
- Around line 65-67: The TaskFormV2 edit flow is incorrectly treating overdue
existing due dates as invalid because the minimum selectable date is always
anchored to today. Update the due-date handling in TaskFormV2 so edit mode
allows an initialDueDate in the past to remain selectable and editable, while
still enforcing today as the minimum only for creating new tasks. Use the
existing dueDate state, initialDueDate, and the date picker/min-date logic in
TaskFormV2 to conditionally derive the minimum date based on whether the form is
editing or creating.
- Around line 84-96: The TaskFormV2 handleFormSubmit path still allows submit
when isDisabled is true, so add an early return in handleFormSubmit before any
validation or onSubmit call. Use the existing isDisabled prop in TaskFormV2 to
block submission regardless of pre-seeded fields, while keeping the current
selectedUsers/message checks and onSubmit payload unchanged for enabled forms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 326fe8e9-088e-4cfb-acc8-49928b979550
📒 Files selected for processing (4)
src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.scsssrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.tsxsrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/__tests__/TaskFormV2.test.tsxsrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/messages.ts
jackiejou
force-pushed
the
feat/task-modal-v2-form
branch
from
1a417b1 to
40c1844
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/elements/content-sidebar/activity-feed-v2/task-modal-v2/__tests__/TaskFormV2.test.tsx (1)
27-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpose callback hooks in the mocks.
These mocks are fully static, so the suite never drives
onSelectedUsersChangeor theDatePickerchange path. That means the newUserContactType[]state updates andDateValue→Datesubmit conversion can regress while these tests still pass. Add a minimal trigger in each mock and cover one interactive assignee change plus one due-date submit.🤖 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/task-modal-v2/__tests__/TaskFormV2.test.tsx` around lines 27 - 60, The test mocks for UserSelectorContainer and DatePicker are static, so they never exercise the onSelectedUsersChange callback or the DatePicker change path. Update these mocks to expose minimal interaction hooks that let tests trigger assignee selection and due-date changes, then add coverage in TaskFormV2.test.tsx for one UserContactType[] update and one DateValue-to-Date submit flow. Use the existing mock identifiers UserSelectorContainer, DatePicker, and the lastUserSelectorProps capture to locate the changes.
🤖 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.
Inline comments:
In `@src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.tsx`:
- Around line 42-54: Only carry forward a seeded due-date time when the form is
in edit mode, not whenever initialDueDate exists. Update TaskFormV2 so the logic
around initialDueDate/initialDueDateTime only sets originalDueDateTime for
existing tasks, while create mode still falls through to toSubmitDate’s
end-of-day normalization. Use the TaskFormV2 submission flow and the
toSubmitDate helper to keep new-task dates at 23:59:59.999 and preserve the
original time only for true edits.
---
Nitpick comments:
In
`@src/elements/content-sidebar/activity-feed-v2/task-modal-v2/__tests__/TaskFormV2.test.tsx`:
- Around line 27-60: The test mocks for UserSelectorContainer and DatePicker are
static, so they never exercise the onSelectedUsersChange callback or the
DatePicker change path. Update these mocks to expose minimal interaction hooks
that let tests trigger assignee selection and due-date changes, then add
coverage in TaskFormV2.test.tsx for one UserContactType[] update and one
DateValue-to-Date submit flow. Use the existing mock identifiers
UserSelectorContainer, DatePicker, and the lastUserSelectorProps capture to
locate the changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85c9d31c-ba2f-4203-87e7-9f216ba55b97
📒 Files selected for processing (5)
src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.scsssrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.tsxsrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskModalV2.scsssrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/__tests__/TaskFormV2.test.tsxsrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/messages.ts
✅ Files skipped from review due to trivial changes (2)
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.scss
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskModalV2.scss
jackiejou
force-pushed
the
feat/task-modal-v2-form
branch
from
40c1844 to
411e34a
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/elements/content-sidebar/activity-feed-v2/task-modal-v2/__tests__/TaskFormV2.test.tsx (2)
49-67: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a submit-path test for due-date round-tripping.
The date tests stop at
minValue. They never driveDatePickerthroughonChangeand assert the submitteddueDate, so theDateValue→Dateconversion inTaskFormV2is still unguarded.Also applies to: 158-173, 240-257
🤖 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/task-modal-v2/__tests__/TaskFormV2.test.tsx` around lines 49 - 67, Add a submit-path test in TaskFormV2.test.tsx that drives the mocked DatePicker through onChange and verifies the form submits the expected dueDate. Use the existing DatePicker mock/lastDatePickerProps setup to simulate a DateValue selection, then submit the form and assert the TaskFormV2 onSubmit payload contains the converted Date dueDate. Keep the test aligned with the existing date-related cases around DatePicker, minValue, and TaskFormV2 so the DateValue-to-Date round-trip is covered.
27-47: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDrive assignee updates through the mock.
All assignee-path assertions here depend on
initialAssignees, so the suite never proves that a post-mountonSelectedUsersChangeupdatesselectedUsers, clears validation, or recomputes the completion-rule checkbox state.Suggested test hook
type UserSelectorMockProps = { disabled?: boolean; error?: string; label?: React.ReactNode; onSelectedUsersChange?: (users: UserContactType[]) => void; selectedUsers?: UserContactType[]; }; jest.mock('`@box/user-selector`', () => ({ UserSelectorContainer: (props: UserSelectorMockProps) => { lastUserSelectorProps = props; return ( <div data-testid="user-selector"> + <button + type="button" + onClick={() => + props.onSelectedUsersChange?.([{ name: 'Alice', type: 'user', value: '1' } as UserContactType]) + } + > + select-user + </button> <label> {props.label} <input aria-label="assignee input" data-testid="user-selector-input" /> </label>Also applies to: 131-148, 223-237
🤖 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/task-modal-v2/__tests__/TaskFormV2.test.tsx` around lines 27 - 47, The TaskFormV2 tests are still asserting only the initial assignee state, so they never verify post-mount updates from the UserSelectorContainer mock. Add a test flow that captures the mock props via lastUserSelectorProps and invokes onSelectedUsersChange to simulate assignee changes, then assert selectedUsers updates, validation errors clear, and the completion-rule checkbox state recomputes accordingly. Apply this to the relevant assignee-path cases in TaskFormV2.test.tsx so the suite exercises the actual update path instead of only initialAssignees.
🤖 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/task-modal-v2/__tests__/TaskFormV2.test.tsx`:
- Around line 49-67: Add a submit-path test in TaskFormV2.test.tsx that drives
the mocked DatePicker through onChange and verifies the form submits the
expected dueDate. Use the existing DatePicker mock/lastDatePickerProps setup to
simulate a DateValue selection, then submit the form and assert the TaskFormV2
onSubmit payload contains the converted Date dueDate. Keep the test aligned with
the existing date-related cases around DatePicker, minValue, and TaskFormV2 so
the DateValue-to-Date round-trip is covered.
- Around line 27-47: The TaskFormV2 tests are still asserting only the initial
assignee state, so they never verify post-mount updates from the
UserSelectorContainer mock. Add a test flow that captures the mock props via
lastUserSelectorProps and invokes onSelectedUsersChange to simulate assignee
changes, then assert selectedUsers updates, validation errors clear, and the
completion-rule checkbox state recomputes accordingly. Apply this to the
relevant assignee-path cases in TaskFormV2.test.tsx so the suite exercises the
actual update path instead of only initialAssignees.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74009a32-7374-4a7e-9978-36f850ff96ac
📒 Files selected for processing (5)
src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.scsssrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.tsxsrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskModalV2.scsssrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/__tests__/TaskFormV2.test.tsxsrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/messages.ts
✅ Files skipped from review due to trivial changes (1)
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.scss
🚧 Files skipped from review as they are similar to previous changes (3)
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/messages.ts
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskModalV2.scss
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.tsx
jackiejou
force-pushed
the
feat/task-modal-v2-form
branch
from
411e34a to
dff9877
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/elements/content-sidebar/activity-feed-v2/task-modal-v2/__tests__/TaskFormV2.test.tsx (2)
53-67: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDrive one due-date test through
DatePicker.onChange.The current due-date tests only use
initialDueDate, which bypasses theDateValue→Dateconversion this component owns. If that boundary breaks, this suite stays green. Add a small hook in the mock and assert submission after a synthetic picker change.🤖 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/task-modal-v2/__tests__/TaskFormV2.test.tsx` around lines 53 - 67, The due-date coverage in TaskFormV2.test.tsx only relies on initialDueDate and never exercises the DatePicker boundary, so add a test that drives the form through the mocked DatePicker.onChange path. Update the DatePicker mock in the Jest setup to expose/trigger onChange from DatePickerMockProps, then in one of the TaskFormV2 submit tests simulate a synthetic picker change and assert the submitted due date after the DateValue to Date conversion. Use the existing TaskFormV2 and DatePicker mock symbols to keep the test aligned with the component-owned conversion logic.
27-47: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise
onSelectedUsersChangein the mock.All assignee scenarios here are seeded through
initialAssignees, so the suite never proves that a selector-driven update recomputes completion-rule state or submit payloads correctly. A regression in theUserSelectorContainercallback path would still pass these tests.🤖 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/task-modal-v2/__tests__/TaskFormV2.test.tsx` around lines 27 - 47, The current `UserSelectorContainer` mock in `TaskFormV2.test.tsx` only renders props and never triggers `onSelectedUsersChange`, so selector-driven state updates are untested. Update the mock to expose and invoke `props.onSelectedUsersChange` from the `UserSelectorContainer` stub, then add coverage in the `TaskFormV2` suite for changing assignees through the selector and verifying the recomputed completion-rule state and submit payload. Use the existing `lastUserSelectorProps`/`UserSelectorMockProps` setup to locate the callback path.
🤖 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/task-modal-v2/__tests__/TaskFormV2.test.tsx`:
- Around line 53-67: The due-date coverage in TaskFormV2.test.tsx only relies on
initialDueDate and never exercises the DatePicker boundary, so add a test that
drives the form through the mocked DatePicker.onChange path. Update the
DatePicker mock in the Jest setup to expose/trigger onChange from
DatePickerMockProps, then in one of the TaskFormV2 submit tests simulate a
synthetic picker change and assert the submitted due date after the DateValue to
Date conversion. Use the existing TaskFormV2 and DatePicker mock symbols to keep
the test aligned with the component-owned conversion logic.
- Around line 27-47: The current `UserSelectorContainer` mock in
`TaskFormV2.test.tsx` only renders props and never triggers
`onSelectedUsersChange`, so selector-driven state updates are untested. Update
the mock to expose and invoke `props.onSelectedUsersChange` from the
`UserSelectorContainer` stub, then add coverage in the `TaskFormV2` suite for
changing assignees through the selector and verifying the recomputed
completion-rule state and submit payload. Use the existing
`lastUserSelectorProps`/`UserSelectorMockProps` setup to locate the callback
path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 273f3a82-36ea-40c5-84ff-ec1b3ab0dae3
📒 Files selected for processing (5)
src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.scsssrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.tsxsrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskModalV2.scsssrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/__tests__/TaskFormV2.test.tsxsrc/elements/content-sidebar/activity-feed-v2/task-modal-v2/messages.ts
✅ Files skipped from review due to trivial changes (2)
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.scss
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskModalV2.scss
🚧 Files skipped from review as they are similar to previous changes (2)
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/messages.ts
- src/elements/content-sidebar/activity-feed-v2/task-modal-v2/TaskFormV2.tsx
jackiejou
force-pushed
the
feat/task-modal-v2-form
branch
from
dff9877 to
0006b0d
Compare
JChan106
approved these changes
Jun 29, 2026
jpan-box
approved these changes
Jun 30, 2026
Contributor
Merge Queue Status
This pull request spent 11 minutes 26 seconds in the queue, including 11 minutes 2 seconds running CI. Required conditions to merge
|
This was referenced Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
TaskFormV2, the assignee/message/due-date/completion-rule form for the V2 task modal. Built on@box/blueprint-webprimitives (TextArea,Checkbox,DatePicker) and@box/user-selector'sUserSelectorContainer, this replaces v1'sPillSelectorDropdown+ContactDatalistItemassignee picker with an accessible user-selector that supports both users and groups.This PR adds the form in isolation. It is not yet wired into the modal shell or
ActivityFeedV2— those land in the next PR (S-4), which will render<TaskFormV2>inside<Modal.Body>and render the modal's footer buttons targeting the form viaform="task-form-v2".Changes
task-modal-v2/TaskFormV2.tsx— functional component, exportsTASK_FORM_V2_ID,TaskFormV2Props,TaskFormV2SubmitPayload. Owns its<form>element so the modal footer (S-4) can submit it via theformHTML attribute.task-modal-v2/TaskFormV2.scss— minimal flex layout; Blueprint primitives own their own visual chrome.task-modal-v2/messages.ts— i18n keys for labels, placeholders, errors, and the date-picker's required aria-labels.task-modal-v2/__tests__/TaskFormV2.test.tsx— 13 RTL tests covering all S-3 acceptance criteria.Design notes
UserContactType[], notTaskCollabAssignee[]. The mapper from PR feat(activity-feed-v2): add contact mapping utility for task modal v2 #4657 converts at the API boundary; everything inside the form stays in the user-selector shape. Completion-rule "any vs all" logic usesselectedUsers.some(u => u.type === 'group')andselectedUsers.filter(u => u.type === 'user').length > 1.TASK_FORM_V2_ID = 'task-form-v2'so the modal's footer<Modal.Footer.PrimaryButton form="task-form-v2" type="submit">(added in S-4) can submit it from a sibling DOM tree.react-aria-components-based: values areDateValue(@internationalized/date), not nativeDate. The form converts at the seam (fromDateon init,toDate(localTimeZone)+ end-of-day on submit) so the rest of the codebase only seesDate.activityFeed.tasks.assignToGroupflag dropped as flagged in the plan. V2 uses one tooltip variant unconditionally (groups are always pickable).pointerEvents: 'none'workarounds. The mock preserves the form's behavior contract (label, value, onChange) while staying isolated.UserSelectorContaineris mocked in tests because the container fetches from the live API and renders Radix popovers. The mock capturesonSelectedUsersChangeso tests can simulate selection.Test plan
yarn test --testPathPattern="task-modal-v2"— 35/35 pass (8 shell + 14 mapper + 13 form)npx eslint --max-warnings=0 src/elements/content-sidebar/activity-feed-v2/task-modal-v2/— cleannpx stylelint "src/elements/content-sidebar/activity-feed-v2/task-modal-v2/**/*.scss"— cleannpx tsc --noEmit— cleanScenarios covered in tests:
onSubmitwith the full payload when validALL_ASSIGNEESandANY_ASSIGNEEtype: 'group'initialMessage(edit-mode prefill scaffolding)data-resin-iseditingcorrectly for create vs edit modeisDisabledcascades to assignee selector + message fieldFollow-ups
TaskFormV2intoTaskModalV2with edit-mode prefill and submit handling (next PR — S-4)ActivityFeedV2.tsxto renderTaskModalV2Summary by CodeRabbit