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.

Sign up for GitHub

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

Jump to bottom

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 •
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
Member

@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

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
Contributor Author

jenkins, test it

@jbudz
Copy link
Member

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
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.

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

tasks/ui_framework.js

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?

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
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
@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

@cjcenizal cjcenizal cjcenizal approved these changes

@jbudz jbudz jbudz approved these changes

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