feat: Add unified callback API for easier Phoenix integration#831
feat: Add unified callback API for easier Phoenix integration#831GaneshPatil7517 wants to merge 3 commits intoHSF:mainfrom
Conversation
- Add public callback registration methods to EventDisplay: * onObjectSelected() - Called when an object is selected * onObjectDeselected() - Called when an object is deselected * onObjectHovered() - Called when an object is hovered * onObjectHoverEnd() - Called when hover ends * onSelectionChanged() - Called when selection state changes - Add internal callback infrastructure in SelectionManager: * Setter methods for registering callbacks * Fire callbacks in selectObject/deselectObject methods - Connect EventDisplay callbacks to SelectionManager via ThreeManager: * New setSelectionCallbacks() method in ThreeManager * setupSelectionCallbacks() method in EventDisplay init This allows external applications to easily integrate with Phoenix by registering callbacks on common selection actions without needing to understand the internal manager architecture. Fixes HSF#826
There was a problem hiding this comment.
Pull request overview
Adds an external callback/subscription mechanism for object selection and hover events, wiring SelectionManager → ThreeManager → EventDisplay so external consumers can react to interaction changes.
Changes:
- Added configurable selection/hover callbacks to
SelectionManagerand invoked selection-related callbacks in programmatic select/deselect flows. - Added a
ThreeManager.setSelectionCallbacksbridge to attach callbacks ontoSelectionManager. - Added
EventDisplaysubscription APIs (onObjectSelected,onObjectDeselected, etc.) and wired them intoThreeManagerduring init. - Added a new
documentation/js/routes/routes_index.jsfile.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| packages/phoenix-event-display/src/managers/three-manager/selection-manager.ts | Introduces callback fields + setters and fires callbacks in selectObject/deselectObject. |
| packages/phoenix-event-display/src/managers/three-manager/index.ts | Adds setSelectionCallbacks wrapper to register callbacks on the SelectionManager. |
| packages/phoenix-event-display/src/event-display.ts | Adds subscription APIs and wires them into ThreeManager on init. |
| packages/phoenix-event-display/documentation/js/routes/routes_index.js | Adds a routes index file under documentation/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/phoenix-event-display/src/managers/three-manager/selection-manager.ts
Show resolved
Hide resolved
packages/phoenix-event-display/src/managers/three-manager/selection-manager.ts
Show resolved
Hide resolved
packages/phoenix-event-display/documentation/js/routes/routes_index.js
Outdated
Show resolved
Hide resolved
packages/phoenix-event-display/src/managers/three-manager/selection-manager.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR enables previously skipped unit tests in the Angular packages and introduces a new selection/hover callback subscription API in phoenix-event-display intended for external integrations.
Changes:
- Unskips and updates several Jest tests in
phoenix-ngto use more explicit mocks/spies. - Adds selection/hover callback plumbing across
SelectionManager,ThreeManager, andEventDisplay. - Adds a documentation route index JS file under
phoenix-event-display/documentation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/phoenix-ng/projects/phoenix-ui-components/lib/services/file-loader.service.component.test.ts | Unskips and rewrites FileLoaderService test using instance spies instead of a standalone mock object. |
| packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/event-data-explorer/event-data-explorer-dialog/event-data-explorer-dialog.component.ts | Removes circular type import by introducing a local EventDataExplorerDialogData type. |
| packages/phoenix-ng/projects/phoenix-ui-components/lib/components/ui-menu/event-data-explorer/event-data-explorer-dialog/event-data-explorer-dialog.component.test.ts | Unskips dialog tests and injects FileLoaderService via a mock provider; improves eventDisplay mock. |
| packages/phoenix-ng/projects/phoenix-app/src/app/sections/cms/cms.component.test.ts | Unskips CMSComponent tests and mocks CMSLoader behavior via prototype spies. |
| packages/phoenix-event-display/src/managers/three-manager/selection-manager.ts | Adds callback registration + firing hooks for selection and hover state changes. |
| packages/phoenix-event-display/src/managers/three-manager/index.ts | Adds ThreeManager.setSelectionCallbacks to forward callbacks into SelectionManager. |
| packages/phoenix-event-display/src/event-display.ts | Adds public subscription APIs (onObjectSelected, etc.) and wires them to ThreeManager selection callbacks. |
| packages/phoenix-event-display/documentation/js/routes/routes_index.js | Adds a ROUTES_INDEX variable file under documentation routes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...-menu/event-data-explorer/event-data-explorer-dialog/event-data-explorer-dialog.component.ts
Outdated
Show resolved
Hide resolved
packages/phoenix-event-display/src/managers/three-manager/selection-manager.ts
Show resolved
Hide resolved
packages/phoenix-event-display/src/managers/three-manager/selection-manager.ts
Outdated
Show resolved
Hide resolved
packages/phoenix-event-display/src/managers/three-manager/index.ts
Outdated
Show resolved
Hide resolved
packages/phoenix-ng/projects/phoenix-app/src/app/sections/cms/cms.component.test.ts
Show resolved
Hide resolved
packages/phoenix-event-display/documentation/js/routes/routes_index.js
Outdated
Show resolved
Hide resolved
|
hey @sponce and @EdwardMoyse the pr is ready to review please review it when you get the time...... |
|
Thanks for this @GaneshPatil7517 ! I won't have time to do a deep review in the short term but this is already a nice base. Ideally, we should try to use this interface already, e.g. in the LHCb masterclass, to validate that it fulfills the needs. This will obviously take some time |
hello @sponce I agree, real validation in the LHCb masterclass is the right next step. I can create a minimal integration prototype and report back on what works well and what should be improved in the interface. Then we can do small targeted follow-up changes based on that validation. |
Implements issue #826 by providing a unified, easy-to-use callback API that allows external applications to easily register callbacks on Phoenix actions without needing direct access to internal managers.
Changes Made
EventDisplay Class (
src/event-display.ts)Added 5 new public methods for registering selection callbacks:
onObjectSelected(callback)- Called when an object is selectedonObjectDeselected(callback)- Called when an object is deselectedonObjectHovered(callback)- Called when an object is hoveredonObjectHoverEnd(callback)- Called when hover endsonSelectionChanged(callback)- Called when selection changesAll methods return unsubscribe functions for proper cleanup
Added internal
setupSelectionCallbacks()to wire up callbacks on initAdded internal fire*Callback methods to dispatch events to registered listeners
SelectionManager Class (
src/managers/three-manager/selection-manager.ts)setOnObjectSelectedCallback(),setOnObjectDeselectedCallback(), etc.selectObject()anddeselectObject()methodsThreeManager Class (
src/managers/three-manager/index.ts)setSelectionCallbacks()method to configure SelectionManager callbacksBenefits
Testing
Example Usage
Fixes #826