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

Remove libsass as a dependency #9803

Merged
merged 4 commits into from
Jan 10, 2017
Merged

Remove libsass as a dependency #9803

merged 4 commits into from
Jan 10, 2017

Conversation

tylersmalley
Copy link
Contributor

@tylersmalley tylersmalley commented Jan 9, 2017

Fixes: #9764

libsass is platform specific, therefore we can not ship it as a dependency. Instead, we will commit the compiled CSS for the UI Framework to the repository. This is updated when running npm run uiFramework:start which also starts the docs site.

@tylersmalley tylersmalley force-pushed the 9764-no-libsass branch 3 times, most recently from 2fb223f to c2b3a6a Compare January 9, 2017 22:03
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

I wasn't able to build

Running "run:optimizeBuild" (run) task
Warning: non-zero exit code 1 Use --force to continue.

edit: fixed now

libsass is platform specific, therefore we can not ship it as a dependency. Instead, we will commit the compiled CSS for the UI Framework to the repository. This is updated when running `npm run uiFramework:start` which also starts the docs site.

Signed-off-by: Tyler Smalley <[email protected]>
@tylersmalley
Copy link
Contributor Author

jenkins, test it

@jbudz
Copy link
Member

jbudz commented Jan 9, 2017

I'm not very familiar running the ui framework, did hot reloading work before? I have to do full page refreshes now for styles.

@jbudz
Copy link
Member

jbudz commented Jan 9, 2017

Question above, but otherwise this LGTM. Rebasing on master should fix tests.

@tylersmalley
Copy link
Contributor Author

@jbudz, pushed a fix for the hot reloading and merged upstream. Hopefully that resolves the tests.

@tylersmalley
Copy link
Contributor Author

@jbudz @cjcenizal - we're all green.

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.

Looks good overall. I had one question.

Also, it looks like the UI Framework CSS isn't being run through PostCSS + Autoprefixer any more.

Ideally, we should update the UI Framework task to run the compiled CSS through PostCSS so we can view the UI Framework documentation in various browsers.

If that's too much trouble at the moment, we can also update base_optimizer.js to run all CSS through PostCSS:

const makeStyleLoader = preprocessor => {
  let loaders = [
    loaderWithSourceMaps('css-loader'),
    {
      name: 'postcss-loader',
      query: {
        config: require.resolve('./postcss.config')
      }
    },
  ];

  if (preprocessor) {
    loaders = [
      ...loaders,
      loaderWithSourceMaps(preprocessor)
    ];
  }

  return ExtractTextPlugin.extract(makeLoaderString(loaders));
};


grunt.log.error(message);

reject();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to return reject() here?

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.

LGTM! Great work @tylersmalley!

@tylersmalley
Copy link
Contributor Author

Thanks for the review @cjcenizal. @jbudz, let me know if the additional changes look good to you.

@tylersmalley tylersmalley merged commit e6669ee into master Jan 10, 2017
@tylersmalley tylersmalley deleted the 9764-no-libsass branch January 10, 2017 04:27
tylersmalley added a commit to tylersmalley/kibana that referenced this pull request Jan 10, 2017
Remove libsass as a dependency

libsass is platform specific, therefore we can not ship it as a dependency. Instead, we will commit the compiled CSS for the UI Framework to the repository. This is updated when running `npm run uiFramework:start` which also starts the docs site.

Signed-off-by: Tyler Smalley <[email protected]>
tylersmalley added a commit to tylersmalley/kibana that referenced this pull request Jan 10, 2017
Remove libsass as a dependency

libsass is platform specific, therefore we can not ship it as a dependency. Instead, we will commit the compiled CSS for the UI Framework to the repository. This is updated when running `npm run uiFramework:start` which also starts the docs site.

Signed-off-by: Tyler Smalley <[email protected]>
tylersmalley added a commit that referenced this pull request Jan 10, 2017
Remove libsass as a dependency

libsass is platform specific, therefore we can not ship it as a dependency. Instead, we will commit the compiled CSS for the UI Framework to the repository. This is updated when running `npm run uiFramework:start` which also starts the docs site.

Signed-off-by: Tyler Smalley <[email protected]>
tylersmalley added a commit that referenced this pull request Jan 10, 2017
Remove libsass as a dependency

libsass is platform specific, therefore we can not ship it as a dependency. Instead, we will commit the compiled CSS for the UI Framework to the repository. This is updated when running `npm run uiFramework:start` which also starts the docs site.

Signed-off-by: Tyler Smalley <[email protected]>
weltenwort added a commit to cjcenizal/kibana that referenced this pull request Jan 12, 2017
weltenwort pushed a commit to weltenwort/kibana that referenced this pull request Jan 12, 2017
* Move ContextLoadingButtons out of header into Context view.
* Add kuiButton--iconText modifier to ContextLoadingButtons.
* Use style guide color for highlighted Context Log row.
* Add Bar component. Use Bar to surface entry-pagination controls in Context Log. Refactor ContextSizePicker directive to surface a single input without surrounding text.
* Use Panel component to present loading feedback and table within Context app.
* Remove fading effect from truncated doc table rows, because it doesn't work with highlighted backgrounds. It also looks weird.
* Move bar component to match elastic#9803
* Update ui framework css bundle
* Move getter/setter into the size-picker
* Remove superfluous loading-button styling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants