Skip to content

Add entity pinning feature with sidebar widget#3858

Open
jbecke wants to merge 1 commit into
mainfrom
claude/laughing-fermi-xkjyt9
Open

Add entity pinning feature with sidebar widget#3858
jbecke wants to merge 1 commit into
mainfrom
claude/laughing-fermi-xkjyt9

Conversation

@jbecke

@jbecke jbecke commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements a client-side pinning system for entities with persistent storage and UI integration. Users can now pin documents, chats, projects, emails, calls, and channels for quick access from the sidebar.

Key Changes

  • New pinning signal store (pins.ts): Manages pinned entities with local persistence via localStorage while syncing with the backend. Supports pinning any entity type that has backend pin support.
  • Pinned widget component (pinned-widget.tsx): Displays pinned entities in the sidebar with two layouts:
    • Expanded: Full list with entity icons and titles
    • Slim: Compact grid showing up to 4 items with overflow indicator
  • Pin action (make-pin-action.ts): Implements toggle pin/unpin functionality for entity selections with toast notifications
  • Sidebar integration: Added PinnedWidget to the main sidebar with scrollable container (10-20rem height)
  • Context menu support: Right-click options to open pinned items in current or new split, or unpin them
  • Action menu integration: Pin/unpin action added to entity action menus in the soup view

Implementation Details

  • The system maintains a local snapshot of pinned entities for rendering while the backend remains the source of truth
  • Pin operations are write-through: local state updates immediately for responsive UI, then syncs to backend (best effort)
  • Supports mapping frontend entity types to backend pin types (e.g., emailemail_thread)
  • Gracefully handles localStorage unavailability—in-memory signal continues to work
  • Pinned items can be opened in current split (click) or new split (shift+click)

https://claude.ai/code/session_01SgxdRrh8YABj1ANhdDNJUe

Adds a "Pinned" section to the app sidebar below the Unread channels
section, and a right-click "Pin"/"Unpin" action in the soup entity
context menu for all access-backed entity types (documents/files,
emails, channels, chats, projects, calls).

The macro backend already persists pins via the /pins endpoints, but its
hydrating GET /pins query only returns documents, chats and projects. To
support pinning any entity type for display, a small client-side store
keeps a persisted snapshot of pinned entities and writes the pin set
through to the backend.

https://claude.ai/code/session_01SgxdRrh8YABj1ANhdDNJUe
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added a "Pinned" sidebar widget displaying pinned entities with full or compact display modes
    • Introduced pin/unpin functionality for entities with persistent storage
    • Added pin/unpin actions to entity context menus
    • Pinned items can be opened in the current split or a new split

Walkthrough

This PR introduces an entity pinning feature across three surfaces. The core (pins.ts) defines a SolidJS reactive store that manages pinned entities with localStorage persistence and backend synchronization via storageServiceClient. A pin/unpin action factory (make-pin-action.ts) exposes canExecute, allPinned, and execute methods that toggle pin state and emit toast notifications. The action integrates into the entity context menu (create-soup-entity-actions.ts), adding a conditional "Pin/Unpin" item. Finally, a new sidebar widget (pinned-widget.tsx) displays pinned entities in full or slim layout (with overflow indicator at 4 items), each supporting mouse-down to open and a context menu for split management and unpinning.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title does not follow the required conventional commits format (missing prefix like feat:, fix:, chore:, etc.). Rewrite the title to follow conventional commits format, e.g., 'feat: Add entity pinning feature with sidebar widget' (ensure it remains under 72 characters).
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description is well-detailed and directly related to the changeset, covering the pinning system implementation, UI components, and integration points.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

@jbecke jbecke marked this pull request as ready for review June 9, 2026 01:05

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@js/app/packages/app/signal/pins.ts`:
- Around line 31-49: Replace the native switch in toPinType with ts-pattern's
match: import { match } from 'ts-pattern' and use
match(entity.type).with('document', () => 'document').with('chat', () =>
'chat').with('project', () => 'project').with('email', () =>
'email_thread').with('call', () => 'call').with('channel', () =>
'channel').otherwise(() => null) so the mapping stays identical but becomes
exhaustive and type-safe; update the toPinType function to use this match
expression and remove the switch.
🪄 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: CHILL

Plan: Pro

Run ID: 3ff3b174-a5e0-4c14-98b5-ff0f5e0fbf3a

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd4c94 and 595f3d4.

📒 Files selected for processing (6)
  • js/app/packages/app/component/app-sidebar/pinned-widget.tsx
  • js/app/packages/app/component/app-sidebar/sidebar.tsx
  • js/app/packages/app/component/next-soup/actions/index.ts
  • js/app/packages/app/component/next-soup/actions/make-pin-action.ts
  • js/app/packages/app/component/next-soup/soup-view/create-soup-entity-actions.ts
  • js/app/packages/app/signal/pins.ts

Comment on lines +31 to +49
function toPinType(entity: EntityData): string | null {
switch (entity.type) {
case 'document':
return 'document';
case 'chat':
return 'chat';
case 'project':
return 'project';
case 'email':
return 'email_thread';
case 'call':
return 'call';
case 'channel':
return 'channel';
// channel_message, automation and foreign entities are not pinnable.
default:
return null;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use match from ts-pattern for exhaustive entity type switching.

Per coding guidelines, exhaustive switch statements should use match from ts-pattern instead of native switch for better type safety and explicit case handling.

♻️ Refactor to use ts-pattern
+import { match } from 'ts-pattern';
+
 /**
  * Maps a frontend entity to the backend pin type (`EntityType`, snake_case).
  * Returns `null` for entities that cannot be pinned (no entity_access backing).
  */
 function toPinType(entity: EntityData): string | null {
-  switch (entity.type) {
-    case 'document':
-      return 'document';
-    case 'chat':
-      return 'chat';
-    case 'project':
-      return 'project';
-    case 'email':
-      return 'email_thread';
-    case 'call':
-      return 'call';
-    case 'channel':
-      return 'channel';
-    // channel_message, automation and foreign entities are not pinnable.
-    default:
-      return null;
-  }
+  return match(entity.type)
+    .with('document', () => 'document' as const)
+    .with('chat', () => 'chat' as const)
+    .with('project', () => 'project' as const)
+    .with('email', () => 'email_thread' as const)
+    .with('call', () => 'call' as const)
+    .with('channel', () => 'channel' as const)
+    .otherwise(() => null);
 }

As per coding guidelines: "For exhaustive switch statements in TypeScript, use match from ts-pattern instead of native switch" and "Use match from ts-pattern for exhaustive switch/case logic."

🤖 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 `@js/app/packages/app/signal/pins.ts` around lines 31 - 49, Replace the native
switch in toPinType with ts-pattern's match: import { match } from 'ts-pattern'
and use match(entity.type).with('document', () => 'document').with('chat', () =>
'chat').with('project', () => 'project').with('email', () =>
'email_thread').with('call', () => 'call').with('channel', () =>
'channel').otherwise(() => null) so the mapping stays identical but becomes
exhaustive and type-safe; update the toPinType function to use this match
expression and remove the switch.

Source: Coding guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants