-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 container plugin #38974
Dashboard embeddable container plugin #38974
Conversation
c6dca72
to
fd14ce5
Compare
Pinging @elastic/kibana-app-arch |
💚 Build Succeeded |
maximizedPanelId: string; | ||
useMargins: boolean; | ||
}) { | ||
// This is to prevent a bug where view mode changes when the panel is expanded. View mode changes will trigger |
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.
Not a complete copy from the original dashboard_grid.tsx
, some code was removed that I think is no longer needed with the newer styles, like the old onBlur, onFocus handling, and the in-grid panel migrations away from older style data is moved out, all migrations should have occurred before we get here, and data be in the shape we expect.
💔 Build Failed |
jenkins, test this |
💚 Build Succeeded |
ff5cede
to
41672e9
Compare
💚 Build Succeeded |
41672e9
to
140fd5c
Compare
💔 Build Failed |
jenkins, test this |
💚 Build Succeeded |
4be536b
to
0bdea47
Compare
💔 Build Failed |
Failed on
jenkins, test this |
💔 Build Failed |
1600b8d
to
7c69835
Compare
💚 Build Succeeded |
7c69835
to
f436730
Compare
💚 Build Succeeded |
💔 Build Failed |
ca7f0b8
to
8e1c294
Compare
💔 Build Failed |
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.
It looks like my recent removal of createRegistry
breaks this build.
src/legacy/core_plugins/dashboard_embeddable_container/public/actions/index.ts
Show resolved
Hide resolved
src/legacy/core_plugins/dashboard_embeddable_container/index.ts
Outdated
Show resolved
Hide resolved
...y/core_plugins/dashboard_embeddable_container/public/embeddable/dashboard_container.test.tsx
Outdated
Show resolved
Hide resolved
...core_plugins/dashboard_embeddable_container/public/embeddable/dashboard_container_factory.ts
Outdated
Show resolved
Hide resolved
src/legacy/core_plugins/dashboard_embeddable_container/public/np_core.test.mocks.ts
Show resolved
Hide resolved
💔 Build Failed |
84b903a
to
c314da3
Compare
💔 Build Failed |
Looks like a flaky failure, everything passed but:
jenkins, test this |
💚 Build Succeeded |
be21a95
to
610b706
Compare
💚 Build Succeeded |
Think I've addressed all pending comments, lmk if I missed anything! |
|
||
// TODO: uncomment once the duplicate styles are removed from the dashboard app itself. | ||
// MUST STAY AT THE BOTTOM BECAUSE OF DARK THEME IMPORTS | ||
// @import './embeddable/index'; |
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.
Over here, cc @cchaos
* Dashboard embeddable plugin * comment out duplicate scss styles * review: conform closer to NP standards * export/import from index file in folder
Adding
dashboard_embeddable_container
plugin which uses the new embeddable API. Still no real life usage except in sample plugin.The final step will be swapping out the old infrastructure with the new in the dashboard plugin, along with the three current embeddable types on the old infrastructure.
It shouldn't be a huge PR but I'm still trying to clean up some dashboard_app stuff, and migrations, to make it a little easier to review, so figured to not waste time I can send this one out separately.