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

Return EUI CSS to Shareable Runtime #72990

Merged
merged 3 commits into from
Jul 24, 2020

Conversation

clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented Jul 22, 2020

Summary

Fixes #72967

As titled. EUI added tree shaking to the package, which allowed the Shareable Runtime build to remove it entirely.

To Test w/o Kibana:

  1. canvas: node scripts/shareable_runtime --dev --run
  2. Open http://localhost:8080

Screen Shot 2020-07-22 at 7 50 06 PM

@clintandrewhall clintandrewhall added review regression Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:small Small Level of Effort v8.0.0 release_note:skip Skip the PR/issue when compiling release notes impact:critical This issue should be addressed immediately due to a critical level of impact on the product. v7.9.0 labels Jul 22, 2020
@clintandrewhall clintandrewhall requested a review from a team as a code owner July 22, 2020 23:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas (Team:Canvas)

@crob611
Copy link
Contributor

crob611 commented Jul 23, 2020

@clintandrewhall I'm seeing this work properly when running the --dev way, but building the runtime and downloading the zip, it still appears to be missing styles. Does it work correctly for you doing it this way?

@clintandrewhall
Copy link
Contributor Author

@corey I can test it again to be sure.

Did you remember to build the runtime again?

https://github.com/elastic/kibana/blob/master/x-pack/plugins/canvas/shareable_runtime/README.md#prerequisite

@crob611
Copy link
Contributor

crob611 commented Jul 23, 2020

@clintandrewhall Yep, did that multiple times to make sure I wasn't crazy. Maybe I still am though

@clintandrewhall clintandrewhall changed the title Add Canvas Styles to Shareable Runtime Return EUI CSS to Shareable Runtime Jul 23, 2020
@clintandrewhall
Copy link
Contributor Author

@elasticmachine merge upstream

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.

That fixes it for me. Good find 👍

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@clintandrewhall clintandrewhall merged commit 120ce71 into elastic:master Jul 24, 2020
@clintandrewhall clintandrewhall deleted the shareable_styles branch July 24, 2020 19:39
clintandrewhall added a commit to clintandrewhall/kibana that referenced this pull request Jul 24, 2020
* Add Canvas Styles

* Tree-shaking in EUI omits CSS in the runtime

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
clintandrewhall added a commit to clintandrewhall/kibana that referenced this pull request Jul 24, 2020
* Add Canvas Styles

* Tree-shaking in EUI omits CSS in the runtime

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
clintandrewhall added a commit that referenced this pull request Jul 25, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
clintandrewhall added a commit that referenced this pull request Jul 25, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 27, 2020
* master: (111 commits)
  Remove flaky note from gauge tests (elastic#73240)
  Convert functional vega tests to ts and unskip tests (elastic#72238)
  [Graph] Unskip graph tests (elastic#72291)
  Add default Elasticsearch credentials to docs (elastic#72617)
  [APM] Read body from indicesStats in upload-telemetry-data (elastic#72732)
  The directory in the command was missing the /generated directory and would cause all definitions to be regenerated in the wrong place. (elastic#72766)
  [KP] use new ES client in SO service (elastic#72289)
  [Security Solution][Exceptions] Prevents value list entries from co-existing with non value list entries (elastic#72995)
  Return EUI CSS to Shareable Runtime (elastic#72990)
  Removed useless karma test (elastic#73190)
  [INGEST_MANAGER] Make package config name blank for endpoint on Package Config create (elastic#73082)
  [Ingest Manager] Support DEGRADED state in fleet agent event (elastic#73104)
  [Security Solution][Detections] Change detections breadcrumb title (elastic#73059)
  [ML] Fixing unnecessary deleting job polling (elastic#73087)
  [ML] Fixing recognizer wizard create job button (elastic#73025)
  [Composable template] Preview composite template (elastic#72598)
  [Uptime] Use manual intervals for ping histogram (elastic#72928)
  [Security Solution][Endpoint] Task/policy save modal text change, remove duplicate policy details text (elastic#73130)
  [Maps] fix tile layer attibution text and attribution link validation errors (elastic#73160)
  skip ingest pipeline api tests
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:small Small Level of Effort regression release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Canvas] Shareables looks to be missing some styles
4 participants