fix(knowledge): prevent navigation on context menu actions and widen tags modal#4015
fix(knowledge): prevent navigation on context menu actions and widen tags modal#4015waleedlatif1 merged 4 commits intostagingfrom
Conversation
PR SummaryLow Risk Overview Widens both the base tags modal and document tags modal by changing Reviewed by Cursor Bugbot for commit 2a82159. Configure here. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
apps/sim/app/workspace/[workspaceId]/knowledge/components/base-card/base-card.tsx
Outdated
Show resolved
Hide resolved
Greptile SummaryFixes KB card context menu actions (view tags, edit, delete, copy ID) incorrectly triggering card navigation, and widens tag modals from
Confidence Score: 5/5Safe to merge — targeted bug fix with a clean ref-guard pattern and no risk of regressions All three changed files have narrow, well-understood diffs. The guard pattern is sound: the ref is set synchronously before the action runs and reset only after the current macro-task queue drains via setTimeout(0), so handleClick reliably sees the flag during event bubbling. The prior reviewer concern about requestAnimationFrame timing has been addressed. Modal size changes are trivial UX improvements. No new P0 or P1 issues found. No files require special attention Important Files Changed
Sequence DiagramsequenceDiagram
actor User
participant Card as KB Card (onClick: handleClick)
participant CtxMenu as Context Menu Item
participant Guard as withActionGuard
participant Ref as actionTakenRef
participant Timer as Macro-task Queue
User->>Card: right-click
Card-->>CtxMenu: context menu opens
User->>CtxMenu: click item (Edit / Delete / View Tags / Copy ID)
CtxMenu->>Guard: withActionGuard(fn)
Guard->>Ref: actionTakenRef.current = true
Guard->>Guard: fn()
Guard->>Timer: setTimeout(() => actionTakenRef.current = false, 0)
Note over Card: click event bubbles up synchronously
CtxMenu-->>Card: click bubbles → handleClick(e)
Card->>Ref: read actionTakenRef.current
Ref-->>Card: true
Card->>Card: e.preventDefault() — no navigation
Card-->>User: action modal opens instead
Note over Timer: macro-task executes later
Timer->>Ref: actionTakenRef.current = false
Reviews (3): Last reviewed commit: "fix(knowledge): wrap withActionGuard cal..." | Re-trigger Greptile |
apps/sim/app/workspace/[workspaceId]/knowledge/components/base-card/base-card.tsx
Outdated
Show resolved
Hide resolved
|
@greptile |
|
@cursor review |
apps/sim/app/workspace/[workspaceId]/knowledge/components/base-card/base-card.tsx
Show resolved
Hide resolved
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2a82159. Configure here.
Summary
sm(400px) tomd(500px) for better UX with tag listsType of Change
Testing
Tested manually
Checklist