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

[feature][Canvas] Share Workpads in other Websites #46278

Merged
merged 27 commits into from
Oct 11, 2019

Conversation

clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented Sep 20, 2019

Reconstitutes #42545 for 7.5. Tracks the feature branch (feature/canvas-snapshots), and changes will be merged to the feature branch by separate PR for a cleaner commit history.

Summary

This is a pull request for embedding workpads externally, without an iframe. It consists of a custom runtime that can render the transient, data-complete state of a given workpad.

Aug-26-2019 18-39-55

demo

Coverage

Test coverage currently stands at 92%. Most "uncovered" lines and statements require either an upgrade to react and react-dom to 16.9, or are webpack-related build files. See "Gathering Coverage" below to see the report.

Screen Shot 2019-09-20 at 4 46 27 PM

Details about how to build and run this are in README.md.

Prerequisite

Before testing or running this PR locally, you must run node scripts/shareable_runtime from /canvas after yarn kbn bootstrap and before starting Kibana. It is only built automatically when Kibana is built for production.

Testing

Cloud Deployment

I've deployed this to a Cloud instance for testing by Elastic employees. If you'd like access, reach out to @clintandrewhall.

You can test this functionality in a number of ways. The easiest would be:

Download a ZIP from Canvas

  1. Load a workpad in Canvas.
  2. Click "Export" -> "Share on a website" -> "download a ZIP file"
  3. Extract and change to the extracted directory.
  4. Run python -m SimpleHTTPServer 9001
  5. Open a web browser to http://localhost:9001

Test the Runtime Directly from Webpack

  1. Load a workpad in Canvas.
  2. Click "Export" -> "Share on a website" -> "Download Workpad"
  3. Copy the workpad to canvas/shareable_runtime/test.
  4. Edit canvas/shareable_runtime/index.html to include your workpad.
  5. From /canvas, run node scripts/shareable_runtime --dev --run
  6. Open a web browser to http://localhost:8080

Run the Storybook

From /canvas: node scripts/storybook

Run the Jest Tests and Gather Coverage

From /canvas: node scripts/jest --path shareable_runtime --coverage.

Checklist

For maintainers

@clintandrewhall clintandrewhall changed the title Feature/canvas snapshots [feature][Canvas] Embed Workpad Snapshots in other Websites Sep 20, 2019
@clintandrewhall clintandrewhall added enhancement New value added to drive a business result impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:x-large Extra Large Level of Effort release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.5.0 v8.0.0 WIP Work in progress labels Sep 20, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas

@elasticmachine
Copy link
Contributor

💔 Build Failed

@clintandrewhall clintandrewhall force-pushed the feature/canvas-snapshots branch 2 times, most recently from 6f11a80 to 8a4902b Compare September 20, 2019 21:27
Copy link
Contributor Author

@clintandrewhall clintandrewhall left a comment

Choose a reason for hiding this comment

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

Some comments for @elastic/kibana-operations

handler(request, handler) {
const workpad = request.payload;

const archive = archiver('zip');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

operations: using archiver to create a zip file.

execa.sync(process.execPath, ['legacy/plugins/canvas/scripts/external_runtime'], {
cwd: resolve(__dirname, '..'),
stdio: ['ignore', 'inherit', 'inherit'],
buffer: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

operations: added the external runtime build step to the task list.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

* Add localization + tweak naming

* Fix duplicate key

* Update storyshots
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

* [perf] Fix perf issue with unsupported renderers

* Fixing snapshots
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@crob611 crob611 left a comment

Choose a reason for hiding this comment

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

Looked through the code and played around with it a bit. It looks great to me. Very nice work on this.

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

Works well, just a couple of very small items:

  • For me, the script run at /canvas initially failed as I needed to install webpack first. It would be helpful to note that as I'm sure others will encounter this is future testing/local work.
  • The buttons in the flyouts have periods in the text label

Screenshot 2019-10-10 10 45 54

@clintandrewhall
Copy link
Contributor Author

clintandrewhall commented Oct 10, 2019

@ryankeairns It's in the PR under Prerequisites, and it's in the README... is there somewhere else I should put it?

@ryankeairns
Copy link
Contributor

@clintandrewhall my point was that the command/script fails if you don't have webpack installed. I am proposing adding a note to check that you have webpack installed first. Unfortunately, the error was not clear in this regard, it was a lucky guess to try installing webpack :)

@clintandrewhall
Copy link
Contributor Author

@ryankeairns Webpack shouldn't need to be installed... it should be using the version bundled with x-pack... let me review the script.

@ryankeairns ryankeairns self-requested a review October 10, 2019 17:17
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

Fix for button text looks good and we've sorted out my issues with running the script.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@clintandrewhall clintandrewhall requested a review from a team October 10, 2019 18:07
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM

* Branding Changes

* Update snapshots
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@clintandrewhall clintandrewhall merged commit 27cbdf5 into master Oct 11, 2019
@clintandrewhall clintandrewhall deleted the feature/canvas-snapshots branch October 11, 2019 17:15
clintandrewhall added a commit to clintandrewhall/kibana that referenced this pull request Oct 11, 2019
* [Canvas] Embedding Workpads in other Websites (elastic#42545)

* Testing for Workpad Snapshots

* Rename Snapshots to Shareables; update documentation. (elastic#46513)

* [canvas][shareables] Add Localization + Tweaks (elastic#46632)

* Add localization + tweak naming

* Fix duplicate key

* Update storyshots

* [shareables] Unsupported Renderer Warning (elastic#46862)

* [shareables] Unsupported Renderer Warning

* Update snapshots; add test

* Addressing Feedback

* [canvas][shareables] Simplify and complete testing (elastic#47328)

* Simplify

* Updates

* Finishing up

* A few tweaks

* Fix eslint errors; how would those happen??

* Fix CI build of runtime; assorted visual tweaks

* Update x-pack/legacy/plugins/canvas/shareable_runtime/test/index.ts

Co-Authored-By: Spencer <[email protected]>

* Addressing feedback

* Remove null-loader from root package

* re-add null-loader until mitigation is found

* [perf] Fix unsupported renderers performance issue (elastic#47769)

* [perf] Fix perf issue with unsupported renderers

* Fixing snapshots

* Addressing review feedback (elastic#47775)

* Addressing feedback

* Addressing feedback (elastic#47883)

* Branding Changes (elastic#47913)

* Branding Changes

* Update snapshots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:x-large Extra Large Level of Effort release_note:enhancement review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants