-
Notifications
You must be signed in to change notification settings - Fork 680
[Proposal] trigger client side panel event from python #6304
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { default as usePanelClientEvent } from "./usePanelClientEvent"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import { usePanelId } from "@fiftyone/spaces"; | ||
|
|
||
| const events: PanelsEvents = {}; | ||
|
|
||
| export default function usePanelEvents(panelId?: string) { | ||
| const id = usePanelId(); | ||
| const computedPanelId = panelId ?? id; | ||
|
|
||
| function register( | ||
| event: string, | ||
| callback: PanelEventHandler, | ||
| panelId?: string | ||
| ) { | ||
| const registerPanelId = panelId ?? computedPanelId; | ||
| assertPanelId(registerPanelId); | ||
| if (!events[registerPanelId]) { | ||
| events[registerPanelId] = {}; | ||
| } | ||
| events[registerPanelId][event] = callback; | ||
| } | ||
|
|
||
| function trigger(event: string, params: unknown, panelId?: string) { | ||
| const triggerPanelId = panelId ?? computedPanelId; | ||
| assertPanelId(triggerPanelId); | ||
| const callback = events[triggerPanelId]?.[event]; | ||
| if (callback) { | ||
| callback(params); | ||
| } | ||
| } | ||
|
|
||
| function unregister(event: string, panelId?: string) { | ||
| const unregisterPanelId = panelId ?? computedPanelId; | ||
| assertPanelId(unregisterPanelId); | ||
| if (events[unregisterPanelId]) { | ||
| delete events[unregisterPanelId][event]; | ||
| } | ||
| } | ||
|
|
||
| return { register, trigger, unregister }; | ||
| } | ||
|
Comment on lines
+39
to
+40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainExpose a non-hook trigger for non-React call sites (operators, services) Triggers don’t require React state; calling this hook from non-React code would violate Rules of Hooks. Provide a pure helper and prefer it in operator code paths. Add this helper near the top-level (outside the hook): export function triggerPanelEvent(panelId: string, event: string, params?: unknown): boolean {
if (!panelId) return false;
const bucket = events[panelId]?.[event];
if (!bucket) return false;
if (bucket instanceof Set) {
if (!bucket.size) return false;
for (const cb of bucket) cb(params);
return true;
}
// Back-compat if single-handler shape remains somewhere
(bucket as unknown as (p: unknown) => void)(params);
return true;
}Verification (scan for hook usage outside components/hooks): 🏁 Script executed: #!/bin/bash
# Find direct invocations of the hook; inspect contexts for non-React usage
rg -n -C2 -S '\b(usePanelClientEvent|usePanelEvents)\s*\(' app/packagesLength of output: 940 Extract trigger logic into a standalone helper and update operator call sites • In app/packages/operators/src/Panel/hooks/usePanelClientEvent.tsx, add above the hook definition: export function triggerPanelEvent(
panelId: string,
event: string,
params?: unknown
): boolean {
if (!panelId) return false;
const bucket = events[panelId]?.[event];
if (!bucket) return false;
if (bucket instanceof Set) {
if (!bucket.size) return false;
for (const cb of bucket) cb(params);
return true;
}
(bucket as unknown as (p: unknown) => void)(params);
return true;
}• In app/packages/operators/src/built-in-operators.ts (line 1513), replace: const { trigger } = usePanelClientEvent();with: import { triggerPanelEvent } from './Panel/hooks/usePanelClientEvent';
const trigger = (params) => triggerPanelEvent(panelId, eventName, params);🤖 Prompt for AI Agents |
||
|
|
||
| function assertPanelId(panelId?: string) { | ||
| if (!panelId) { | ||
| throw new Error("Panel ID is required"); | ||
| } | ||
| return panelId; | ||
| } | ||
|
|
||
| type PanelEventHandler = (params: unknown) => void; | ||
| type PanelEvents = { | ||
| [key: string]: PanelEventHandler; | ||
| }; | ||
| type PanelsEvents = { | ||
| [key: string]: PanelEvents; | ||
| }; | ||
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.
🛠️ Refactor suggestion
Allow multiple handlers per event and return a boolean from trigger
Currently, registering the same event twice overwrites the previous handler. Support a Set of handlers per event and have trigger return whether anything ran.
Apply these diffs:
Also applies to: 22-29, 49-55
🤖 Prompt for AI Agents