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

Adds SCSS support for plugins #19643

Merged
merged 17 commits into from Jun 21, 2018
Merged

Conversation

tylersmalley
Copy link
Contributor

@tylersmalley tylersmalley commented Jun 4, 2018

Closes: #15274

This adds styleSheetPath as an option for UIExport app

styleSheetPath: Absolute path to the stylesheet within the plugins public directory. When provided, this will be used to when the plugin is active. When an SCSS file is provide, it will be transpiled into CSS. If a CSS file is provide, it will be loaded when the plugin is active.

Example:

index.js:

return new kibana.Plugin({
  require: ['kibana', 'elasticsearch'],
  uiExports: {
    app: {
      title: 'Timelion',
      order: -1000,
      description: 'Time series expressions for everything',
      icon: 'plugins/timelion/icon.svg',
      main: 'plugins/timelion/app',
      styleSheetPath: `${__dirname}/public/timelion.scss`,
    },
    ...

The same is for external plugins, However styleSheetToCompile needs to be specified in .kibana-plugin-helpers.json and is relative to the root of the plugin.

Example:

index.js:

return new kibana.Plugin({
  require: ['elasticsearch'],
  name: 'my-test-plugin',
  uiExports: {
    app: {
      title: 'My Test Plugin',
      description: 'An awesome Kibana plugin',
      main: 'plugins/my-test-plugin/app',
      styleSheetPath: `${__dirname}/public/main.scss`,
    },
  },
  ...

.kibana-plugin-helpers.json:

{
  "styleSheetToCompile": "public/main.scss"
}

Testing

For core plugins, I would suggest modifying one to use SCSS.

Adding styleSheetPath to the Timelion UiExports app configuration. Kibana will need to correctly pick these settings up.

return new kibana.Plugin({
  require: ['kibana', 'elasticsearch'],
  uiExports: {
    app: {
      title: 'Timelion',
      order: -1000,
      description: 'Time series expressions for everything',
      icon: 'plugins/timelion/icon.svg',
      main: 'plugins/timelion/app',
      styleSheetPath: `${__dirname}/public/timelion.scss`,
    },
    ...

Next, create src/core_plugins/timelion/public/timelion.scss and add a style. For demonstration, I am using the following:

body {
  background-color: blue;
}

When in development mode, a message will be logged when we transpile the SCSS into CSS.

managr   info   [00:36:01.280] [scss] Compiled CSS: /Users/tyler/elastic/kibana/src/core_plugins/timelion/public/timelion.scss

Errors are expected to be logged. Removing one of the brackets in the example will cause this.

managr   error  [00:36:42.559] [scss] Compiling CSS failed: /Users/tyler/elastic/kibana/src/core_plugins/timelion/public/timelion.scss
managr   error  [00:36:42.560] [scss] Error: Invalid CSS after "...d-color: green;": expected "}", was ""
    at Object.module.exports.renderSync (/Users/tyler/elastic/kibana/node_modules/node-sass/lib/index.js:439:16)
    at SassBuilder.build (/Users/tyler/elastic/kibana/src/cli/cluster/sass_builder.js:70:33)
    at SassBuilder.buildIfInPath (/Users/tyler/elastic/kibana/src/cli/cluster/sass_builder.js:58:18)
    at FSWatcher.watcher.on (/Users/tyler/elastic/kibana/src/cli/cluster/cluster_manager.js:171:36)
    at emitTwo (events.js:126:13)
    at FSWatcher.emit (events.js:214:7)
    at FSWatcher.<anonymous> (/Users/tyler/elastic/kibana/node_modules/chokidar/index.js:192:38)
    at FSWatcher._emit (/Users/tyler/elastic/kibana/node_modules/chokidar/index.js:233:5)
    at FSWatcher.<anonymous> (/Users/tyler/elastic/kibana/node_modules/chokidar/lib/fsevents-handler.js:204:14)
    at addOrChange (/Users/tyler/elastic/kibana/node_modules/chokidar/lib/fsevents-handler.js:210:7)
    at FSWatcher.<anonymous> (/Users/tyler/elastic/kibana/node_modules/chokidar/lib/fsevents-handler.js:236:16)
    at filteredListener (/Users/tyler/elastic/kibana/node_modules/chokidar/lib/fsevents-handler.js:58:7)
    at /Users/tyler/elastic/kibana/node_modules/chokidar/lib/fsevents-handler.js:83:11
    at Array.forEach (<anonymous>)
    at FSEvents.<anonymous> (/Users/tyler/elastic/kibana/node_modules/chokidar/lib/fsevents-handler.js:82:34)
    at emitThree (events.js:136:13)
    at FSEvents.emit (events.js:217:7)
    at Immediate._onImmediate (/Users/tyler/elastic/kibana/node_modules/fsevents/fsevents.js:47:11)
    at runCallback (timers.js:794:20)
    at tryOnImmediate (timers.js:752:5)
    at processImmediate [as _immediateCallback] (timers.js:729:5)

During development, the CSS file will be generated with source-maps when an SCSS file is modified and served when the plugin is active. This CSS file is in addition to the plugins CSS file created by the optimizer, however, that will be phased out as we migrate to SCSS.

When building a plugin outside of Kibana, you need to also set styleSheetToCompile in the .kibana-plugin-helpers.json.

For testing of external plugins, node scripts/generate_plugin.js will setup SCSS support based on your response to the question Should SCSS be used?.

@tylersmalley tylersmalley added review Team:Operations Team label for Operations Team v7.0.0 v6.4.0 labels Jun 4, 2018
@@ -48,6 +48,12 @@ export class PluginSpec {
* directory for this plugin. The final directory must have the name "public",
* though it can be located somewhere besides the root of the plugin. Set
* this to false to disable exposure of a public directory
* @param {String|False} [opts.styleSheet] path to the stylesheet within
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thoughts on the naming of these two options? Would styleSheetPath and scssPath be better?

@tylersmalley
Copy link
Contributor Author

Still working on figuring out how we should trigger transpiring the SCSS when running a build for both Kibana and individual plugins. During the plugin build process, we currently don't have a way of accessing the Kibana Plugin options. We might need to resort to calling the index file with babel register.

@tylersmalley
Copy link
Contributor Author

tylersmalley commented Jun 4, 2018

07:06:06    │ proc  [ftr]        └- ✖ fail: "dashboard mode Dashboard View Mode Dashboard viewer shows only the dashboard app link"
07:06:06    │ proc  [ftr]        │        tryForTime timeout: Error: Login is not completed yet

@tylersmalley
Copy link
Contributor Author

Actually, we shouldn't need to transpile SCSS during the build if we required that the CSS be committed. Thoughts?

@elastic elastic deleted a comment from elasticmachine Jun 6, 2018
@elastic elastic deleted a comment from elasticmachine Jun 6, 2018
@elastic elastic deleted a comment from elasticmachine Jun 7, 2018
@elastic elastic deleted a comment from elasticmachine Jun 7, 2018
@tylersmalley tylersmalley force-pushed the sass-15274 branch 4 times, most recently from 0097594 to 44e815b Compare June 13, 2018 16:50
@elastic elastic deleted a comment from elasticmachine Jun 13, 2018
@elastic elastic deleted a comment from cjcenizal Jun 13, 2018
@elastic elastic deleted a comment from elasticmachine Jun 13, 2018
@elastic elastic deleted a comment from elasticmachine Jun 13, 2018
@elastic elastic deleted a comment from elasticmachine Jun 13, 2018
@elastic elastic deleted a comment from elasticmachine Jun 13, 2018
@elastic elastic deleted a comment from elasticmachine Jun 13, 2018
Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

🌟 I tested this locally and it works as described. I also read through the code and it seems clear enough. Great job @tylersmalley!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@tylersmalley
Copy link
Contributor Author

Spencer notes:

  • Absolute path for both options - fail is not in public or absolute (ui_apps.js)
  • Include styleSheetToCompile for external plugins as well
  • Build when in source and not in dev-mode.

@elasticmachine

This comment has been minimized.

@tylersmalley
Copy link
Contributor Author

retest

Chrome 65.0.3325 (Linux 0.0.0) geohash_layer GeohashGridLayer Shaded Circle Markers FAILED

@elasticmachine

This comment has been minimized.

@tylersmalley
Copy link
Contributor Author

retest

fail: "homepage app sample data should install sample data set"

@elasticmachine
Copy link
Contributor

💔 Build Failed

@tylersmalley
Copy link
Contributor Author

retest

this is absurd ... fail: "Monitoring app Elasticsearch shard legends Shard Allocation Per Node "before all" hook"

@elasticmachine
Copy link
Contributor

💔 Build Failed

@tylersmalley
Copy link
Contributor Author

retest

fail: "visualize app visualize app vega chart should have some initial vega spec text"

@tylersmalley
Copy link
Contributor Author

^ looks like this is also happening on master. Pinged the viz team.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

main: 'plugins/<%= kebabCase(name) %>/app'
main: 'plugins/<%= kebabCase(name) %>/app',
<% if (generateScss) { %>
styleSheetPath: `${require('path').resolve(__dirname, 'public/app.scss')}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: unnecessary template literal

@@ -168,6 +168,36 @@ export class PluginSpec {
return this._uiExports;
}

getExportAppSpecs() {
Copy link
Contributor

Choose a reason for hiding this comment

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

These methods and their tests should be removed.

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.

One nit and some simple removal, but once those are taken care of LGTM!

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@tylersmalley tylersmalley merged commit 699fb25 into elastic:master Jun 21, 2018
tylersmalley added a commit to tylersmalley/kibana that referenced this pull request Jun 21, 2018
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@snide
Copy link
Contributor

snide commented Jun 21, 2018

Hooray! Thanks all.

@uboness
Copy link

uboness commented Jun 22, 2018

❤️!!!

tylersmalley added a commit that referenced this pull request Jun 22, 2018
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley
Copy link
Contributor Author

6.4/6.x: a1b5a56

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Operations Team label for Operations Team v6.4.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants