Skip to content

Conversation

@streamich
Copy link
Contributor

Summary

Closes #72826

Moves core embeddable types on base interface from visualize embeddable interface.

@streamich streamich requested a review from a team July 23, 2020 12:09
@streamich streamich requested a review from a team as a code owner July 23, 2020 12:09
@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Jul 23, 2020
@Dosant
Copy link
Contributor

Dosant commented Jul 23, 2020

In this pr I removed explicit dependency from embeddable to data, but current pr makes decencies a bit worse?
At least good that it is just types, let's explicitly use #73033 (comment) to not create accidentally unwanted runtime dependency later?


I am ok with merging this, but I wonder if this is a bad thing that BaseEmbeddable interface has interfaces from data plugin? (even though they are optional)

Maybe we should have data_embeddable plugin which would have DateEmbeddable extends IEmbeddable and some other helpers around that area. And then visualise would depend on it?

@streamich streamich added the WIP Work in progress label Jul 24, 2020
Copy link
Contributor

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@streamich streamich added Team:AppArch v7.10.0 v8.0.0 and removed WIP Work in progress labels Aug 21, 2020
@elasticmachine
Copy link
Contributor

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

@streamich streamich added the release_note:skip Skip the PR/issue when compiling release notes label Aug 21, 2020
@streamich
Copy link
Contributor Author

@Dosant Chatted with Peter, agreed that it is ok to import types from data plugin. Also, we can consider adding a new data_embeddable plugin later if we really think it is worth it.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

page load bundle size

id value diff baseline
discoverEnhanced 29.2KB -210.0B 29.4KB

History

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

@streamich streamich merged commit 74ab989 into elastic:master Aug 21, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 21, 2020
* master: (71 commits)
  [Lens] Show 'No data for this field' for empty field in accordion (elastic#73772)
  Skip failing lens test
  Configure ScopedHistory consistenty regardless of URL used to mount app (elastic#75074)
  Fix returned payload by "search" usage collector (elastic#75340)
  [Security Solution] Fix missing key error (elastic#75576)
  Upgrade EUI to v27.4.1 (elastic#75240)
  Update datasets UI copy to data streams (elastic#75618)
  [Lens] Register saved object references (elastic#74523)
  [DOCS] Update links to Beats documentation (elastic#70380)
  [Enterprise Search] Convert our `public_url` route to `config_data` and collect initialAppData (elastic#75616)
  [Usage Collection Schemas] Remove Legacy entries (elastic#75652)
  [Dashboard First] Lens Originating App Breadcrumb (elastic#75470)
  Improve login UI error message. (elastic#75642)
  [Security Solution] modify circular deps checker to output images of circular deps graphs (elastic#75579)
  [Data Telemetry] Add index pattern to identify "meow" attacks (elastic#75163)
  Migrate CSP usage collector to `kibana_usage_collection` plugin (elastic#75536)
  [Console] Get ES Config from core (elastic#75406)
  [Uptime] Add delay in telemetry test (elastic#75162)
  [Lens] Use index pattern service instead saved object client (elastic#74654)
  Embeddable input (elastic#73033)
  ...
thomasneirynck pushed a commit to thomasneirynck/kibana that referenced this pull request Aug 21, 2020
* refactor: 💡 move timeRange, filters and query to base embeddabl

* refactor: 💡 use new base embeddable input in explore data

* feat: 🎸 import types as types
streamich added a commit that referenced this pull request Aug 22, 2020
* refactor: 💡 move timeRange, filters and query to base embeddabl

* refactor: 💡 use new base embeddable input in explore data

* feat: 🎸 import types as types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update base embeddable input

6 participants