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

[canvas] Create Notify Service; remove legacy service #103821

Merged

Conversation

clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented Jun 30, 2021

Summary

This PR creates a Notify Service based on the Services Abstraction API and removes the legacy service. It also hooks into Storybook actions automatically to display notifications from the API.

What is this?

In #88112 the Presentation Team created a Service Abstraction API which allows any of our solutions to uniformly (and safely) create abstractions between external services and the means by which we use them. It also provides a standard method for "swapping" service implementations at "start" time, (e.g. Kibana setup/start, or Storybook start, or Jest setup, etc). That means, for example, Storybook Controls can give arguments to the services to adjust mocks on-the-fly from the Storybook application.

For more information, check out the docs.

Canvas already had a rough service abstraction layer that was a bit rigid, but was also unsafe: the singleton providing services was defined as a module-level constant. This means it exists and can be accessed outside the setup/start lifecycle of a Kibana plugin. 😞

This forthcoming collection of PRs moves each individual service abstraction to our "official" API and removes the legacy service. This makes it easier to review and test each service.

@clintandrewhall clintandrewhall added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:medium Medium Level of Effort v8.0.0 release_note:skip Skip the PR/issue when compiling release notes impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Feature:Canvas v7.15.0 labels Jun 30, 2021
@clintandrewhall clintandrewhall force-pushed the canvas/notify-service branch 2 times, most recently from eab0e15 to 8c61b24 Compare June 30, 2021 18:19
@clintandrewhall clintandrewhall force-pushed the canvas/notify-service branch from 8c61b24 to e781468 Compare June 30, 2021 21:22
@clintandrewhall clintandrewhall marked this pull request as ready for review July 1, 2021 00:32
@clintandrewhall clintandrewhall requested a review from a team as a code owner July 1, 2021 00:32
@elasticmachine
Copy link
Contributor

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

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
canvas 1.2MB 1.2MB +294.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
canvas 463.6KB 463.4KB -174.0B

History

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

Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

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

Sweet!

@clintandrewhall clintandrewhall merged commit 8bb13a9 into elastic:master Jul 1, 2021
@clintandrewhall clintandrewhall deleted the canvas/notify-service branch July 1, 2021 16:09
@clintandrewhall clintandrewhall added the auto-backport Deprecated - use backport:version if exact versions are needed label Jul 1, 2021
clintandrewhall added a commit to clintandrewhall/kibana that referenced this pull request Jul 1, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 1, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Canvas impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants