[Canvas] Adds editor menu to Canvas #113194

cqliu1 · 2021-09-27T21:54:07Z

Summary

Related to #108439.
Related to #81812.
~~Blocked by #94489.~~
~~Blocked by #111411.~~

This adds an editor menu to Canvas where users can create embeddables from editors outside of Canvas.

Outstanding tasks:

Add to library/Unlink from library actions
Save and return takes you to Canvas home instead of workpad you left Preserves originating path when returning from editor #115118
Remove embeddable border [Canvas] Hide dashed border on embeddable panel #113985
ML embeddables aren't supported yet [Canvas] Enable ML Embeddables in Canvas #113309

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

ryankeairns · 2021-09-30T15:23:12Z

Enabling embeddables in Canvas and aligning the toolbar UX is a nice milestone. I took an initial pass to review the functionality and came away with the following notes:

Consistent toolbars. Overall, the toolbars feel consistent between Dashboard and Canvas. Eventually, we could move View, Edit, and Share up into header/top menu, but the current layout feels acceptable for this initial version.
Within 'All types' we may want to hide/remove a few items: Aggregation, Controls, and Text are all types of Visualize objects that we are moving away from in favor of Lens. That said, they also make up a large portion of customer's library content and I'm less certain we would want to remove 'Add from library'. Product input here would be helpful.
Fun with filters! Catherine noted some known limitations in how these currently behave (i.e. there is conflict between the Canvas date filter and the date filter within the embeddable panel itself). My understanding is that this topic involves deeper, technical considerations that are out of scope for this PR, so we agreed that documentation should be added, at minimum.
Remove the thick, dashed border. There is a known limitation here where the embeddable content is always in it's 'edit' state, for now. Given this, it would be preferable to remove the border and have this content appear visually like other native Canvas elements.
What to do about the 'Add to/Remove from library' actions. I feel like these should appear here, on panels, just as they do in Dashboard. We're enabling the 'Add from library' feature in the toolbar and also creating non-library content (via All types), so it seems as though it should be here too as means for people getting unstuck. Prior results of user testing revealed frustration when objects created in a dashboard did not appear in the library and these actions provide a way out of that jam.
A Canvas 'wrapper'. I can see how this might resolve the header feeling 'out of place' in the Canvas context, however, the header bar is the only grabbable area for moving the embedded content and it seems necessary to display the time filter given the tricky relationship it currently has with the Canvas date filter. Perhaps we should revisit this later if/when we can diferentiate between 'edit' and 'view' modes. Or, maybe we only show the header (title, time filter badge, gear actions) on hover or selection of the panel? At minimum, this will look better after the thick, dashed border is removed.

All in all, great progress and nothing feels like a blocker. Being able to add all of this new non-native content would be a nice boost for Canvas users.

Update
🐞 There does seem to be a bug with the 'Save and return' logic. It works when I start from a brand new workpad, but whenever I edit an existing panel, I end up back on the Canvas home screen.

cqliu1 · 2021-10-12T18:43:48Z

5. What to do about the 'Add to/Remove from library' actions. I feel like these should appear here, on panels, just as they do in Dashboard. We're enabling the 'Add from library' feature in the toolbar and also creating non-library content (via All types), so it seems as though it should be here too as means for people getting unstuck. Prior results of user testing revealed frustration when objects created in a dashboard did not appear in the library and these actions provide a way out of that jam.

After digging into the add to/unlink from library actions on Dashboard, they only work specifically for Dashboard, and there isn't a way to reuse that code for Canvas. I'd have to create new actions in Canvas to achieve the same actions, which I think should be tackled as a follow up enhancement. I'll file an issue for it.

andreadelrio

I'm not sure you need to override the fills for the context menu icons.

Also, something is up with the quickButtonGroup's border-radius

andreadelrio · 2021-10-20T05:44:04Z

x-pack/plugins/canvas/public/components/workpad_header/editor_menu/editor_menu.scss

+  .euiIcon path {
+    fill: $euiColorGhost;
+  }
+}
+
+.canvasSolutionToolbar__editorContextMenu--light {
+  .euiIcon path {
+    fill: $euiColorInk;


Do you really need these two fill declarations here? As far as I can tell they're not making a difference.

It looks like these styles are no longer necessary to apply to get monochrome icons. Before some of the app icons would be two toned, but that seems to be fixed in EUI. I'll go ahead and remove this file.

crob611

Looks good to me 👍

cqliu1 · 2021-10-20T18:38:24Z

Also, something is up with the quickButtonGroup's border-radius

It looks like the wrapper for the group is applying a border radius but the actual border outline is coming from the buttons within which don't apply a border radius.

I just pushed a commit (a442afe) that fixes the border.

andreadelrio · 2021-10-20T19:00:40Z

src/plugins/presentation_util/public/components/solution_toolbar/items/quick_group.scss

-      border-style: solid !important;
-      border-color: $euiBorderColor !important;
+  .euiButtonGroup__buttons {
+    border-radius: 6px;


Remember to use EUI variables whenever possible. This one should be something like $euiBorderRadius

I updated the styles to use $euiBorderRadius.

andreadelrio

Thanks for making those changes. LGTM.

Added stories Updated editor menu button label in Canvas and Dashboard Fixed label typo

kibanamachine · 2021-10-21T01:11:44Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`canvas`	1079	1082	+3

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.0MB	1.0MB	+2.8KB
`dashboard`	143.3KB	143.3KB	+2.0B
total			+2.8KB

Page load bundle

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

id	before	after	diff
`canvas`	17.3KB	17.6KB	+341.0B
`presentationUtil`	43.8KB	44.8KB	+994.0B
total			+1.3KB

History

💔 Build #719 failed 3e993123d4fd28d94da62ae6165e17100b15d9b5
💔 Build #693 failed 2e55da3d9b41ea7b3f792f860d6801a7f33e7840
💔 Build #645 failed a442afeb1d51263525ccfeab11d1754f64c50cd7
💔 Build #614 failed c27437f4682a2b44ff67c11f308d9f1b4bbbb0f6

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

* [Canvas] Generic embeddable function (#104499) * Created generic embeddable function Fixed telemetry Updates expression on input change Fixed ts errors Store embeddable input to expression Added lib functions Added comments Fixed type errors Fixed ts errors Clean up Removed extraneous import Added context type to embeddable function def Fix import Update encode/decode fns Moved embeddable data url lib file Added embeddable test Updated comment * Fix reference extract/inject in embeddable fn * Simplify embeddable toExpression * Moved labsService to flyout.tsx * Added comment * [Canvas] Adds Save and Return Workflow (#111411) * [Canvas] Adds editor menu to Canvas (#113194) * Merge existing embeddable input with incoming embeddable input (#116026) * [Canvas] Extract and inject references for by-value embeddables (#115124) * Extract/inject references for by-value embeddables in embeddable function Fixed server interpreter setup Register external functions in canvas_plugin_src plugin def * Fixed ref name in embeddable.inject * Fixed ts errors * Fix missing type error Co-authored-by: Kibana Machine <[email protected]>

cqliu1 added enhancement New value added to drive a business result Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:large Large Level of Effort Feature:Canvas labels Sep 27, 2021

cqliu1 force-pushed the canvas/editor-menu branch from 77970cb to cac0861 Compare September 27, 2021 22:05

cqliu1 mentioned this pull request Sep 28, 2021

[Canvas] Enable ML Embeddables in Canvas #113309

Closed

9 tasks

cqliu1 force-pushed the canvas/editor-menu branch from cac0861 to f506db1 Compare October 3, 2021 22:57

cqliu1 force-pushed the canvas/by-value-embeddables branch from e669879 to ddb8632 Compare October 5, 2021 18:54

cqliu1 force-pushed the canvas/editor-menu branch from 55cd7ea to 72592c4 Compare October 5, 2021 19:03

cqliu1 force-pushed the canvas/by-value-embeddables branch from ddb8632 to b706dfd Compare October 5, 2021 20:57

cqliu1 force-pushed the canvas/editor-menu branch from 72592c4 to 2e5bf8a Compare October 5, 2021 20:58

spalger mentioned this pull request Oct 6, 2021

Failing test: Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/canvas/custom_elements·ts - Canvas app custom elements adds the custom element to the workpad when prompted #63339

Closed

cqliu1 force-pushed the canvas/by-value-embeddables branch from b706dfd to 2030a15 Compare October 12, 2021 18:55

cqliu1 force-pushed the canvas/editor-menu branch 6 times, most recently from f67b41d to 1ec23f6 Compare October 14, 2021 17:50

cqliu1 mentioned this pull request Oct 14, 2021

Preserves originating path when returning from editor #115118

Merged

9 tasks

cqliu1 force-pushed the canvas/editor-menu branch from 1ec23f6 to 6115f78 Compare October 15, 2021 00:13

cqliu1 force-pushed the canvas/by-value-embeddables branch from 2030a15 to afea023 Compare October 15, 2021 17:33

cqliu1 force-pushed the canvas/editor-menu branch from 6115f78 to 27d7cdd Compare October 15, 2021 17:35

cqliu1 force-pushed the canvas/by-value-embeddables branch from afea023 to 6b2bc48 Compare October 15, 2021 19:47

cqliu1 force-pushed the canvas/editor-menu branch 2 times, most recently from 88cccad to 634eabb Compare October 15, 2021 19:53

cqliu1 mentioned this pull request Oct 15, 2021

[Canvas] Removes by-value embeddable lab flag #115277

Closed

9 tasks

cqliu1 force-pushed the canvas/editor-menu branch from 634eabb to e38e252 Compare October 15, 2021 21:30

andreadelrio requested changes Oct 20, 2021

View reviewed changes

crob611 approved these changes Oct 20, 2021

View reviewed changes

cqliu1 force-pushed the canvas/by-value-embeddables branch from 34831c5 to 6a29232 Compare October 20, 2021 18:58

cqliu1 force-pushed the canvas/editor-menu branch from 7a9e23e to a442afe Compare October 20, 2021 18:58

andreadelrio requested changes Oct 20, 2021

View reviewed changes

cqliu1 force-pushed the canvas/editor-menu branch from 52b545f to 2e55da3 Compare October 20, 2021 20:46

andreadelrio self-requested a review October 20, 2021 20:51

andreadelrio approved these changes Oct 20, 2021

View reviewed changes

cqliu1 force-pushed the canvas/by-value-embeddables branch from 6a29232 to 8ac6d41 Compare October 20, 2021 22:45

cqliu1 added 7 commits October 20, 2021 15:45

Added editor menu to canvas

534a19e

Added stories Updated editor menu button label in Canvas and Dashboard Fixed label typo

Removed editor_menu scss

38b5413

Fixed button group border-radius

538accf

Fix snapshots

f542778

Use euiBorderRadius

d1e5c8a

Fixed ts errors

18a4c89

Fixed imports

d6ca746

cqliu1 force-pushed the canvas/editor-menu branch from 3e99312 to d6ca746 Compare October 20, 2021 22:51

cqliu1 merged commit c637735 into elastic:canvas/by-value-embeddables Oct 21, 2021

cqliu1 deleted the canvas/editor-menu branch October 21, 2021 01:39

cqliu1 mentioned this pull request Oct 21, 2021

[Canvas] By-Value Embeddables #113827

Merged

11 tasks

cqliu1 added a commit that referenced this pull request Oct 25, 2021

[Canvas] Adds editor menu to Canvas (#113194)

780f71c

cqliu1 added a commit that referenced this pull request Oct 25, 2021

[Canvas] Adds editor menu to Canvas (#113194)

151ceac

cqliu1 mentioned this pull request Oct 26, 2021

[Canvas] Adds support for palette in embeddable function #115278

Closed

9 tasks

cqliu1 added a commit that referenced this pull request Oct 26, 2021

[Canvas] Adds editor menu to Canvas (#113194)

0e10972

cqliu1 added a commit that referenced this pull request Oct 28, 2021

[Canvas] Adds editor menu to Canvas (#113194)

ea89036

cqliu1 added a commit that referenced this pull request Oct 28, 2021

[Canvas] Adds editor menu to Canvas (#113194)

e0e3ca3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Canvas] Adds editor menu to Canvas #113194

[Canvas] Adds editor menu to Canvas #113194

cqliu1 commented Sep 27, 2021 •

edited

Loading

ryankeairns commented Sep 30, 2021 •

edited

Loading

cqliu1 commented Oct 12, 2021

andreadelrio left a comment

andreadelrio Oct 20, 2021

cqliu1 Oct 20, 2021

crob611 left a comment

cqliu1 commented Oct 20, 2021 •

edited

Loading

andreadelrio Oct 20, 2021

cqliu1 Oct 20, 2021

andreadelrio left a comment

kibanamachine commented Oct 21, 2021

[Canvas] Adds editor menu to Canvas #113194

[Canvas] Adds editor menu to Canvas #113194

Conversation

cqliu1 commented Sep 27, 2021 • edited Loading

Summary

Outstanding tasks:

Checklist

Risk Matrix

For maintainers

ryankeairns commented Sep 30, 2021 • edited Loading

cqliu1 commented Oct 12, 2021

andreadelrio left a comment

Choose a reason for hiding this comment

andreadelrio Oct 20, 2021

Choose a reason for hiding this comment

cqliu1 Oct 20, 2021

Choose a reason for hiding this comment

crob611 left a comment

Choose a reason for hiding this comment

cqliu1 commented Oct 20, 2021 • edited Loading

andreadelrio Oct 20, 2021

Choose a reason for hiding this comment

cqliu1 Oct 20, 2021

Choose a reason for hiding this comment

andreadelrio left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 21, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

cqliu1 commented Sep 27, 2021 •

edited

Loading

ryankeairns commented Sep 30, 2021 •

edited

Loading

cqliu1 commented Oct 20, 2021 •

edited

Loading