Repository navigation
Restore iOS workspace group row actions - #9182
azooz2003-bit wants to merge 31 commits into
Conversation
|
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:
📝 WalkthroughWalkthroughChangesWorkspace group icon propagation
Workspace group actions
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TabManager
participant MobileWorkspaceListObserver
participant TerminalController
participant MobileSyncWorkspaceListResponse
participant WorkspaceGroupHeaderRow
TabManager->>MobileWorkspaceListObserver: update group icon
MobileWorkspaceListObserver->>MobileWorkspaceListObserver: include iconSymbol in summaryHash
TerminalController->>TerminalController: serialize icon_symbol
TerminalController->>MobileSyncWorkspaceListResponse: provide group payload
MobileSyncWorkspaceListResponse->>WorkspaceGroupHeaderRow: decode and render iconSymbol
sequenceDiagram
participant WorkspaceListTableCoordinator
participant WorkspaceListView
participant WorkspaceGroupRenameSheet
participant ConfirmationDialog
WorkspaceListTableCoordinator->>WorkspaceListView: request group action
WorkspaceListView->>WorkspaceGroupRenameSheet: present rename UI
WorkspaceGroupRenameSheet->>WorkspaceListView: submit rename
WorkspaceListView->>WorkspaceListView: dispatch renameWorkspaceGroup
WorkspaceListView->>ConfirmationDialog: present destructive confirmation
ConfirmationDialog->>WorkspaceListView: dispatch ungroupWorkspaceGroup or deleteWorkspaceGroup
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+Actions.swift (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd Localizable.xcstrings entries for the new mobile group strings.
The group action and confirmation strings are wired through
L10n.string(key, defaultValue:), but theirmobile.workspaceGroup.*keys are not present in the mobile catalog. Add the matching entries so non-English locales can be translated instead of falling back to the English default.🤖 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 `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator`+Actions.swift at line 1, Add the missing mobile catalog entries for every `mobile.workspaceGroup.*` key used by the group action and confirmation strings through `L10n.string(key, defaultValue:)`, preserving each key’s English default value so non-English locales can translate them without fallback.Source: Path instructions
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView`+Actions.swift:
- Around line 197-215: Add focused unit tests for
confirmWorkspaceGroupDestructiveAction() and
clearWorkspaceGroupDestructiveRequest(), covering both .ungroup and .delete
actions. Assert that the matching backend closure receives the pending group ID,
the other closure is not invoked, and pending destructive state is cleared after
confirmation; also verify the explicit clear method resets both pending
properties.
---
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator`+Actions.swift:
- Line 1: Add the missing mobile catalog entries for every
`mobile.workspaceGroup.*` key used by the group action and confirmation strings
through `L10n.string(key, defaultValue:)`, preserving each key’s English default
value so non-English locales can translate them without fallback.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c416385d-1f9b-4206-ae09-e301d089ca6e
📒 Files selected for processing (20)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileStateSyncRecords.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/MobileStateSyncFrameCodingTests.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileSyncWorkspaceListResponse.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspacePreview+RemoteMapping.swiftPackages/iOS/CmuxMobileRPC/Tests/CmuxMobileRPCTests/MobileCoreRPCClientTests.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+StateSync.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupPreview.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceGroupHeaderRow.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTable.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+Actions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Actions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Table.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceNavigationRow.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollUpdateTests.swiftSources/Mobile/MobileStateSync.swiftSources/Mobile/MobileWorkspaceListObserver.swiftSources/TerminalController+MobileWorkspaceList.swiftcmuxTests/MobileWorkspaceListFidelityTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceGroupDestructiveRequestState.swift`:
- Around line 6-7: Make pending destructive requests atomic by replacing the
independently writable groupID and action properties in
WorkspaceGroupDestructiveRequestState with one private request value containing
both fields, plus enqueue, consume, and clear operations. In WorkspaceListView,
remove the writable proxies and derive reads and bindings from that canonical
request. Update the three specified WorkspaceGroupDestructiveActionTests sites
to enqueue ungroup, delete, and clear-path requests atomically; apply these
changes respectively in
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceGroupDestructiveRequestState.swift#L6-L7,
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swift#L147-L153,
and
Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceGroupDestructiveActionTests.swift#L15-L16,
`#L34-L35`, and `#L47-L48`.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift`:
- Around line 387-398: The native action reload predicate must track all
group-header action inputs, not only anchor identity and unread state. In
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift
lines 387-398, update nativeActionPayloadChanged to compare read/close
capability and callback availability; in lines 789-797, include anchor
workspace-action callback availability in the group-header payload so the
predicate executes. In
Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollUpdateTests.swift
lines 217-245, add transitions for read/close capability changes and callback
removal, asserting that native reload is requested.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0a1e64aa-fc20-4af5-a012-d4402098ba0b
📒 Files selected for processing (6)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceGroupDestructiveRequestState.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Actions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceGroupDestructiveActionTests.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollUpdateTests.swift
…oup-actions # Conflicts: # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollUpdateTests.swift`:
- Around line 354-377: Update the read and unread configurations used by the
group-header swipe test to enable supportsReadStateActions and provide the
setUnread callback (or equivalent close action) in both fixtures. Keep the
existing willBeginEditingRowAt invocation and coordinator.update flow so UIKit
exercises the reachable swipe-completion path.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 40046840-951c-492b-8076-6a677cd84692
📒 Files selected for processing (3)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollUpdateTests.swiftios/cmuxUITests/cmuxUITests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Summary
Testing
WorkspaceGroupDestructiveActionTestsandWorkspaceListScrollUpdateTests: 18 tests passed on isolated iOS 18.4 simulator3B697F98-97BF-45EB-B9B5-6F44A78DC8F1.WorkspaceListScrollUpdateTests: 16 tests passed on isolated iOS 26.5 simulator9C943972-DB3E-474F-B65D-6200F2758913.MobileStateSyncFrameCodingTests: 8 tests passed.git diff --checkpassed.Video verification
Review trigger
Checklist