Skip to content
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

[dashboard][embeddable] break dashboard and embeddable triggers into separate registries #202986

Closed
nreese opened this issue Dec 4, 2024 · 2 comments
Labels
discuss Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@nreese
Copy link
Contributor

nreese commented Dec 4, 2024

Problem

Currently, embeddable and dashboard actions are registered in uiActions trigger action registry.

To register a context menu action

import { synchronizeMovementAction } from './some_file';
plugins.uiActions.addTriggerAction(CONTEXT_MENU_TRIGGER, synchronizeMovementAction);

To register an "Add panel" action

import { swimlaneAddPanelAction } from './some_file';
plugins.uiActions.addTriggerAction(ADD_PANEL_TRIGGER, swimlaneAddPanelAction);

There are 2 problems with this pattern

  1. trigger action registry is synchronous so action is included in initial page load. See [uiActions] make trigger action registry async #191642 for more details
  2. Syntax is obtuse and discoverability is low. Developers have to know about uiActions registry and trigger constant. The registry is not tied to the embeddable setup API.

Proposed solution

Instead, each trigger should be broken into its own registry.

To register a context menu action

plugins.embeddable.registerContextMenuAction(SYNCRONIZE_MOVEMENT_ACTION_ID, async () => {
  const { synchronizeMovementAction } = await import(
     './actions/synchronize_movement_action'
  );
  return synchronizeMovementAction;
});

To register a add panel action

plugins.embeddable.registerAddPanelAction(ADD_SWIMLANE_PANEL_ACTION_ID, async () => {
  const { swimlaneAddPanelAction } = await import(
     './some_file'
  );
  return swimlaneAddPanelAction;
});

Benefits

  1. Resolve uiActions synchronous registry issue for our use cases.
  2. Reduce uiActions synchronous registry scope. Most actions belong to CONTEXT_MENU_TRIGGER and ADD_PANEL_TRIGGER triggers. Moving these to separate registries will reduce the complexity in migrating uiActions registry.
  3. Better developer syntax. Registering embeddable and dashboard actions are better scoped to embeddable setup API for easier discoverability.
  4. Moving registry closer to its consumption improves maintainability. Testing becomes easier since tests no longer need to mock uiActions but instead mock a local resource.
  5. Reduced complexity by eliminating trigger and usage of uiActions one-to-many registry.

Potential problems

  1. URL drilldowns can be added to context menu, Context menu trigger for URL Drilldown #81158. Work around to merge uiActions CONTEXT_MENU_TRIGGER actions and registry actions.

@nreese nreese added discuss Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Dec 4, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@nreese nreese changed the title [dashboard] break dashboard and embeddable triggers into separate registries [dashboard][embeddable] break dashboard and embeddable triggers into separate registries Dec 4, 2024
@ThomThomson
Copy link
Contributor

Closing this issue as we're likely to focus on making the whole UI actions registry async rather than splitting off our own registries. We can reopen this if we revisit the decision.

@ThomThomson ThomThomson closed this as not planned Won't fix, can't repro, duplicate, stale Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

No branches or pull requests

3 participants