fix(wework): support project task round-trip navigation - #2415
Conversation
📝 WalkthroughWalkthroughThis change adds bidirectional navigation between project-space board tasks and runtime tasks. ChangesProject-space and runtime task navigation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TodoEditor
participant CloudTodoWorkspace
participant DesktopWorkbenchLayout
participant WorkspaceTabsContext
User->>TodoEditor: click local execution entry
TodoEditor->>CloudTodoWorkspace: onOpenRuntimeTask(deviceId, taskId)
CloudTodoWorkspace->>DesktopWorkbenchLayout: openProjectSpaceRuntimeTask(address)
DesktopWorkbenchLayout->>WorkspaceTabsContext: selectTab(tabId, updates)
WorkspaceTabsContext-->>DesktopWorkbenchLayout: tab activated with runtime task route
DesktopWorkbenchLayout-->>User: runtime task tab opened
User->>DesktopWorkbenchLayout: click linked project-space task
DesktopWorkbenchLayout->>WorkspaceTabsContext: selectTab(boardTabId, {contentRoute})
WorkspaceTabsContext-->>CloudTodoWorkspace: focusedItemId set via route
CloudTodoWorkspace->>CloudTodoWorkspace: wait for items, select item
CloudTodoWorkspace-->>DesktopWorkbenchLayout: onFocusedItemHandled()
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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.
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 `@wework/src/features/todo/CloudTodoWorkspace.tsx`:
- Around line 1326-1338: Move the focusedItemRequestRef.current assignment in
the focused-item effect so it occurs inside the queued microtask, immediately
after the active guard. Preserve the existing requestKey deduplication and state
updates, ensuring Strict Mode’s second setup can process the request and invoke
onFocusedItemHandled.
In `@wework/src/features/todo/TodoEditor.tsx`:
- Around line 1029-1041: Update the button rendered in the cloud todo execution
list, identified by the `data-testid` value `cloud-todo-execution-${task.id}`,
to use a minimum 44px height on screens at or below 767px. Keep its current
compact sizing for desktop through an appropriate responsive class override,
without changing its click behavior or other styling.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ea44877-e120-4819-b7b1-56db5114a1c8
📒 Files selected for processing (10)
docs/en/wework/workbench.mddocs/zh/wework/workbench.mdwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/features/todo/CloudTodoWorkspace.test.tsxwework/src/features/todo/CloudTodoWorkspace.tsxwework/src/features/todo/TodoEditor.tsxwework/src/features/workspace-tabs/WorkspaceTabsContext.test.tsxwework/src/features/workspace-tabs/WorkspaceTabsContext.tsxwework/src/features/workspace-tabs/workspaceTabsContextValue.ts
| focusedItemRequestRef.current = requestKey | ||
| let active = true | ||
| queueMicrotask(() => { | ||
| if (!active) return | ||
| setRootView('projects') | ||
| setProjectView('board') | ||
| setBoardParentId(focusedItem.parent_id) | ||
| setSelectedItem(focusedItem) | ||
| onFocusedItemHandled?.() | ||
| }) | ||
| return () => { | ||
| active = false | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Locate file"
fd -a 'CloudTodoWorkspace\.tsx$' . || true
echo "## File excerpt around referenced lines"
file="$(fd 'CloudTodoWorkspace\.tsx$' . | head -n 1)"
if [ -n "${file:-}" ]; then
sed -n '1290,1365p' "$file" | cat -n
echo "## Search focusedItemRequestRef usages"
rg -n "focusedItemRequestRef|onFocusedItemHandled|queueMicrotask" "$file" || true
fi
echo "## Behavioral model for StrictMode-like setup-cleanup setup"
python3 - <<'PY'
# Standalone model of the reported Race in React Strict Mode:
# effect mounts => sets ref and active => cleanup runs => second mount runs.
request_key = "k"
focused_item_request_ref_current = None
active = None
opened = False
called_handler = False
def old_effect():
global focused_item_request_ref_current, active, opened, called_handler
focused_item_request_ref_current = request_key
active = True
def cleanup():
global active
active = False
return cleanup
def strict_setup_cleanup_setup():
cleanup1 = old_effect()
# cleanup from first effect before second effect
cleanup1()
cleanup2 = old_effect()
cleanup2()
return focused_item_request_ref_current, active
ref_after, active_after = strict_setup_cleanup_setup()
print("old_effect_ref_after=", ref_after)
print("old_effect_active_after=", active_after)
print("old_effect_opens_task_after_2nd_cleanup=", not active_after)
print("old_effect_skips_onFocusedItemHandled_after_2nd_cleanup=", not active_after)
# Proposed model: ref set inside queued microtask only, after active test.
ref_after2 = None
active2 = True # second effect's active local after cleanup of first effect?
def proposed_effect():
global ref_after2, active2
if not active2:
return
ref_after2 = request_key
# microtask would run while active2 is true
# then active2 would become false during cleanup before second effect, but ref isn't set
def proposed_cleanup():
global active2
active2 = False
cleanup_prop = proposed_effect()
proposed_cleanup()
proposed_effect()
print("proposed_effect_ref_after=", ref_after2)
PYRepository: wecode-ai/Wegent
Length of output: 4401
🌐 Web query:
React Strict Mode double invokes effects setup cleanup before second setup documentation
💡 Result:
In React Strict Mode (development only), React intentionally performs an extra mount-unmount-remount cycle to verify that your components and effects are resilient and properly handle setup and cleanup [1][2][3]. The specific sequence of execution in Strict Mode for a component mount is: 1. Setup (First mount) [3] 2. Cleanup (First unmount) [3] 3. Setup (Second mount) [3] This behavior was introduced in React 18 to help developers identify bugs caused by missing cleanup functions or impure logic [2][4]. Because Effects are expected to be synchronized with the component's lifecycle, they should always have a cleanup function if they perform side effects (like subscriptions, network requests, or timers) [1][2]. When your component mounts in development, this sequence runs: - React calls your Effect's setup code [1]. - React immediately runs your Effect's cleanup function [1][3]. - React calls your Effect's setup code again [1]. This cycle ensures that if your code has a bug—such as a missing cleanup function that leaves an event listener or a WebSocket connection open—you will notice the issue immediately in development [1][2]. In production builds, Strict Mode checks are stripped out, and the component mounts exactly once [2][3]. If you are concerned about side effects occurring twice (e.g., duplicate API requests), the recommended approach is to implement proper cleanup (such as using an AbortController to cancel fetches or a boolean flag to ignore results) rather than disabling Strict Mode [4][5][3]. Top results: [1][2][3]
Citations:
- 1: https://react.dev/reference/react/StrictMode
- 2: https://legacy.reactjs.org/docs/strict-mode.html
- 3: https://bitsfolio.com/react-useeffect-firing-twice-dev-once-production/
- 4: Bug: useEffect runs twice on component mount (StrictMode, NODE_ENV=development) react/react#24502
- 5: Bug: v18 - How to deal with useEffect being called twice in Strict Mode? react/react#24455
Set focusedItemRequestRef after the active microtask guard.
React Strict Mode can run setup, cleanup, then setup again before the queued microtask. The first cleanup sets active to false, and the second setup exits at focusedItemRequestRef.current === requestKey, so the focused task does not open and onFocusedItemHandled is not called. Move the ref update into the microtask after the !active return.
Proposed fix
- focusedItemRequestRef.current = requestKey
let active = true
queueMicrotask(() => {
if (!active) return
+ focusedItemRequestRef.current = requestKey
setRootView('projects')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| focusedItemRequestRef.current = requestKey | |
| let active = true | |
| queueMicrotask(() => { | |
| if (!active) return | |
| setRootView('projects') | |
| setProjectView('board') | |
| setBoardParentId(focusedItem.parent_id) | |
| setSelectedItem(focusedItem) | |
| onFocusedItemHandled?.() | |
| }) | |
| return () => { | |
| active = false | |
| } | |
| let active = true | |
| queueMicrotask(() => { | |
| if (!active) return | |
| focusedItemRequestRef.current = requestKey | |
| setRootView('projects') | |
| setProjectView('board') | |
| setBoardParentId(focusedItem.parent_id) | |
| setSelectedItem(focusedItem) | |
| onFocusedItemHandled?.() | |
| }) | |
| return () => { | |
| active = false | |
| } |
🤖 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 `@wework/src/features/todo/CloudTodoWorkspace.tsx` around lines 1326 - 1338,
Move the focusedItemRequestRef.current assignment in the focused-item effect so
it occurs inside the queued microtask, immediately after the active guard.
Preserve the existing requestKey deduplication and state updates, ensuring
Strict Mode’s second setup can process the request and invoke
onFocusedItemHandled.
| <button | ||
| key={task.id} | ||
| className="flex items-center gap-2.5 rounded-lg px-2.5 py-2 text-xs transition-colors hover:bg-muted/60" | ||
| type="button" | ||
| data-testid={`cloud-todo-execution-${task.id}`} | ||
| onClick={() => | ||
| void onOpenRuntimeTask?.({ | ||
| deviceId: task.device_id, | ||
| taskId: task.task_id, | ||
| }) | ||
| } | ||
| disabled={!onOpenRuntimeTask} | ||
| className="flex w-full items-center gap-2.5 rounded-lg px-2.5 py-2 text-left text-xs transition-colors hover:bg-muted/60 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-focus disabled:cursor-default" | ||
| aria-label={`打开本地执行 ${task.task_title || task.task_id}`} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Provide a 44px mobile touch target.
This new local-execution button is about 32px high from its text and py-2 padding. It has no mobile override. On screens at or below 767px, it does not meet the required 44px × 44px minimum control size.
Add a 44px minimum height for mobile. Preserve the compact height at desktop only if needed.
🤖 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 `@wework/src/features/todo/TodoEditor.tsx` around lines 1029 - 1041, Update the
button rendered in the cloud todo execution list, identified by the
`data-testid` value `cloud-todo-execution-${task.id}`, to use a minimum 44px
height on screens at or below 767px. Keep its current compact sizing for desktop
through an appropriate responsive class override, without changing its click
behavior or other styling.
Source: Coding guidelines
What changed
Why
项目空间任务与本地运行任务之间原先只有单向跳转,用户在项目空间任务详情中点击“本地执行”后无法回到对应的 Wework 任务。此次补齐双向导航,并保留项目、任务和标签页路由状态,避免重复打开标签页。
Validation
pnpm --filter wework test— 268 个测试文件、2664 个测试通过pnpm --filter wework exec tsc --noEmitgit diff --checkWegentBug修复的任务WEGENTBUB4E7A6-2打开“本地执行”,成功回到 runtime taskruntime-969548342Summary by CodeRabbit
New Features
Documentation