Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

@waleedlatif1 waleedlatif1 commented Jan 28, 2026

Summary

  • Refactor search modal from custom implementation to cmdk library
  • Fix 16 SVG icons with hardcoded gradient/clipPath IDs causing rendering issues when multiple instances appear
  • Pre-compute search data on sidebar mount instead of on modal open
  • Single-pass processing of blocks/tools/triggers instead of multiple iterations
  • Remove unused search-utils.ts (241 lines)
  • Fix mcp.tsx event handler type error (pre-existing build issue)

Type of Change

  • Improvement (performance and bug fix)

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Jan 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 28, 2026 6:26pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 28, 2026

Greptile Overview

Greptile Summary

This PR refactors the search modal from a custom implementation to use the cmdk library, improving maintainability and performance.

Key Changes:

  • Replaced custom search implementation with cmdk library for better keyboard navigation and built-in filtering
  • Pre-computes search data on sidebar mount instead of on modal open, improving perceived performance
  • Fixed 16 SVG icons with hardcoded gradient/clipPath IDs by using React's useId() hook, preventing rendering conflicts when multiple instances appear
  • Removed 241 lines of custom search utilities (search-utils.ts), replaced by cmdk's built-in filtering
  • Added keyboard arrow key support for moving blocks inside containers in the workflow canvas
  • Single-pass processing of blocks/tools/triggers instead of multiple iterations

Issues Found:

  • Search data won't update when user permissions change due to early return in initializeData
  • Store missing devtools middleware per project standards

Confidence Score: 4/5

  • Safe to merge with one logic issue that should be addressed
  • The refactor is well-executed with proper SVG ID fixes and performance improvements. However, there's a logic bug where search data won't update when permissions change, and a missing devtools middleware style issue. The core functionality works but the permission change scenario needs addressing.
  • apps/sim/stores/modals/search/store.ts - needs logic fix for permission changes

Important Files Changed

Filename Overview
apps/sim/stores/modals/search/store.ts Refactored to pre-compute search data, moving data initialization from modal open to sidebar mount with filterBlocks dependency
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.tsx Major refactor from custom implementation to cmdk library, simplified event handlers, removed memo wrapper, uses portal for rendering
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx Renamed updateContainerDimensionsDuringDrag to updateContainerDimensionsDuringMove, added keyboard arrow key support for moving blocks in containers
apps/sim/components/icons.tsx Fixed 8 SVG icons with hardcoded gradient/clipPath IDs using useId() hook for unique IDs per instance

Sequence Diagram

sequenceDiagram
    participant User
    participant Sidebar
    participant SearchStore
    participant PermissionConfig
    participant SearchModal
    participant CMDK

    Note over Sidebar,SearchStore: On Sidebar Mount
    Sidebar->>PermissionConfig: usePermissionConfig()
    PermissionConfig-->>Sidebar: { filterBlocks }
    Sidebar->>SearchStore: initializeData(filterBlocks)
    SearchStore->>SearchStore: Check isInitialized flag
    alt Not Initialized
        SearchStore->>SearchStore: getAllBlocks()
        SearchStore->>SearchStore: Filter & process blocks/tools/triggers
        SearchStore->>SearchStore: Set isInitialized = true
    end

    Note over User,CMDK: User Opens Search Modal (Cmd+K)
    User->>SearchModal: Press Cmd+K
    SearchModal->>SearchStore: Read pre-computed data
    SearchStore-->>SearchModal: { blocks, tools, triggers, toolOperations, docs }
    SearchModal->>CMDK: Render Command palette with data
    CMDK-->>User: Display search interface

    Note over User,CMDK: User Searches
    User->>CMDK: Type search query
    CMDK->>CMDK: customFilter() scores items
    CMDK-->>User: Show filtered results

    Note over User,SearchModal: User Selects Item
    User->>CMDK: Select item (Enter/Click)
    alt Block/Tool/Trigger
        CMDK->>SearchModal: handleBlockSelect()
        SearchModal->>SearchModal: Dispatch 'add-block-from-toolbar' event
    else Tool Operation
        CMDK->>SearchModal: handleToolOperationSelect()
        SearchModal->>SearchModal: Dispatch with presetOperation
    else Workflow/Workspace
        CMDK->>SearchModal: handleWorkflowSelect()
        SearchModal->>SearchModal: router.push(href)
    else Doc/Page
        CMDK->>SearchModal: handleDocSelect()
        SearchModal->>SearchModal: window.open(href)
    end
    SearchModal->>SearchModal: onOpenChange(false)
    SearchModal-->>User: Close modal
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

@waleedlatif1 waleedlatif1 merged commit 304cf71 into staging Jan 28, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/cmdk branch January 28, 2026 18:38
waleedlatif1 added a commit that referenced this pull request Jan 28, 2026
…Ds (#3044)

* improvement(cmdk): refactor search modal to use cmdk + fix icon SVG IDs

* chore: remove unrelated workflow.tsx changes

* chore: remove comments

* chore: add devtools middleware to search modal store

* fix: allow search data re-initialization when permissions change

* fix: include keywords in search filter + show service name in tool operations

* fix: correct filterBlocks type signature

* fix: move generic to function parameter position

* fix(mcp): correct event handler type for onInput

* perf: always render command palette for instant opening

* fix: clear search input when modal reopens
waleedlatif1 added a commit that referenced this pull request Jan 28, 2026
* fix(workflow): update container dimensions on keyboard movement

* fix(workflow): avoid duplicate container updates during drag

Add !change.dragging check to only handle keyboard movements in
onNodesChange, since mouse drags are already handled by onNodeDrag.

* fix(workflow): persist keyboard movements to backend

Keyboard arrow key movements now call collaborativeBatchUpdatePositions
to sync position changes to the backend for persistence and real-time
collaboration.

* improvement(cmdk): refactor search modal to use cmdk + fix icon SVG IDs (#3044)

* improvement(cmdk): refactor search modal to use cmdk + fix icon SVG IDs

* chore: remove unrelated workflow.tsx changes

* chore: remove comments

* chore: add devtools middleware to search modal store

* fix: allow search data re-initialization when permissions change

* fix: include keywords in search filter + show service name in tool operations

* fix: correct filterBlocks type signature

* fix: move generic to function parameter position

* fix(mcp): correct event handler type for onInput

* perf: always render command palette for instant opening

* fix: clear search input when modal reopens

* fix(helm): move rotationPolicy under privateKey for cert-manager compatibility (#3046)

* fix(helm): move rotationPolicy under privateKey for cert-manager compatibility

* docs(helm): add reclaimPolicy Retain guidance for production database storage

* fix(helm): prevent empty branding ConfigMap creation

* fix(workflow): avoid duplicate position updates on drag end

Check isInDragOperation before persisting in onNodesChange to prevent
duplicate calls. Drag-end events have dragStartPosition still set,
while keyboard movements don't, allowing proper distinction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants