Skip to content

Remove libsass as a dependency#9803

Merged
tylersmalley merged 4 commits intomasterfrom
9764-no-libsass
Jan 10, 2017
Merged

Remove libsass as a dependency#9803
tylersmalley merged 4 commits intomasterfrom
9764-no-libsass

Conversation

@tylersmalley
Copy link
Copy Markdown
Member

@tylersmalley tylersmalley commented Jan 9, 2017 •
edited
Loading

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
jbudz
jbudz reviewed Jan 9, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@jbudz jbudz left a comment •
edited
Loading

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

@tylersmalley
Remove libsass as a dependency
efb2a5c 
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 <tyler.smalley@elastic.co>
@tylersmalley
Copy link
Copy Markdown
Member Author

tylersmalley commented Jan 9, 2017

jenkins, test it

@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Jan 9, 2017 •
edited
Loading

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
Copy Markdown
Contributor

jbudz commented Jan 9, 2017

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

@tylersmalley
Copy link
Copy Markdown
Member Author

tylersmalley commented Jan 9, 2017

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

@tylersmalley
Copy link
Copy Markdown
Member Author

tylersmalley commented Jan 10, 2017

@jbudz @cjcenizal - we're all green.

cjcenizal
cjcenizal suggested changes Jan 10, 2017
View reviewed changes
Copy link
Copy Markdown
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));
};

tasks/ui_framework.js Outdated

grunt.log.error(message);

reject();
Copy link
Copy Markdown
Contributor

@cjcenizal cjcenizal Jan 10, 2017

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?

@tylersmalley
Pass CSS through PostCSS + AutoPrefixer
0fe4f0a 
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
cjcenizal
cjcenizal approved these changes Jan 10, 2017
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Member Author

tylersmalley commented Jan 10, 2017

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
@tylersmalley
Remove libsass as a dependency (elastic#9803)
f03d8ed 
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 <tyler.smalley@elastic.co>
@tylersmalley tylersmalley mentioned this pull request Jan 10, 2017
Merged
tylersmalley added a commit to tylersmalley/kibana that referenced this pull request Jan 10, 2017
@tylersmalley
Remove libsass as a dependency (elastic#9803)
3b0722c 
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 <tyler.smalley@elastic.co>
@tylersmalley tylersmalley mentioned this pull request Jan 10, 2017
Merged
tylersmalley added a commit that referenced this pull request Jan 10, 2017
@tylersmalley
Remove libsass as a dependency (#9803) (#9811)
b190867 
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 <tyler.smalley@elastic.co>
tylersmalley added a commit that referenced this pull request Jan 10, 2017
@tylersmalley
Remove libsass as a dependency (#9803) (#9812)
95ac4c1 
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 <tyler.smalley@elastic.co>
weltenwort added a commit to cjcenizal/kibana that referenced this pull request Jan 12, 2017
@weltenwort
Move bar component to match elastic#9803
1b40e3c
weltenwort pushed a commit to weltenwort/kibana that referenced this pull request Jan 12, 2017
@cjcenizal @weltenwort
Improve Context Log highlighting and UI. (#1)
cb4d95f 
* 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
@BigFunger BigFunger mentioned this pull request Jan 17, 2017
Closed
This was referenced Nov 28, 2017
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jbudz jbudz jbudz approved these changes

+1 more reviewer

@cjcenizal cjcenizal cjcenizal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

review v5.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tylersmalley @jbudz @cjcenizal