Repository navigation
feat:拆分wework tab - #2358
feat:拆分wework tab#2358
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR removes several demo pages and assets. It adds shared app-preference state, unified desktop titlebar behavior, reusable sidebar primitives, Tauri collapsed-sidebar controls, updated workspace layouts, and corresponding test coverage. ChangesDesktop shell and demo cleanup
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/src/features/todo/CloudTodoWorkspace.tsx (1)
1537-1545: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the project action button reachable by keyboard.
Line 1543 uses
hidden, which appliesdisplay: none. A keyboard user cannot focus the button, sofocus:flexcannot make it visible. This blocks the Rename, Archive, and Copy Project ID actions.Use opacity-based visibility, or keep an always-visible accessible action control.
Proposed fix
- className="hidden h-7 w-7 shrink-0 items-center justify-center rounded-md text-[rgb(var(--color-sidebar-text-muted))] transition hover:bg-[rgb(var(--color-sidebar-hover))] hover:text-[rgb(var(--color-sidebar-text-primary))] focus:flex group-hover:flex" + className="absolute right-0 flex h-7 w-7 shrink-0 items-center justify-center rounded-md text-[rgb(var(--color-sidebar-text-muted))] opacity-0 transition-opacity hover:bg-[rgb(var(--color-sidebar-hover))] hover:text-[rgb(var(--color-sidebar-text-primary))] focus-visible:opacity-100 group-hover:opacity-100 group-focus-within:opacity-100"🤖 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 1537 - 1545, Update the project action button in the project sidebar, identified by its data-testid and onClick handler, so it is not removed from the keyboard focus order by the hidden display utility. Replace the display-none visibility approach with an opacity/visibility pattern that preserves keyboard focus and allows the existing focus styling to reveal it, while retaining the current hover behavior and project actions.
🧹 Nitpick comments (2)
wework/src/components/layout/DesktopWorkbenchMain.tsx (1)
137-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the constant to drop the stale
MACOS_prefix.
MACOS_COLLAPSED_SIDEBAR_CONTROL_ALIGNMENT_CLASSis applied unconditionally wheneversidebarCollapsedistruein the unified Tauri header (lines 2135-2149), regardless of platform. Windows-specific paths were removed in this same change, so the name no longer describes its scope and can mislead future platform-specific edits.✏️ Proposed rename
-const MACOS_COLLAPSED_SIDEBAR_CONTROL_ALIGNMENT_CLASS = 'pl-2' +const COLLAPSED_SIDEBAR_CONTROL_ALIGNMENT_CLASS = 'pl-2'Update the one usage at line 2140 to match.
As per coding guidelines, "/*.{py,ts,tsx,js,jsx}: ... names must be clear."
🤖 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/components/layout/DesktopWorkbenchMain.tsx` at line 137, Rename MACOS_COLLAPSED_SIDEBAR_CONTROL_ALIGNMENT_CLASS to COLLAPSED_SIDEBAR_CONTROL_ALIGNMENT_CLASS and update its usage in the unified Tauri header where sidebarCollapsed is handled, preserving the existing styling behavior.Source: Coding guidelines
wework/src/components/layout/DesktopCollapsedSidebarToggle.tsx (1)
1-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct test coverage for this shared toggle.
This component now gates Tauri sidebar expansion for four pages (
AutomationsPage,CloudWorkPage,PluginsPage,SitesPage). No dedicated test file forDesktopCollapsedSidebarToggleappears in this change set. Add a focused test that covers: not rendering whencollapsedisfalse, not rendering outside Tauri runtime, and callingonToggleon click when both conditions hold.🤖 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/components/layout/DesktopCollapsedSidebarToggle.tsx` around lines 1 - 27, Add a focused test suite for DesktopCollapsedSidebarToggle covering the non-collapsed and non-Tauri cases as non-rendering, plus rendering and invoking onToggle when collapsed in Tauri runtime; mock or control isTauriRuntime and trigger the exposed toggle control via its test identifier.
🤖 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/App.tsx`:
- Around line 301-320: Remove the hardcoded titlebarOverlaysContent constant and
update the affected render branches to use 'flex flex-col' and 'flex-1'
directly. Locate the two ternaries in the App component and eliminate their
obsolete conditional paths while preserving the surrounding layout behavior.
In `@wework/src/components/layout/DesktopAppSwitcher.tsx`:
- Around line 277-313: Update the tabs-variant buttons rendered in
DesktopAppSwitcher around the option.label output to constrain and truncate long
labels, reusing the existing chrome-tab button sizing pattern with max-width,
min-width, min-w-0, and truncate classes while preserving the current active,
disabled, and layout styling.
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Line 2489: Remove the hardcoded usesOverlayTitlebar variable and all
conditional JSX branches that depend on it, including the pt-[44px] styling,
MacOSTitleBarDragRegion, and duplicate
desktop-sidebar-chrome-controls/DesktopWindowControls block. Retain the active
DesktopSidebarHeader path and remove the now-unused MacOSTitleBarDragRegion
import.
---
Outside diff comments:
In `@wework/src/features/todo/CloudTodoWorkspace.tsx`:
- Around line 1537-1545: Update the project action button in the project
sidebar, identified by its data-testid and onClick handler, so it is not removed
from the keyboard focus order by the hidden display utility. Replace the
display-none visibility approach with an opacity/visibility pattern that
preserves keyboard focus and allows the existing focus styling to reveal it,
while retaining the current hover behavior and project actions.
---
Nitpick comments:
In `@wework/src/components/layout/DesktopCollapsedSidebarToggle.tsx`:
- Around line 1-27: Add a focused test suite for DesktopCollapsedSidebarToggle
covering the non-collapsed and non-Tauri cases as non-rendering, plus rendering
and invoking onToggle when collapsed in Tauri runtime; mock or control
isTauriRuntime and trigger the exposed toggle control via its test identifier.
In `@wework/src/components/layout/DesktopWorkbenchMain.tsx`:
- Line 137: Rename MACOS_COLLAPSED_SIDEBAR_CONTROL_ALIGNMENT_CLASS to
COLLAPSED_SIDEBAR_CONTROL_ALIGNMENT_CLASS and update its usage in the unified
Tauri header where sidebarCollapsed is handled, preserving the existing styling
behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f0d49698-e98e-45dd-92a1-552e64d89f59
⛔ Files ignored due to path filters (15)
demo/kanban-redesign/preview.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/files.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/index-detail.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/index-members.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/index-new-task.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/index-start-task.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/index.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/my-work.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/new-task.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/projects-new-project.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/projects.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/verify/my-work-calendar.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/verify/my-work-group.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/verify/my-work-list.pngis excluded by!**/*.pngdemo/kanban-redesign/shots/verify/my-work-timeline.pngis excluded by!**/*.png
📒 Files selected for processing (42)
demo/cloud-space-mention/index.htmldemo/kanban-redesign/assets/app.jsdemo/kanban-redesign/assets/my-work.jsdemo/kanban-redesign/assets/style.cssdemo/kanban-redesign/files.htmldemo/kanban-redesign/index.htmldemo/kanban-redesign/my-work.htmldemo/kanban-redesign/new-task.htmldemo/kanban-redesign/projects.htmldemo/kanban-redesign/shots/verify/shot-mywork.jswework/src-tauri/tauri.conf.jsonwework/src/App.apps.test.tsxwework/src/App.plugins.test.tsxwework/src/App.tsxwework/src/components/layout/DesktopAppSwitcher.test.tsxwework/src/components/layout/DesktopAppSwitcher.tsxwework/src/components/layout/DesktopCollapsedSidebarToggle.tsxwework/src/components/layout/DesktopSidebar.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopSidebarPrimitives.tsxwework/src/components/layout/DesktopWindowsTitlebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/topnav/AppIframe.tsxwework/src/components/topnav/ChromeTitlebar.test.tsxwework/src/components/topnav/ChromeTitlebar.tsxwework/src/components/topnav/TitlebarActionsPortal.tsxwework/src/features/app-preferences/AppPreferencesProvider.test.tsxwework/src/features/app-preferences/AppPreferencesProvider.tsxwework/src/features/app-preferences/appPreferencesContext.tswework/src/features/app-preferences/useAppPreferencesState.tswework/src/features/experimental-features/useExperimentalFeaturesEnabled.tswework/src/features/todo/CloudTodoWorkspace.test.tsxwework/src/features/todo/CloudTodoWorkspace.tsxwework/src/features/todo/TodoNavigation.tsxwework/src/pages/AutomationsPage.tsxwework/src/pages/CloudWorkPage.test.tsxwework/src/pages/CloudWorkPage.tsxwework/src/pages/PluginsPage.tsxwework/src/pages/SitesPage.tsxwework/src/styles/globals.css
💤 Files with no reviewable changes (13)
- demo/kanban-redesign/shots/verify/shot-mywork.js
- demo/kanban-redesign/files.html
- demo/cloud-space-mention/index.html
- demo/kanban-redesign/assets/style.css
- wework/src/pages/CloudWorkPage.test.tsx
- demo/kanban-redesign/projects.html
- demo/kanban-redesign/new-task.html
- demo/kanban-redesign/my-work.html
- demo/kanban-redesign/index.html
- demo/kanban-redesign/assets/my-work.js
- wework/src/components/layout/DesktopWindowsTitlebar.tsx
- demo/kanban-redesign/assets/app.js
- wework/src/components/topnav/TitlebarActionsPortal.tsx
| const isPopoutWindow = isPopoutWindowRuntime() | ||
| const usesDesktopVibrancy = isTauri && !isPopoutWindow && getPlatform() === 'mac' | ||
| const titlebarOverlaysContent = false | ||
| const showChromeTitlebar = isTauri && !isPopoutWindow | ||
| const titlebarActiveKey = path === '/todo' ? 'todo' : activeAppKey | ||
| const [workbenchStartupReady, setWorkbenchStartupReady] = useState(false) | ||
| const [workbenchStartupRevealTimedOut, setWorkbenchStartupRevealTimedOut] = useState(false) | ||
| const isPopoutWindow = isPopoutWindowRuntime() | ||
| const openWeworkForAppshot = useCallback(() => { | ||
| navigateToApp('wework') | ||
| }, [navigateToApp]) | ||
|
|
||
| useEffect(() => { | ||
| if (!usesDesktopVibrancy) return undefined | ||
|
|
||
| document.documentElement.dataset.desktopVibrancy = 'true' | ||
| return () => { | ||
| delete document.documentElement.dataset.desktopVibrancy | ||
| } | ||
| }, [usesDesktopVibrancy]) | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the dead titlebarOverlaysContent branch.
titlebarOverlaysContent is now hardcoded to false and never recomputed. Lines 507 and 524 still branch on it (titlebarOverlaysContent ? 'relative' : 'flex flex-col' and titlebarOverlaysContent ? 'h-full' : 'flex-1'), but both always resolve to the second arm. Remove the constant and the ternaries, and use 'flex flex-col' and 'flex-1' directly.
🧹 Proposed cleanup
- const titlebarOverlaysContent = false
const showChromeTitlebar = isTauri && !isPopoutWindow- titlebarOverlaysContent ? 'relative' : 'flex flex-col'
+ 'flex flex-col'- titlebarOverlaysContent ? 'h-full' : 'flex-1'
+ 'flex-1'Based on learnings, this matches the repo-wide instruction to "remove obsolete paths, and fix the primary flow rather than hiding defects behind fallbacks."
🤖 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/App.tsx` around lines 301 - 320, Remove the hardcoded
titlebarOverlaysContent constant and update the affected render branches to use
'flex flex-col' and 'flex-1' directly. Locate the two ternaries in the App
component and eliminate their obsolete conditional paths while preserving the
surrounding layout behavior.
Source: Coding guidelines
| if (variant === 'tabs') { | ||
| return ( | ||
| <nav | ||
| data-testid="desktop-app-switcher" | ||
| aria-label={t('workbench.app_navigation', '应用导航')} | ||
| className={cn('flex shrink-0 items-center gap-1', className)} | ||
| > | ||
| {options.map(option => { | ||
| const active = option.key === activeApp | ||
| const optionTestId = testIds?.[option.key] ?? `chrome-tab-${option.key}` | ||
|
|
||
| return ( | ||
| <button | ||
| key={option.key} | ||
| type="button" | ||
| data-testid={optionTestId} | ||
| onClick={() => !option.disabled && onNavigate(option.key)} | ||
| disabled={option.disabled} | ||
| title={option.availabilityLabel ?? option.description} | ||
| aria-label={option.label} | ||
| aria-current={active ? 'page' : undefined} | ||
| className={cn( | ||
| 'flex h-8 items-center rounded-lg px-2 text-sm font-medium leading-none transition-colors', | ||
| active | ||
| ? 'bg-black/[0.045] text-text-primary' | ||
| : 'text-text-secondary hover:bg-black/[0.04] hover:text-text-primary', | ||
| option.disabled && 'cursor-not-allowed opacity-50 hover:bg-transparent' | ||
| )} | ||
| > | ||
| {option.label} | ||
| </button> | ||
| ) | ||
| })} | ||
| </nav> | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add truncation to the tabs-variant buttons.
The tabs-variant buttons render {option.label} with no truncate, min-w-0, or max-w class. The default labels are short Chinese strings, but a longer translation could overflow the fixed h-[38px] titlebar row. The existing chrome-tab-* buttons in ChromeTitlebar.tsx already guard against this with max-w-[220px] min-w-24 ... truncate.
🐛 Proposed fix
<button
key={option.key}
type="button"
data-testid={optionTestId}
onClick={() => !option.disabled && onNavigate(option.key)}
disabled={option.disabled}
title={option.availabilityLabel ?? option.description}
aria-label={option.label}
aria-current={active ? 'page' : undefined}
className={cn(
- 'flex h-8 items-center rounded-lg px-2 text-sm font-medium leading-none transition-colors',
+ 'flex h-8 max-w-[160px] min-w-0 items-center rounded-lg px-2 text-sm font-medium leading-none transition-colors',
active
? 'bg-black/[0.045] text-text-primary'
: 'text-text-secondary hover:bg-black/[0.04] hover:text-text-primary',
option.disabled && 'cursor-not-allowed opacity-50 hover:bg-transparent'
)}
>
- {option.label}
+ <span className="truncate">{option.label}</span>
</button>📝 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.
| if (variant === 'tabs') { | |
| return ( | |
| <nav | |
| data-testid="desktop-app-switcher" | |
| aria-label={t('workbench.app_navigation', '应用导航')} | |
| className={cn('flex shrink-0 items-center gap-1', className)} | |
| > | |
| {options.map(option => { | |
| const active = option.key === activeApp | |
| const optionTestId = testIds?.[option.key] ?? `chrome-tab-${option.key}` | |
| return ( | |
| <button | |
| key={option.key} | |
| type="button" | |
| data-testid={optionTestId} | |
| onClick={() => !option.disabled && onNavigate(option.key)} | |
| disabled={option.disabled} | |
| title={option.availabilityLabel ?? option.description} | |
| aria-label={option.label} | |
| aria-current={active ? 'page' : undefined} | |
| className={cn( | |
| 'flex h-8 items-center rounded-lg px-2 text-sm font-medium leading-none transition-colors', | |
| active | |
| ? 'bg-black/[0.045] text-text-primary' | |
| : 'text-text-secondary hover:bg-black/[0.04] hover:text-text-primary', | |
| option.disabled && 'cursor-not-allowed opacity-50 hover:bg-transparent' | |
| )} | |
| > | |
| {option.label} | |
| </button> | |
| ) | |
| })} | |
| </nav> | |
| ) | |
| } | |
| if (variant === 'tabs') { | |
| return ( | |
| <nav | |
| data-testid="desktop-app-switcher" | |
| aria-label={t('workbench.app_navigation', '应用导航')} | |
| className={cn('flex shrink-0 items-center gap-1', className)} | |
| > | |
| {options.map(option => { | |
| const active = option.key === activeApp | |
| const optionTestId = testIds?.[option.key] ?? `chrome-tab-${option.key}` | |
| return ( | |
| <button | |
| key={option.key} | |
| type="button" | |
| data-testid={optionTestId} | |
| onClick={() => !option.disabled && onNavigate(option.key)} | |
| disabled={option.disabled} | |
| title={option.availabilityLabel ?? option.description} | |
| aria-label={option.label} | |
| aria-current={active ? 'page' : undefined} | |
| className={cn( | |
| 'flex h-8 max-w-[160px] min-w-0 items-center rounded-lg px-2 text-sm font-medium leading-none transition-colors', | |
| active | |
| ? 'bg-black/[0.045] text-text-primary' | |
| : 'text-text-secondary hover:bg-black/[0.04] hover:text-text-primary', | |
| option.disabled && 'cursor-not-allowed opacity-50 hover:bg-transparent' | |
| )} | |
| > | |
| <span className="truncate">{option.label}</span> | |
| </button> | |
| ) | |
| })} | |
| </nav> | |
| ) | |
| } |
🤖 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/components/layout/DesktopAppSwitcher.tsx` around lines 277 - 313,
Update the tabs-variant buttons rendered in DesktopAppSwitcher around the
option.label output to constrain and truncate long labels, reusing the existing
chrome-tab button sizing pattern with max-width, min-width, min-w-0, and
truncate classes while preserving the current active, disabled, and layout
styling.
| const requiresCloudLogin = usesCloudAccount && !cloud.isConnected | ||
| const platform = getPlatform() | ||
| const usesOverlayTitlebar = isTauriRuntime() && platform === 'mac' | ||
| const usesOverlayTitlebar = false |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the now-unreachable overlay-titlebar branch.
usesOverlayTitlebar is hardcoded to false at line 2489, so the pt-[44px] branch (line 2943), the MacOSTitleBarDragRegion render (lines 2947-2949), and the duplicate desktop-sidebar-chrome-controls block with its own DesktopWindowControls (lines 2950-2964) can never execute. The new DesktopSidebarHeader (lines 2965-2989) already renders the equivalent toggle unconditionally when onToggleSidebar is provided, so this block is redundant as well as dead.
Remove usesOverlayTitlebar, the dead JSX branches, and the now-unused MacOSTitleBarDragRegion import/usage to keep only the active path.
As per coding guidelines, "Delete dead code and do not add compatibility shims or fallback paths without agreement; correct the primary path."
♻️ Proposed cleanup
- const usesOverlayTitlebar = false
const isWindowsTauri = isTauriRuntime() && platform === 'win'
@@
<div
className={cn(
'relative flex h-full flex-col px-1.5',
- usesOverlayTitlebar ? 'pt-[44px]' : 'pt-1.5'
+ 'pt-1.5'
)}
style={{ width: sidebarWidth }}
>
- {usesOverlayTitlebar && (
- <MacOSTitleBarDragRegion className="absolute inset-x-0 top-0 z-0 h-[38px]" />
- )}
- {usesOverlayTitlebar && onToggleSidebar && (
- <div
- data-testid="desktop-sidebar-chrome-controls"
- className={cn(
- 'absolute inset-x-0 top-0 z-chrome flex h-[38px] items-center justify-between pr-2',
- 'pl-[92px]'
- )}
- >
- <DesktopWindowControls
- sidebarCollapsed={false}
- onToggleSidebar={onToggleSidebar}
- className="gap-1"
- />
- </div>
- )}
<DesktopSidebarHeaderAlso applies to: 2941-2964
🤖 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/components/layout/DesktopSidebar.tsx` at line 2489, Remove the
hardcoded usesOverlayTitlebar variable and all conditional JSX branches that
depend on it, including the pt-[44px] styling, MacOSTitleBarDragRegion, and
duplicate desktop-sidebar-chrome-controls/DesktopWindowControls block. Retain
the active DesktopSidebarHeader path and remove the now-unused
MacOSTitleBarDragRegion import.
Source: Coding guidelines
Summary by CodeRabbit
New Features
Style
Chores