Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

Build Tools: Update NPM modules #164

Merged
merged 8 commits into from
Jun 4, 2017
Merged

Build Tools: Update NPM modules #164

merged 8 commits into from
Jun 4, 2017

Conversation

ntwb
Copy link
Member

@ntwb ntwb commented Jun 3, 2017

No description provided.

@ntwb
Copy link
Member Author

ntwb commented Jun 3, 2017

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

@ntwb
Copy link
Member Author

ntwb commented Jun 3, 2017

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

@hnla
Copy link
Collaborator

hnla commented Jun 3, 2017

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
Copy link
Member Author

ntwb commented Jun 3, 2017

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 sass/libsass#2140

Does any of that make sense to you @hnla ?

@hnla
Copy link
Collaborator

hnla commented Jun 4, 2017

@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 hnla merged commit 3083251 into master Jun 4, 2017
@hnla hnla deleted the update-npm branch June 4, 2017 10:35
@hnla
Copy link
Collaborator

hnla commented Jun 4, 2017

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
Copy link
Member Author

ntwb commented Jun 4, 2017

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 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants