-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Add Lens embeddables #57499
Conversation
cfc9227
to
ab5a817
Compare
@timductive I never ran into the issue you encountered, but the branch has been updated and should be working now. |
8d7f860
to
5e37003
Compare
Pinging @elastic/kibana-canvas (Team:Canvas) |
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.
LGTM from the design and functionality side, works as expected/described.
This is going to be a huge win for Canvas and Lens!
Thanks for moving those styles to a better home :)
@cqliu1 I spoke with @rayafratkina and let's throw the "b" beta icon next to the elements in the flyout to get this in 7.7 |
I'm not sure it's possible to add a beta icon next to each Lens object in our embeddable flyout. We're using the |
52c049a
to
d487046
Compare
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.
Looks good. Let's just clean up for sure the way we determine if it's edit vs view mode.
x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/embeddable/embeddable.tsx
Outdated
Show resolved
Hide resolved
x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/embeddable/embeddable.tsx
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,33 @@ | |||
.canvasEmbeddable { | |||
.embPanel { | |||
border: none; |
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.
Maybe more of a @ryankeairns concern, but I think we should keep the border at least in edit mode. It feels weird having the gear hanging out in the top disconnected from everything, but the border at least boxes it in and gives you some clue that all of that is tied in together.
Fixed import embedded panel styles (#58654) Merging to WIP draft branch
Update: I removed the changes for dynamically setting the embeddable view mode based on app state. The way I had it working by retrieving the entire app state from redux every time an embeddable was rendered, and that's not ideal. I'll tackle the view mode enhancement in a separate PR. |
485f8de
to
bc74d9e
Compare
bc74d9e
to
d6d43ed
Compare
* Added lens embeddables to embed flyout Fixed import embedded panel styles (#58654) Merging to WIP draft branch * Added i18n strings for savedLens * Added tests for lens embeddables * Updated tests * Updated tests * Added style overrides for lens table * DDisables triggers on lens emebeddable * Updated test * Sets embeddable view mode according to app state * Fix embeddable component * Removed embeddable view mode logic * Removed unused import
💔 Build FailedTest FailuresKibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/discover/_discover_histogram·js.discover app discover histogram should visualize monthly data with different day intervalsStandard Out
Stack Trace
Kibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/visualize/_area_chart·js.visualize app area charts axis scaling does not scale top hit aggStandard Out
Stack Trace
Kibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/visualize/_area_chart·js.visualize app area charts axis scaling does not scale top hit aggStandard Out
Stack Trace
and 1 more failures, only showing the first 3. History
To update your PR or re-run it, just comment with: |
Summary
This adds support for Lens embeddables within Canvas and allows Lens visualizations to be added to workpads.
Other Notable Changes
Default styles for embeddable wrapper
We applied CSS overrides to reset background color and border on the wrapper component for all embeddables
Before
After
Note: There is a little funkiness with the Lens charts that allow you to toggle the visibility of each series by clicking on the labels in the legend. It doesn't persist if you turn off a series or two and switch to fullscreen mode or refresh the the page. Users can get around this by filtering out the series they don't want to see, so it's a weird UX edge case that we don't necessarily need to fix for. Ideally we would disable these interactions in Canvas, but for now, I don't think it's a blocker for this feature.
Checklist
Delete any items that are not applicable to this PR.
For maintainers