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

Ui Actions explorer example #57006

Merged

Conversation

stacey-gammon
Copy link
Contributor

@stacey-gammon stacey-gammon commented Feb 6, 2020

Ui actions examples.

Default hello world trigger and action.

Screen Shot 2020-02-06 at 11 56 46 AM

Ability to dynamically add new ones to highlight the context menu:
Screen Shot 2020-02-06 at 11 56 51 AM

Screen Shot 2020-02-06 at 11 56 54 AM

Screen Shot 2020-02-06 at 11 56 57 AM

Newly added, triggers with context. Highlights the issue with nested executeTriggerContext calls. Types could really be improved here too, no type checking that the context emitted by the trigger and that needed by the action match up.

Screen Shot 2020-02-06 at 4 26 02 PM

Screen Shot 2020-02-06 at 4 25 55 PM

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@stacey-gammon stacey-gammon added v7.7.0 release_note:skip Skip the PR/issue when compiling release notes labels Feb 6, 2020
@elasticmachine
Copy link
Contributor

💔 Build Failed

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💔 Build Failed

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@ppisljar
Copy link
Member

@elasticmachine merge upstream

);
},
});
uiActionsApi.registerAction(dynamicAction);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be cleaning this up somewhere ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think so, though a page refresh will remove the dynamically added actions. I added a note in the success callout:

Refresh the page to reset state. It's up to the user of the system to persist state like this.

Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 75 to 80
deps.uiActions.registerAction(createPhoneUserAction(depsStart.uiActions));
deps.uiActions.registerAction(lookUpWeatherAction);
deps.uiActions.registerAction(viewInMapsAction);
deps.uiActions.registerAction(createEditUserAction(coreStart.overlays.openModal));
deps.uiActions.registerAction(makePhoneCallAction);
deps.uiActions.registerAction(showcasePluggability);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we register these in start life-cycle, or even better, in setup life-cycle?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved what I could to setup, rest in start, a couple needed some start context.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this PR, but in general, should we register everything in setup life-cycles? As that was the idea for the setup life-cycle. For example, if something needs start life-cycle contracts we can pass in the promise:

setup(core) {
  const start = core.getStartServices();
  deps.uiActions.registerAction(createPhoneUserAction(start));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Thanks, updated in this PR.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@stacey-gammon stacey-gammon merged commit 2ae70d9 into elastic:master Feb 11, 2020
stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Feb 11, 2020
* wip

* Move action registration out of AppMountContext fn

* Move all registration to setup

* Fix type error
stacey-gammon added a commit that referenced this pull request Feb 11, 2020
* wip

* Move action registration out of AppMountContext fn

* Move all registration to setup

* Fix type error
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 11, 2020
* master: (27 commits)
  Include actions new platform plugin for codeowners (elastic#57252)
  [APM][docs] 7.6 documentation updates (elastic#57124)
  Expressions refactor (elastic#54342)
  [ML] New Platform server shim: update annotation routes to use new platform router  (elastic#57067)
  Remove injected ui app vars from Canvas (elastic#56190)
  update max_anomaly_score route schema to handle possible undefined values (elastic#57339)
  [Add panel flyout] Moving create new to the top of SavedObjectFinder (elastic#56428)
  Add mock of a legacy ui api to re-enable Canvas storybook (elastic#56673)
  [monitoring] Removes noisy event received log (elastic#57275)
  Remove use of copied MANAGEMENT_BREADCRUMBS and use `setBreadcrumbs` from management section's mount (elastic#57324)
  Advanced Settings management app to kibana platform plugin (elastic#56931)
  [ML] New Platform server shim: update recognize modules routes to use new platform router (elastic#57206)
  [ML] Fix overall stats for saved search on the Data Visualizer page (elastic#57312)
  [ML] [NP] Removing ui imports (elastic#56358)
  [SIEM] Fixes failing Cypress tests (elastic#57202)
  Create observability CODEOWNERS reference (elastic#57109)
  fix results service schema (elastic#57217)
  don't register a wrapper if browser side function exists. (elastic#57196)
  Ui Actions explorer example (elastic#57006)
  Fix update alert API to still work when AAD is out of sync (elastic#57039)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Actions release_note:skip Skip the PR/issue when compiling release notes review v7.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants