Add v2 mark workspace as unread#3773
Conversation
Adds a "Mark as Unread"/"Mark as Read" toggle to the v2 sidebar
workspace context menu. Marking unread inserts a manual notification
source ({type:"manual",id:workspaceId}) with status "review", so the
workspace shows the same green dot as a real ready-for-review
notification and is auto-cleared by the existing
clearWorkspaceAttention call when the workspace is clicked.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a manual "unread" notification source to the v2 notification store and wires a toggle action through workspace sidebar hook and context menu, allowing users to mark workspace items as read/unread via the sidebar context menu. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Sidebar ContextMenu
participant Hook as Workspace Item Hook
participant Store as V2 Notification Store
User->>UI: Open context menu & select toggle
UI->>Hook: call onToggleUnread()
Hook->>Store: setManualUnread(workspaceId) OR clear attention
Store->>Store: update sources (manual type, status "review")
Store-->>Hook: updated isUnread selector
Hook-->>UI: updated isUnread prop
UI-->>User: update menu/indicator state
rect rgba(200,230,255,0.5)
note right of Hook: Hook coordinates UI <> Store
end
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 adds a Mark as Unread / Mark as Read toggle to the v2 sidebar workspace right-click context menu. It introduces a new Confidence Score: 5/5Safe to merge — the change is well-scoped, reuses existing store infrastructure correctly, and has no logic bugs. All findings are P2 or lower. The manual unread state is in-memory only (ephemeral across sessions), which appears intentional. The No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/stores/v2-notifications/store.ts | Adds manual source type, setManualUnread/clearManualUnread actions, and selectV2WorkspaceIsManuallyUnread selector; logic is correct and consistent with existing patterns |
| apps/desktop/src/renderer/stores/v2-notifications/index.ts | Re-exports new public symbols (getV2ManualNotificationSource, selectV2WorkspaceIsManuallyUnread, useV2WorkspaceIsManuallyUnread); no issues |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts | Wires isManuallyUnread, setManualUnread, clearManualUnread from the store into handleToggleUnread; straightforward and correct |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsx | Adds isManuallyUnread prop and toggleable menu item with LuEye/LuEyeOff icons; UI logic is correct |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx | Propagates isManuallyUnread and onToggleUnread to both collapsed and expanded DashboardSidebarWorkspaceContextMenu instances; no issues |
Sequence Diagram
sequenceDiagram
participant User
participant ContextMenu as DashboardSidebarWorkspaceContextMenu
participant Hook as useDashboardSidebarWorkspaceItemActions
participant Store as V2NotificationStore
User->>ContextMenu: Right-click workspace
ContextMenu-->>User: Show "Mark as Unread" (LuEyeOff) or "Mark as Read" (LuEye)
User->>ContextMenu: Click "Mark as Unread"
ContextMenu->>Hook: onToggleUnread()
Hook->>Store: setManualUnread(workspaceId)
Store->>Store: setSourceStatus({type:"manual", id:workspaceId}, workspaceId, "review")
Store-->>ContextMenu: isManuallyUnread = true → green dot shown
User->>ContextMenu: Right-click → Click "Mark as Read"
ContextMenu->>Hook: onToggleUnread()
Hook->>Store: clearManualUnread(workspaceId)
Store->>Store: clearSourceStatus({type:"manual", id:workspaceId})
Store-->>ContextMenu: isManuallyUnread = false → dot removed
User->>Hook: Click workspace (navigate)
Hook->>Store: clearWorkspaceAttention(workspaceId)
Store->>Store: Remove all "review" sources for workspaceId (incl. manual)
Store-->>ContextMenu: isManuallyUnread = false → dot removed
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/v2-notifications/store.ts
Line: 318-319
Comment:
**Redundant `workspaceId` guard in selector**
The check `?.workspaceId === workspaceId` is always true when the entry exists, because the source key is `manual:${workspaceId}` and `setManualUnread` always writes the same `workspaceId` into the entry. A simpler expression would be `!!state.sources[sourceKey]`, which makes the intent (entry presence) clearer.
```suggestion
state.sources[sourceKey] != null;
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "add v2 mark workspace as unread" | Re-trigger Greptile
| return (state: V2NotificationState) => | ||
| state.sources[sourceKey]?.workspaceId === workspaceId; |
There was a problem hiding this comment.
Redundant
workspaceId guard in selector
The check ?.workspaceId === workspaceId is always true when the entry exists, because the source key is manual:${workspaceId} and setManualUnread always writes the same workspaceId into the entry. A simpler expression would be !!state.sources[sourceKey], which makes the intent (entry presence) clearer.
| return (state: V2NotificationState) => | |
| state.sources[sourceKey]?.workspaceId === workspaceId; | |
| state.sources[sourceKey] != null; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/v2-notifications/store.ts
Line: 318-319
Comment:
**Redundant `workspaceId` guard in selector**
The check `?.workspaceId === workspaceId` is always true when the entry exists, because the source key is `manual:${workspaceId}` and `setManualUnread` always writes the same `workspaceId` into the entry. A simpler expression would be `!!state.sources[sourceKey]`, which makes the intent (entry presence) clearer.
```suggestion
state.sources[sourceKey] != null;
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/stores/v2-notifications/store.ts (1)
314-324: Optional: redundant workspaceId equality check.Since
sourceKeyis derived fromworkspaceId(manual:${workspaceId}), any entry stored under that key is guaranteed to have a matchingworkspaceId. The?.workspaceId === workspaceIdcheck is equivalent to a presence check.♻️ Optional simplification
export function selectV2WorkspaceIsManuallyUnread(workspaceId: string) { const sourceKey = getV2NotificationSourceKey( getV2ManualNotificationSource(workspaceId), ); - return (state: V2NotificationState) => - state.sources[sourceKey]?.workspaceId === workspaceId; + return (state: V2NotificationState) => sourceKey in state.sources; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/v2-notifications/store.ts` around lines 314 - 324, The selectV2WorkspaceIsManuallyUnread function does an unnecessary equality check because sourceKey is derived from workspaceId; replace the current condition state.sources[sourceKey]?.workspaceId === workspaceId with a simple presence check like Boolean(state.sources[sourceKey]) (or !!state.sources[sourceKey]) to return whether the manual source exists; update selectV2WorkspaceIsManuallyUnread (and confirm useV2WorkspaceIsManuallyUnread remains unchanged) and ensure references to getV2NotificationSourceKey and getV2ManualNotificationSource are used to compute sourceKey as before.
🤖 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/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts`:
- Around line 137-143: The current handleToggleUnread in
useDashboardSidebarWorkspaceItemActions toggles manual unread even when the
workspace isActive, causing a green dot to appear on the active workspace;
change handleToggleUnread to no-op when isActive is true (i.e., if (isActive)
return) so marking unread does nothing for the currently-viewed workspace,
otherwise keep the existing logic calling clearManualUnread(workspaceId) or
setManualUnread(workspaceId); update any related tests or UX notes to reflect
this behavior and ensure references: handleToggleUnread, isActive,
isManuallyUnread, clearManualUnread, setManualUnread, and
clearWorkspaceAttention are considered.
---
Nitpick comments:
In `@apps/desktop/src/renderer/stores/v2-notifications/store.ts`:
- Around line 314-324: The selectV2WorkspaceIsManuallyUnread function does an
unnecessary equality check because sourceKey is derived from workspaceId;
replace the current condition state.sources[sourceKey]?.workspaceId ===
workspaceId with a simple presence check like Boolean(state.sources[sourceKey])
(or !!state.sources[sourceKey]) to return whether the manual source exists;
update selectV2WorkspaceIsManuallyUnread (and confirm
useV2WorkspaceIsManuallyUnread remains unchanged) and ensure references to
getV2NotificationSourceKey and getV2ManualNotificationSource are used to compute
sourceKey as before.
🪄 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: 2432262a-dfdc-4b85-b92a-1b51832d0712
📒 Files selected for processing (5)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.tsapps/desktop/src/renderer/stores/v2-notifications/index.tsapps/desktop/src/renderer/stores/v2-notifications/store.ts
| const handleToggleUnread = () => { | ||
| if (isManuallyUnread) { | ||
| clearManualUnread(workspaceId); | ||
| } else { | ||
| setManualUnread(workspaceId); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Minor UX note: marking unread while the workspace is active.
If the user invokes "Mark as Unread" while currently viewing the workspace, the green dot will appear on the active workspace and will only auto-clear once the user navigates away and then re-enters via a sidebar click (the only path that calls clearWorkspaceAttention). Switching tabs/panes within the active workspace won't clear it. Probably fine — and matches the PR description's auto-clear-on-visit model — but worth confirming this is the intended behavior, or consider a no-op when isActive is true.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts`
around lines 137 - 143, The current handleToggleUnread in
useDashboardSidebarWorkspaceItemActions toggles manual unread even when the
workspace isActive, causing a green dot to appear on the active workspace;
change handleToggleUnread to no-op when isActive is true (i.e., if (isActive)
return) so marking unread does nothing for the currently-viewed workspace,
otherwise keep the existing logic calling clearManualUnread(workspaceId) or
setManualUnread(workspaceId); update any related tests or UX notes to reflect
this behavior and ensure references: handleToggleUnread, isActive,
isManuallyUnread, clearManualUnread, setManualUnread, and
clearWorkspaceAttention are considered.
Toggle now reflects any review-status entry (manual or real ready-for-review notification) and clears them all via clearWorkspaceAttention. Drops the now-unused clearManualUnread action.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
LuEye/LuEyeOff).{ type: "manual", id: workspaceId }source with status"review"shows the same green dot as a real ready-for-review notification.clearWorkspaceAttention(workspaceId), so manual unread auto-clears on visit — no extra wiring needed.Test plan
Summary by cubic
Add a Mark as Unread/Read toggle to the v2 sidebar workspace context menu to quickly flag workspaces with the same green dot. “Mark as Unread” creates a
{ type: "manual", id: workspaceId }review entry; “Mark as Read” now clears any review state (manual or real) viaclearWorkspaceAttention, and dots auto‑clear on open.Written for commit 281acab. Summary will update on new commits.
Summary by CodeRabbit