feat(activity-feed-v2): scaffold accessible task modal v2 shell#4656
Conversation
|
Warning Review limit reached
More reviews will be available in 3 minutes and 24 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
WalkthroughAdds ChangesTask modal v2
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/elements/content-sidebar/task-modal-v2/__tests__/TaskModalV2.test.tsx (2)
29-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSource expected labels from
messages.tsinstead of hard-coded English.These tests currently fail on copy-only localization updates even when the title-selection and close-button behavior are still correct. Pull the expected strings from the co-located message descriptors so the suite stays focused on behavior.
Suggested direction
+import messages from '../messages'; + test.each([ - [TASK_TYPE_APPROVAL, TASK_EDIT_MODE_CREATE, 'Create Approval Task'], - [TASK_TYPE_APPROVAL, TASK_EDIT_MODE_EDIT, 'Modify Approval Task'], - [TASK_TYPE_GENERAL, TASK_EDIT_MODE_CREATE, 'Create General Task'], - [TASK_TYPE_GENERAL, TASK_EDIT_MODE_EDIT, 'Modify General Task'], + [TASK_TYPE_APPROVAL, TASK_EDIT_MODE_CREATE, messages.createApprovalTaskTitle.defaultMessage], + [TASK_TYPE_APPROVAL, TASK_EDIT_MODE_EDIT, messages.editApprovalTaskTitle.defaultMessage], + [TASK_TYPE_GENERAL, TASK_EDIT_MODE_CREATE, messages.createGeneralTaskTitle.defaultMessage], + [TASK_TYPE_GENERAL, TASK_EDIT_MODE_EDIT, messages.editGeneralTaskTitle.defaultMessage], ])('renders the correct title for taskType=%s editMode=%s', (taskType, editMode, expectedTitle) => {🤖 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/task-modal-v2/__tests__/TaskModalV2.test.tsx` around lines 29 - 43, The TaskModalV2 tests are asserting hard-coded English titles and should instead use the localized labels from the co-located message descriptors in messages.ts. Update the expectations in TaskModalV2.test.tsx to read the expected heading text from the same message definitions used by the component, so the tests continue to verify title selection and close-button behavior without breaking on copy-only localization changes.
23-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid pinning the shell test to placeholder body copy.
This assertion will need to be rewritten in the very next PR that adds the real form, without increasing confidence in the modal shell itself. Prefer checking stable shell behavior here and let the follow-up PR add form-specific assertions.
🤖 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/task-modal-v2/__tests__/TaskModalV2.test.tsx` around lines 23 - 27, The test in TaskModalV2.test.tsx is asserting placeholder body copy that will disappear once the real form is added. Update the shell test around renderModal() and task-modal-v2 to verify stable modal shell behavior instead, such as visibility/open state or structural elements, and remove the dependency on the “Form goes here” text. Keep form-specific assertions for the follow-up test added with the real form.
🤖 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/task-modal-v2/__tests__/TaskModalV2.test.tsx`:
- Around line 29-43: The TaskModalV2 tests are asserting hard-coded English
titles and should instead use the localized labels from the co-located message
descriptors in messages.ts. Update the expectations in TaskModalV2.test.tsx to
read the expected heading text from the same message definitions used by the
component, so the tests continue to verify title selection and close-button
behavior without breaking on copy-only localization changes.
- Around line 23-27: The test in TaskModalV2.test.tsx is asserting placeholder
body copy that will disappear once the real form is added. Update the shell test
around renderModal() and task-modal-v2 to verify stable modal shell behavior
instead, such as visibility/open state or structural elements, and remove the
dependency on the “Form goes here” text. Keep form-specific assertions for the
follow-up test added with the real form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a6134df-1a48-4824-8a73-6d35d41fb491
⛔ Files ignored due to path filters (1)
i18n/en-US.propertiesis excluded by!i18n/**
📒 Files selected for processing (5)
src/elements/content-sidebar/task-modal-v2/TaskModalV2.scsssrc/elements/content-sidebar/task-modal-v2/TaskModalV2.tsxsrc/elements/content-sidebar/task-modal-v2/__tests__/TaskModalV2.test.tsxsrc/elements/content-sidebar/task-modal-v2/index.tssrc/elements/content-sidebar/task-modal-v2/messages.ts
jackiejou
force-pushed
the
feat/task-modal-v2-shell
branch
2 times, most recently
from
14326ca to
b35a841
Compare
jackiejou
force-pushed
the
feat/task-modal-v2-shell
branch
from
b35a841 to
956db62
Compare
|
Queued — the merge queue status continues in this comment ↓. |
Merge Queue Status
This pull request spent 10 seconds in the queue, including 1 second running CI. Required conditions to merge
|
4 participants
Summary
Scaffolds a new V2 task-creation/edit modal under
src/elements/content-sidebar/task-modal-v2/. This is the first PR in a multi-PR effort to migrate the task modal offPillSelectorDropdown(which has known keyboard-focus and accessible-name gaps) and onto@box/blueprint-webModal +@box/user-selector.Scope of this PR is intentionally narrow: it ships only the modal shell and its tests. The shell renders a placeholder body. Form content, the user-selector swap, and the
ActivityFeedV2integration land in follow-up PRs so each change stays small and reviewable.Changes
src/elements/content-sidebar/task-modal-v2/:TaskModalV2.tsx— functional component, built on@box/blueprint-webModal(Modal.Content+Modal.Header+Modal.Body+Modal.Close).messages.ts— V2-local i18n message keys.index.ts— default export plus exportedTaskModalV2Propstype.TaskModalV2.scss— minimal layout hook; Blueprint Modal owns chrome.__tests__/TaskModalV2.test.tsx— RTL coverage of the shell.TaskModal.js,activity-feed/task-form/,AddTaskButton.js,ActivitySidebar.js) is not touched. No call sites consumeTaskModalV2yet.Design notes
open/onOpenChangeAPI (via Blueprint'sModalwrapper) — noappElementplumbing, no react-modal wrapper. Existing dialogs in the repo (CreateFolderDialog,RenameDialog, etc.) wrap Blueprint chrome inside react-modal; this component skips that layer because Blueprint Modal is fully self-contained.bcs-NewTaskModalrather thanbcs-TaskModalV2because stylelint'sselector-class-patternregex disallows digits. Matches the precedent set bybcs-NewActivityFeed.be.taskModalV2.*) rather than threading through the existing v1 keys in../messages.js.Test plan
yarn test --testPathPattern="task-modal-v2"— 8/8 passnpx eslint --max-warnings=0 src/elements/content-sidebar/task-modal-v2/— cleannpx stylelint "src/elements/content-sidebar/task-modal-v2/**/*.scss"— cleannpx tsc --noEmit— cleanContentSidebarstory to verify the modal opens, closes via Escape and close-button, and renders the right localized title for all(taskType, editMode)permutations; the temporary wiring is NOT part of this PR)Follow-ups
UserMini | GroupMini<->UserContactType<->TaskCollabAssignee).TaskFormV2built onUserSelectorContainerand Blueprint form primitives (TextArea, Checkbox, DatePicker, Button).TaskFormV2intoTaskModalV2including edit-mode prefill and submit handling.ActivityFeedV2.tsxto renderTaskModalV2.Summary by CodeRabbit