Repository navigation
Add mark-read/unread + clear notifications to workspace group menu - #6535
Merged
Merged
Conversation
Workspace groups only exposed group-config actions (rename, pin, ungroup, delete) and lacked the notification actions every normal workspace row has. Since a group is a superset of its member workspaces, its header context menu now offers Mark Group as Read, Mark Group as Unread, and Clear Latest Notifications, each operating over all member workspaces (anchor included) and disabled when not applicable, reusing the same TerminalNotificationStore mutation path as the per-workspace menu. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughWorkspace group headers now support five new context menu actions for managing notification and read state: Mark Group as Read/Unread, Mark All Workspaces in Group as Read/Unread, and Clear Latest Notifications. The implementation adds state properties and action closures to ChangesWorkspace Group Header: Notifications and Drag-and-Drop
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 21 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (21 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 |
Contributor
Greptile SummaryThis PR adds notification actions (mark read/unread, clear latest, bulk mark-all) to the workspace group header context menu, and also inserts a "New Workspace in Group" shortcut. The drop-delegate logic previously co-located in
Confidence Score: 5/5Safe to merge — changes are narrowly scoped to the group header context menu, reuse existing notification store mutation paths, and carry no side effects on other surfaces. All five new menu actions use guarded per-member iteration (no unconditional mutation on already-read/unread workspaces), Bool snapshots keep the .equatable() boundary intact, closures hold stores weakly, and localization is complete across all 20 supported locales. The previously flagged issues (missing km locale, unconditional markUnread on all members) were both addressed in commit 7671684 and are reflected in this head. No files require special attention. Important Files Changed
Reviews (4): Last reviewed commit: "Split drop-delegate types out of Sidebar..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
176214
to
176576
| } | ||
| } | ||
| }, | ||
| "workspaceGroup.contextMenu.markRead": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Mark Group as Read" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "グループを既読にする" | ||
| } | ||
| }, | ||
| "ko": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "그룹을 읽음으로 표시" | ||
| } | ||
| }, | ||
| "zh-Hans": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "将分组标记为已读" | ||
| } | ||
| }, | ||
| "zh-Hant": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "將群組標為已讀" | ||
| } | ||
| }, | ||
| "de": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Gruppe als gelesen markieren" | ||
| } | ||
| }, | ||
| "es": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Marcar grupo como leído" | ||
| } | ||
| }, | ||
| "fr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Marquer le groupe comme lu" | ||
| } | ||
| }, | ||
| "it": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Segna gruppo come letto" | ||
| } | ||
| }, | ||
| "da": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Marker gruppe som læst" | ||
| } | ||
| }, | ||
| "nb": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Merk gruppe som lest" | ||
| } | ||
| }, | ||
| "pl": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Oznacz grupę jako przeczytaną" | ||
| } | ||
| }, | ||
| "ru": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Отметить группу как прочитанную" | ||
| } | ||
| }, | ||
| "th": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "ทำเครื่องหมายกลุ่มว่าอ่านแล้ว" | ||
| } | ||
| }, | ||
| "tr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Grubu Okundu Olarak İşaretle" | ||
| } | ||
| }, | ||
| "uk": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Позначити групу як прочитану" | ||
| } | ||
| }, | ||
| "ar": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "تعليم المجموعة كمقروءة" | ||
| } | ||
| }, | ||
| "bs": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Označi grupu kao pročitanu" | ||
| } | ||
| }, | ||
| "pt-BR": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Marcar Grupo como Lido" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "workspaceGroup.contextMenu.markUnread": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Mark Group as Unread" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "グループを未読にする" | ||
| } | ||
| }, | ||
| "ko": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "그룹을 읽지 않음으로 표시" | ||
| } | ||
| }, | ||
| "zh-Hans": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "将分组标记为未读" | ||
| } | ||
| }, | ||
| "zh-Hant": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "將群組標為未讀" | ||
| } | ||
| }, | ||
| "de": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Gruppe als ungelesen markieren" | ||
| } | ||
| }, | ||
| "es": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Marcar grupo como no leído" | ||
| } | ||
| }, | ||
| "fr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Marquer le groupe comme non lu" | ||
| } | ||
| }, | ||
| "it": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Segna gruppo come non letto" | ||
| } | ||
| }, | ||
| "da": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Marker gruppe som ulæst" | ||
| } | ||
| }, | ||
| "nb": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Merk gruppe som ulest" | ||
| } | ||
| }, | ||
| "pl": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Oznacz grupę jako nieprzeczytaną" | ||
| } | ||
| }, | ||
| "ru": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Отметить группу как непрочитанную" | ||
| } | ||
| }, | ||
| "th": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "ทำเครื่องหมายกลุ่มว่ายังไม่อ่าน" | ||
| } | ||
| }, | ||
| "tr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Grubu Okunmadı Olarak İşaretle" | ||
| } | ||
| }, | ||
| "uk": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Позначити групу як непрочитану" | ||
| } | ||
| }, | ||
| "ar": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "تعليم المجموعة كغير مقروءة" | ||
| } | ||
| }, | ||
| "bs": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Označi grupu kao nepročitanu" | ||
| } | ||
| }, | ||
| "pt-BR": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Marcar Grupo como Não Lido" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "workspaceGroup.contextMenu.clearLatestNotifications": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Clear Latest Notifications" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "最新の通知をクリア" | ||
| } | ||
| }, | ||
| "ko": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "최근 알림 모두 지우기" | ||
| } | ||
| }, | ||
| "zh-Hans": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "清除最新通知" | ||
| } | ||
| }, | ||
| "zh-Hant": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "清除最新通知" | ||
| } | ||
| }, | ||
| "de": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Neueste Mitteilungen löschen" | ||
| } | ||
| }, | ||
| "es": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Borrar últimas notificaciones" | ||
| } | ||
| }, | ||
| "fr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Effacer les dernières notifications" | ||
| } | ||
| }, | ||
| "it": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Cancella ultime notifiche" | ||
| } | ||
| }, | ||
| "da": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Ryd seneste notifikationer" | ||
| } | ||
| }, | ||
| "nb": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Tøm siste varsler" | ||
| } | ||
| }, | ||
| "pl": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Wyczyść najnowsze powiadomienia" | ||
| } | ||
| }, | ||
| "ru": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Очистить последние уведомления" | ||
| } | ||
| }, | ||
| "th": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "ล้างการแจ้งเตือนล่าสุด" | ||
| } | ||
| }, | ||
| "tr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Son bildirimleri temizle" | ||
| } | ||
| }, | ||
| "uk": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Очистити останні сповіщення" | ||
| } | ||
| }, | ||
| "ar": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "مسح أحدث الإشعارات" | ||
| } | ||
| }, | ||
| "bs": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Očisti najnovije obavijesti" | ||
| } | ||
| }, | ||
| "pt-BR": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Limpar Notificações Mais Recentes" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "workspaceGroup.contextMenu.ungroup": { | ||
| "extractionState": "manual", | ||
| "localizations": { |
Contributor
There was a problem hiding this comment.
km (Khmer) locale in all three new strings
Every other workspaceGroup.contextMenu.* string in the catalog (.rename, .pin, .unpin, .editConfig, .openDocs, .delete, .ungroup) includes a km translation — Khmer is present in 443 strings across the catalog. All three new keys (workspaceGroup.contextMenu.markRead, .markUnread, .clearLatestNotifications) are missing the km locale entry. Khmer-locale users will see the raw defaultValue English fallback for these menu items instead of a translated string.
Rule Used: Flag production user-facing text that is not fully... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Collaborator
Author
There was a problem hiding this comment.
Fixed in 7671684 — added km (Khmer) entries to all five new workspaceGroup.contextMenu.* strings.
— Claude Code
Comment on lines
+161
to
+171
| onMarkUnread: { [weak notificationStore, memberWorkspaceIds] in | ||
| guard let notificationStore else { return } | ||
| for id in memberWorkspaceIds { | ||
| notificationStore.markUnread(forTabId: id) | ||
| } | ||
| }, | ||
| onClearLatestNotifications: { [weak notificationStore, memberWorkspaceIds] in | ||
| guard let notificationStore else { return } | ||
| for id in memberWorkspaceIds { | ||
| notificationStore.clearLatestNotification(forTabId: id) | ||
| } |
Contributor
There was a problem hiding this comment.
onMarkUnread sets workspaceManualUnread = true on members that are already unread via notifications
canMarkWorkspaceUnread(forTabIds:) returns true when any member is not unread, but the closure then calls markUnread(forTabId:) on all members unconditionally. For a member that is already unread due to a real notification, markUnread(forTabId:) sets workspaceManualUnread = true even though the user never intended to manipulate that workspace. The per-workspace "Mark as Unread" button is disabled for already-unread workspaces precisely to avoid this — markRead(id:) (fired when the OS notification center dismisses a notification) does not clear the manual flag, so those workspaces can end up stuck unread after the notification is externally dismissed. The fix is to guard with canMarkWorkspaceUnread(forTabIds: [id]) per-member before calling markUnread.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 7671684 — mark-all now guards each member with canMarkWorkspaceUnread(forTabIds: [id]) before calling markUnread, so already-unread members (e.g. unread via a notification) are skipped and never get the manual-unread flag set.
— Claude Code
Address dogfood feedback: - Plain Mark Group as Read/Unread and Clear Latest Notifications now act on the group's own representation (the anchor workspace) only, matching a normal workspace row, instead of cascading to every member. - Add explicit Mark All Workspaces in Group as Read / as Unread items for the bulk action over all members (anchor included). - Add New Workspace in Group to the group header context menu (reuses the existing onTapPlus path and the plus-button's localized string). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mark All Workspaces in Group as Read/Unread now skips the anchor workspace, so it changes only the contained workspaces and never the group's own row read status. The anchor's status stays owned by Mark Group as Read/Unread. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@Sources/VerticalTabsSidebar`+WorkspaceGroups.swift:
- Around line 167-177: The onMarkAllRead and onMarkAllUnread closures currently
loop through memberWorkspaceIds and call notificationStore.markRead(forTabId:)
and notificationStore.markUnread(forTabId:) individually for each workspace,
causing repeated rescans of notification state. Add two new batched methods to
TerminalNotificationStore: markRead(forTabIds: [UUID]) and markUnread(forTabIds:
[UUID]) that process all workspace IDs in a single mutation pass, then replace
the for loops in both closures to call these new batched methods instead,
passing the entire memberWorkspaceIds array directly.
🪄 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
Run ID: 9d63701c-5534-415b-9ce1-b70a3427e477
📒 Files selected for processing (3)
Resources/Localizable.xcstringsSources/SidebarWorkspaceGroupHeaderView.swiftSources/VerticalTabsSidebar+WorkspaceGroups.swift
… locale Address review findings: - Codex P1: mark-all closures captured the member ID list, which is not an Equatable input on the .equatable() header, so a same-count membership swap could leave the action operating on stale IDs. Resolve members live from tabManager.tabs by groupId at action time instead of capturing the list. - Greptile P1: guard each member with canMarkWorkspaceRead/Unread before mutating, so mark-all-unread never sets the manual-unread flag on a member already unread via a notification (which a later dismissal can't clear). - Greptile: add the km (Khmer) locale entry to the five new group strings to match the rest of the workspaceGroup.contextMenu.* catalog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The added menu items plus the upstream hover-tracker rework pushed SidebarWorkspaceGroupHeaderView.swift to 560 lines, over the 500-line file-length budget (workflow-guard-tests). Move the cohesive, separable DropZone/DropAction/DropPolicy/DropDelegate types into a new SidebarWorkspaceGroupHeaderDropDelegate.swift (wired into the pbxproj and normalized), leaving the view file at 372 lines. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 21, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Workspace groups are a superset of their member workspaces, but the group header's right-click menu only offered group-config actions (rename, pin/unpin, edit config, docs, ungroup, delete). It was missing the basic notification actions that every normal workspace row has, most notably mark as unread.
This adds three items to the group context menu, between Pin/Unpin and Edit Group Config:
Each operates over all member workspaces (anchor included) and reuses the existing
TerminalNotificationStoremutation path (markRead/markUnread/clearLatestNotification) that the per-workspace menu uses, so there is one shared behavior path rather than duplicated logic. Each button is disabled when not applicable, usingcanMarkWorkspaceRead/canMarkWorkspaceUnreadand a latest-notification check, matching the per-workspace menu's disabled rules.The header view stays within the sidebar snapshot boundary: the new inputs are plain
Boolvalue snapshots plus action closures (closures hold the store weakly), added to the view'sEquatableso unread-state changes correctly invalidate the row.Localization: added
workspaceGroup.contextMenu.markRead,workspaceGroup.contextMenu.markUnread, andworkspaceGroup.contextMenu.clearLatestNotificationstoResources/Localizable.xcstringswith translations for all locales used by the analogous per-workspace strings (en, ja, ko, zh-Hans, zh-Hant, de, es, fr, it, da, nb, pl, ru, th, tr, uk, ar, bs, pt-BR).🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Add notification controls and quick-create to the workspace group header menu. Plain actions target the group’s anchor; mark-all items update member workspaces and skip the anchor.
New Features
TerminalNotificationStore; buttons disabled viacanMarkRead/canMarkUnread/hasLatestNotificationsandcanMarkAllRead/canMarkAllUnread; header stays snapshot-safe with Bool inputs and weak closures;Equatableextended; localized strings added (incl.km).Refactors
Written for commit 83b9dda. Summary will update on new commits.
Summary by CodeRabbit