buddypress / next-template-packs

is this the next BuddyPress template pack?
35 stars 9 forks source link

Build Tools: Update NPM modules #164

Closed ntwb closed 7 years ago

ntwb commented 7 years ago

Once the tests finish this should be good to merge, all the packages are now up to date

ntwb commented 7 years ago

Once merged, you'll need to run npm install to update these packages 😄

hnla commented 7 years ago

One question, why the changes to the compiled css file for hex colours?

The keyword one is odd as that's a mixin and programmatically generated, not sure why that's happening, can hazard a guess though. The others are also scss generated using darken()/lighten() % values.

ntwb commented 7 years ago

As the calculated colour changes were minor I presumed it was due to updates in the node-sass and libsass libraries and I didn't go looking for specifics for this case.

The keyword gray has me a bit confused though :( The only mentions of gray are in these two files:

ag gray
bp-templates/bp-nouveau/css/buddypress-rtl.css
3878:   color: gray;

bp-templates/bp-nouveau/css/buddypress.css
3882:   color: gray;

In the SCSS files there 62 instances in 20 files of grey but none of gray

Possibly related to this issue https://github.com/sass/libsass/issues/2140

Does any of that make sense to you @hnla ?

hnla commented 7 years ago

@ntwb this definitely appears to be a bug ( it also may be me being daft in how I'm setting the default color value in the function which I can test later) in how libsass is compiling values as these seem to be all instances where a sass function is in use to find a value to output, I'll check that libsass issue later.

I spent ages confused trying to find use of the keyword grey/gray in .sass files until realising there weren't any :)

In the SCSS files there 62 instances in 20 files of grey but none of gray

Yep they're all variable names, probably my bad in using the English spelling of 'grey' ( how can there be two :( )

Big thanks for highlighting this it passed me by completely.

I'm guessing that previously the keyword use in the compiled files wasn't being caught by the linter?.

Having a different value generated for the percentage darken/lighten functions is a bit odd but as you say it's very minor variance unlikely to cause obvious visual change.

I'll merge this PR now and delve deeper later.

hnla commented 7 years ago

typical! Now when I test the mixin and state a colour, then remove again to replicate issue it works properly to output a hex value.

ok the more I look at my mixin function that tries to be smart and set backgrounds and text color based on each other values or defaults I think my logic is flawed and that this isn't a bug in libsas as such more a case that I need to take care defining vars. Keyword does appear under some circumstances when it shouldn't, but haven't figured exact steps yet.

ntwb commented 7 years ago

Ha, for what it's worth libsass supports both spellings of grey and gray so those of us who do use the Queen's English shouldn't run into these types of issues with this colour, should I say 😝

Thanks for merging and the follow up explanations 👍