-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(shortcut): fixed global keyboard commands provider to follow latest ref pattern
#2569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
latest ref pattern
Greptile SummaryFixed keyboard shortcut bug by implementing the "latest ref pattern" in Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Panel as Panel Component
participant Hook as useRegisterGlobalCommands
participant Ref as commandsRef
participant Provider as GlobalCommandsProvider
participant Browser as Browser KeyDown
Panel->>Hook: useRegisterGlobalCommands(() => commands)
Hook->>Ref: Create commandsRef
Note over Hook: On mount (useEffect)
Hook->>Hook: Evaluate commands()
Hook->>Ref: Store commands in commandsRef.current
Hook->>Hook: Wrap each handler
Note over Hook: Wrapper captures cmd.id<br/>but calls commandsRef.current handler
Hook->>Provider: ctx.register(wrappedCommands)
Provider->>Provider: Store in registryRef
Note over Panel,Browser: Later: User presses Mod+Enter
Browser->>Provider: keydown event
Provider->>Provider: Match shortcut in registryRef
Provider->>Hook: Call wrapped handler
Hook->>Ref: Look up fresh command by id
Ref->>Panel: Execute latest handler (runWorkflow/cancelWorkflow)
Note over Hook,Ref: Latest Ref Pattern ensures<br/>fresh closures are called
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files reviewed, 1 comment
apps/sim/app/workspace/[workspaceId]/providers/global-commands-provider.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files reviewed, 1 comment
Summary
https://epicreact.dev/the-latest-ref-pattern-in-react/
Type of Change
Testing
Tested manually
Checklist