patternfly / angular-patternfly

This repo contains instructions and the code for a set of Angular 1 components for the PatternFly project.
http://www.patternfly.org/angular-patternfly/
MIT License
122 stars 90 forks source link

feat(sass): Added Support for Less to Sass Conversion #692

Closed dtaylor113 closed 6 years ago

dtaylor113 commented 6 years ago

Description

From the updated README.md:

Less to Sass Conversion

During the build process Less files are converted to Sass files in /dist/sass. Then the Sass files are compiled into /dist/sass/angular-patternfly.css. If you would like to copy the Sass generated css into the main /dist/styles directory, execute:

grunt build --sass

This task will copy /dist/sass/angular-patternfly.css to /dist/styles/angular-patternfly.css. Then the build process will minimize the css in /dist/styles.

The Less to Sass Conversion step will be accomplished and managed as a part of any Pull Request which includes Less file changes. Although contributors may want to build and test their style changes with Sass before submitting a Pull Request, this step should always be tested and validated by reviewers before a style change is merged and released. If a contributor is having issues with Sass conversion that they cannot resolve, Pull Request reviewers will need to ensure that the Sass conversion step is successfully accomplished, tested, and included in the Pull Request before it is approved and merged.

For more detailed information, please read PatternFly Less to Sass Conversion

Note: When a Less file is added/deleted/renamed it needs to be updated in the main Less import file /styles/angular-patternfly.less and the main Sass import file /styles/_angular-patternfly.scss.

New dist/sass directory and files.

image

Fixes: #688

The sass-2-css file is almost identical to the less-2-css file; just diffs in line breaks. The minified versions are identical!

**Thanks to @TheRealJon for leading the way in the less-2-sass conversion process :-)

TheRealJon commented 6 years ago

Do we want a top level _angular-patternfly.scss file to import the rest of the partials?

dtaylor113 commented 6 years ago

Do we want a top level _angular-patternfly.scss file to import the rest of the partials?

Do you mean in addition to styles/build.scss?

dtaylor113 commented 6 years ago

Don't know why Travis is failing with:


Running "sass:patternfly" (sass) task
>> 
Error: File to import not found or unreadable: ../node_modules/patternfly/dist/sass/patternfly/_color-variables.scss.
>> 
       Parent style sheet: /home/travis/build/patternfly/angular-patternfly/styles/build.scss
>> 
        on line 1 of styles/build.scss
>> 
>> @import '../node_modules/patternfly/dist/sass/patternfly/_color-variables.scss
>> 
   ^

sass:patternfly runs fine locally. package-lock.json has patternfly at v3.31.2 which should have the ../node_modules/patternfly/dist/sass/patternfly/_color-variables.scss file.

TheRealJon commented 6 years ago

Do we want a top level _angular-patternfly.scss file to import the rest of the partials?

Do you mean in addition to styles/build.scss?

Yes. The build.scss file is for dev/test purposes to build css from the sass (since sass ignores partials by default, you can't compile from a file that starts with _). The _angular-patternfly.scss partial would ship in dist so that it could be consumed. That way consumers don't have to import individual partials. Then build.scss could also just @import 'angular-patternfly';.

dtaylor113 commented 6 years ago

The _angular-patternfly.scss partial would ship in dist so that it could be consumed. That way consumers don't have to import individual partials.

Ok, implemented this. May need a little help documenting how app devs are supposed to consume _angular-patternfly.scss. -thanks

TheRealJon commented 6 years ago

@dtaylor113 I just opened a PR on your fork to flatten the dist/sass directory. I also slightly adjusted the includePaths option in your grunt sass task so that the imports in _angular-patternfly.scss can be relative. This was mainly for consistency with the way patternfly core works.

As far as downstream consumption, in most use cases, people should be able to just add node_modules/angular-patternfly/dist/sass to their build tool's search path, then @import 'angular-patternfly'; in their sass.

dtaylor113 commented 6 years ago

Awesome! Thanks @TheRealJon !!!

dtaylor113 commented 6 years ago

Arrrrg...Travis still failing with:

Running "sass:patternfly" (sass) task
>> Error: File to import not found or unreadable: patternfly/color-variables.
>>        Parent style sheet: /home/travis/build/patternfly/angular-patternfly/styles/_angular-patternfly.scss
>>         on line 1 of styles/_angular-patternfly.scss
>> >> @import 'patternfly/color-variables';
>>    ^
TheRealJon commented 6 years ago

Yeah, I just noticed this. I don't know why. The build runs fine on my machine. I'll fiddle around with it and see if I can figure something out.

dtaylor113 commented 6 years ago

Thanks @TheRealJon the switch to "patternfly": "^3.31.0" did the trick!!!!

jeff-phillips-18 commented 6 years ago

I think this should be a feat. This is worthy of a point release.