[codex] simplify workspace controls#3714
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRepositions sidebar add/remove controls into the left column of the V2 Workspaces rows, updates the grid column template to enlarge the first column, and removes an unused placeholder span from the column headers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR simplifies the workspace list controls by moving the add/remove sidebar action from a hidden trailing column into the always-visible leading column, replacing the small dot indicator. The trailing Confidence Score: 5/5This PR is safe to merge — it is a clean UI refactor with no logic changes or regressions. All three files have consistent, matching changes (grid template, header placeholder, and row component). No business logic is altered; only presentation is simplified. No P0 or P1 findings were identified. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx | Sidebar dot indicator replaced with always-visible add/remove icon buttons in the leading column; trailing action div removed. Tooltip side correctly updated from left to right for the new position. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx | Removes the empty trailing <span /> grid placeholder that corresponded to the now-deleted action column. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/constants.ts | Grid template updated: leading column widened from 1.25rem to 2.5rem to fit the size-7 (1.75rem) button, and the trailing 2.5rem action column removed at all breakpoints. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[V2WorkspaceRow renders] --> B{workspace.isInSidebar?}
B -- Yes --> C[Leading column: Remove button LuMinus\nTooltip side=right\ndisabled if isCurrentRoute]
B -- No --> D[Leading column: Add button LuPlus\nTooltip side=right]
C --> E[Name / Host / Branch / Created columns]
D --> E
E --> F[Row click → navigateToV2Workspace]
Reviews (1): Last reviewed commit: "simplify workspace controls" | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx`:
- Around line 136-154: In V2WorkspaceRow replace the native disabled prop on the
Button used inside TooltipTrigger with aria-disabled={isCurrentRoute} so the
Tooltip can open, and modify the onClick to guard against action when disabled
by checking isCurrentRoute (stop propagation/return) before calling
handleRemoveFromSidebar; also apply the visual disabled styles (e.g.,
"cursor-not-allowed" and reduced opacity) conditionally on the Button className
so it looks inert while still allowing pointer events.
🪄 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
Run ID: 7924c90d-fa28-4b47-a67c-c7a810887513
📒 Files selected for processing (3)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/constants.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx:142">
P2: The `disabled` attribute suppresses pointer events, so the Radix UI Tooltip will never show on this button when `isCurrentRoute` is true — exactly the case where the "Can't remove the current workspace" message is intended to appear. Use `aria-disabled={isCurrentRoute}` instead, guard the click handler manually, and add `cursor-not-allowed opacity-50` styles when disabled (this pattern is already established in `CompareBaseBranchPicker.tsx`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
* simplify workspace controls * fix current workspace remove tooltip
upstream の 10 commits は #426 と #427 で fork 固有の差分を保ちながら個別に cherry-pick 取り込み済み。本 merge は ours strategy で **記録だけ** マージ済みに することで behind=0 を達成し、git 履歴上の追跡を正しくする。 Cherry-pick 取り込み済 (PR #426): - 5aab22a fix closed picker filters (superset-sh#3702) → cdb52f9 - 99db5be [codex] simplify workspace controls (superset-sh#3714) → f079606 - 186078a fix(chat): prevent ask_user question from shadowing sandbox access prompt (superset-sh#3662) → 09d6b57 - 47893c2 fix desktop workspace creation title clamp (superset-sh#3718) → 6a8c4ae - 09323ff Add diff pane file viewer action (superset-sh#3715) → 817ed8d - a5891c6 remove pending launch log (superset-sh#3721) → 0764d03 - c83de0c Add Tiptap table support (superset-sh#3719) → e67a885 - 486b621 [codex] Fix v2 terminal lifecycle after sleep (superset-sh#3711) → b71fbbb (+ #426 内 review fixups) Cherry-pick 取り込み済 (PR #427): - e07aef6 feat(desktop): play v2 notification hooks client-side (superset-sh#3675) → 27ac18a - eae6008 [codex] Port v2 terminal hotkeys to v1 (superset-sh#3724) → 05a77b8 (+ #427 内 Windows .ps1 v2 化) Fork 固有領域は変更ゼロで保持: 19 tRPC procedures (workspaces.githubExtended)、 AudioScheduler / Aivis TTS / notification-manager、terminal suggestion handler (新 terminalKeyboardHandler.ts に移植)、TERMINAL_OPTIONS、SUPERSET_WORKSPACE_NAME、 MainWindowEffects、INCEPTION_AUTH_PROVIDER_ID、v1MigrationState、TiptapPromptEditor、 electron-builder.ts (dmg.size="4g", fileAssociations)、Service Status Dashboard、 Linux daemon systemd、Worktree auto-sync、Windows support、DnD scratch route 他。
What changed
Why
This keeps the add/remove sidebar control in the position where the sidebar indicator previously appeared and removes the extra row-end action target.
Validation
bunx biome check --write apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/constants.ts apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsxbun run --cwd apps/desktop typecheckSummary by cubic
Replaced the sidebar dot with always-visible add/remove buttons in the first column and removed the trailing action column. Fixed the tooltip and disabled state when trying to remove the current workspace.
Written for commit 7149187. Summary will update on new commits.
Summary by CodeRabbit
Style
Bug Fixes