-
Notifications
You must be signed in to change notification settings - Fork 19
feat(ui): DH-23183: Event mapping framework for Element plugins #1396
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
Open
mofojed
wants to merge
10
commits into
deephaven:main
Choose a base branch
from
mofojed:DH-23183-event-plugin-framework
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
af3eb80
feat(ui): add ui.notification API using the browser Notifications API
mofojed 37a1141
feat(ui): add ui.tone for playing tones and jingles
mofojed 5d973cc
Wire up EventPlugin framework
mofojed 912b7c5
Revert "feat(ui): add ui.tone for playing tones and jingles"
mofojed 5032d6c
Revert "feat(ui): add ui.notification API using the browser Notificat…
mofojed 31770bc
Refactor to put the event handling stuff into EventUtils
mofojed 9762f8c
Mention custom event handlers
mofojed cd34cbe
Remove reference to ui.notification, which does not exist yet
mofojed b58e04c
Update readmes based on review
mofojed 543a388
feat(ui): namespace built-in events with deephaven.ui prefix
mofojed File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| from __future__ import annotations | ||
|
|
||
| NAVIGATE_EVENT = "deephaven.ui.navigate" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { | ||
| type ElementPlugin, | ||
| isElementPlugin, | ||
| type PluginModuleExport, | ||
| } from '@deephaven/plugin'; | ||
|
|
||
| /** | ||
| * A handler for an event sent from deephaven.ui via `use_send_event`. | ||
| * The params are the JSON-decoded payload of the event, with any callables | ||
| * re-hydrated into callable functions. | ||
| */ | ||
| export type UIEventHandler = (params: Record<string, unknown>) => void; | ||
|
|
||
| /** A mapping of event names to their handlers. */ | ||
| export type UIEventMapping = Record<string, UIEventHandler>; | ||
|
|
||
| /** | ||
| * An event plugin is an {@link ElementPlugin} that additionally handles custom | ||
| * events sent from deephaven.ui via `use_send_event`. The `eventMapping` | ||
| * contains the event names as keys and the handlers as values. | ||
| * | ||
| * Event names should be namespaced with the plugin's package namespace to avoid | ||
| * collisions. Built-in events are namespaced with `deephaven.ui`. | ||
| * | ||
| * Because an event plugin is also an element plugin, the `mapping` property is | ||
| * still required. If the plugin only handles events and does not render any | ||
| * elements, set `mapping` to an empty object. | ||
| */ | ||
| export interface EventPlugin extends ElementPlugin { | ||
| eventMapping: UIEventMapping; | ||
| } | ||
|
|
||
| /** Type guard to check if the given plugin is an {@link EventPlugin}. */ | ||
| export function isEventPlugin( | ||
| plugin: PluginModuleExport | ||
| ): plugin is EventPlugin { | ||
| return ( | ||
| isElementPlugin(plugin) && | ||
| 'eventMapping' in plugin && | ||
| (plugin as Partial<EventPlugin>).eventMapping != null | ||
| ); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import { type PluginModuleMap } from '@deephaven/plugin'; | ||
| import { getPluginsEventMap } from './usePluginsEventMap'; | ||
|
|
||
| function makeEventPlugin( | ||
| name: string, | ||
| eventMapping: Record<string, (params: Record<string, unknown>) => void> | ||
| ): [string, unknown] { | ||
| return [name, { name, type: 'ElementPlugin', mapping: {}, eventMapping }]; | ||
| } | ||
|
|
||
| function makeElementPlugin(name: string): [string, unknown] { | ||
| return [name, { name, type: 'ElementPlugin', mapping: {} }]; | ||
| } | ||
|
|
||
| it('extracts event handlers from event plugins', () => { | ||
| const handlerA = jest.fn(); | ||
| const handlerB = jest.fn(); | ||
| const plugins = new Map([ | ||
| makeEventPlugin('plugin-a', { 'a.event': handlerA }), | ||
| makeElementPlugin('plugin-element'), | ||
| makeEventPlugin('plugin-b', { 'b.event': handlerB }), | ||
| ]) as unknown as PluginModuleMap; | ||
|
|
||
| const eventMap = getPluginsEventMap(plugins); | ||
|
|
||
| expect(eventMap.size).toBe(2); | ||
| expect(eventMap.get('a.event')).toBe(handlerA); | ||
| expect(eventMap.get('b.event')).toBe(handlerB); | ||
| }); | ||
|
|
||
| it('returns an empty map when there are no event plugins', () => { | ||
| const plugins = new Map([ | ||
| makeElementPlugin('plugin-element'), | ||
| ]) as unknown as PluginModuleMap; | ||
|
|
||
| expect(getPluginsEventMap(plugins).size).toBe(0); | ||
| }); | ||
|
|
||
| it('uses the last registered handler and warns on duplicate event names', () => { | ||
| const first = jest.fn(); | ||
| const second = jest.fn(); | ||
| const plugins = new Map([ | ||
| makeEventPlugin('plugin-a', { 'dup.event': first }), | ||
| makeEventPlugin('plugin-b', { 'dup.event': second }), | ||
| ]) as unknown as PluginModuleMap; | ||
|
|
||
| const eventMap = getPluginsEventMap(plugins); | ||
|
|
||
| expect(eventMap.size).toBe(1); | ||
| expect(eventMap.get('dup.event')).toBe(second); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { useMemo } from 'react'; | ||
| import { usePlugins } from '@deephaven/plugin'; | ||
| import Log from '@deephaven/log'; | ||
| import { type UIEventHandler, isEventPlugin } from './EventPlugin'; | ||
|
|
||
| const log = Log.module('usePluginsEventMap'); | ||
|
|
||
| /** | ||
| * Get a mapping of event names to their handlers from the given plugin map. | ||
| * | ||
| * If multiple plugins register a handler for the same event name, the last one | ||
| * registered wins and a warning is logged. | ||
| * | ||
| * @param pluginMap The plugin map to extract event plugins from. | ||
| * @returns A Map of event names to their handlers. | ||
| */ | ||
| export function getPluginsEventMap( | ||
| pluginMap: ReturnType<typeof usePlugins> | ||
| ): Map<string, UIEventHandler> { | ||
| const eventMap = new Map<string, UIEventHandler>(); | ||
| [...pluginMap.values()].filter(isEventPlugin).forEach(plugin => { | ||
| Object.entries(plugin.eventMapping).forEach(([name, handler]) => { | ||
| if (eventMap.has(name)) { | ||
| log.warn( | ||
| `Multiple plugins registered a handler for event "${name}". The last one registered will be used.` | ||
| ); | ||
| } | ||
| eventMap.set(name, handler); | ||
| }); | ||
| }); | ||
| return eventMap; | ||
| } | ||
|
|
||
| /** | ||
| * Get all event handlers registered by {@link EventPlugin}s from the plugins | ||
| * context. | ||
| * @returns A Map of event names to their handlers. | ||
| */ | ||
| export function usePluginsEventMap(): Map<string, UIEventHandler> { | ||
| const plugins = usePlugins(); | ||
| return useMemo(() => getPluginsEventMap(plugins), [plugins]); | ||
| } | ||
|
|
||
| export default usePluginsEventMap; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
There's some collision risk here. We should probably namespace our events + throw warning if there is an eventMap collision rather than just silently override + have a note of that event namespacing in the docs somewhere.