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.

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

Named colours don't work as map keys #652

Closed
mikeebee opened this issue Nov 17, 2014 · 8 comments · Fixed by #695
Closed

Named colours don't work as map keys #652

mikeebee opened this issue Nov 17, 2014 · 8 comments · Fixed by #695

Comments

@mikeebee
Copy link

Hi all,

I'm trying to implement a colour palette function seen in this article here.

http://erskinedesign.com/blog/friendlier-colour-names-sass-maps/

This is the resulting code

// config
$_color-base-grey: rgb(229,231,234);
$palettes: (
    purple: (
        base:   rgb(42,40,80),
        light:  rgb(51,46,140),
        dark:   rgb(40,38,65)
    ),
    grey: (
        base:  $_color-base-grey,
        light: lighten($_color-base-grey, 10%),
        dark: darken($_color-base-grey, 10%)
    )
);

// Palette function,
@function palette($palette, $tone: 'base') {
    @return map-get(map-get($palettes, $palette), $tone);
}


// in module styles
a {
    color: palette(purple);
    &:hover {
        color: palette(purple, light);
    }
}

All signs seem to point to this working but I get this error.

Warning: /Users/xxx/xxx//styles.scss:35: argument `$map` of `map-get($map, $key)` must be a map

I'm using Grunt Sass version - 0.16.1

Any ideas?

Many thanks

@am11
Copy link
Contributor

am11 commented Nov 17, 2014

Confirmed! I get the following (JSON) error from libsass, with the new API implemented in node-sass (unreleased):

{
        "status": 1,
        "path": "c:/temp/foo3.scss",
        "line": 18,
        "column": 13,
        "message": "argument `$map` of `map-get($map, $key)` must be a map\nBacktrace:\n\tc:/temp/foo3.scss:18, in function `map-get`\n\tc:/temp/foo3.scss:18, in function `palette`\n\tc:/temp/foo3.scss:24"
}

@mgreter, separately; in message of JSON, the filename and line number shouldn't be repeated. :)

@xzyfer
Copy link
Contributor

xzyfer commented Nov 17, 2014

I dug into this and it appeared to be because map-get($palettes, $palette) is returning null. It seems as though map-get incorrectly distinguishes between quoted and unquoted string keys.

@mgntrn
Copy link

mgntrn commented Dec 1, 2014

Any solution yet?

@xzyfer xzyfer modified the milestone: 3.0.3 Dec 2, 2014
@xzyfer
Copy link
Contributor

xzyfer commented Dec 3, 2014

I've added this to the 3.0.3 milestone it should make it into the next release.

@xzyfer
Copy link
Contributor

xzyfer commented Dec 3, 2014

It appears my initial thoughts were wrong. The issue is because the map key is a color name. Libsass has some issues when it comes to parsing strings that are color names i.e. #558

Reduced case: http://sassmeister.com/gist/d2be1def3619bd6c3a54

@Snugug
Copy link

Snugug commented Dec 5, 2014

As a heads up, it looks like the user may be running in to the issue related to sass/sass#363 in that colors as map keys aren't strings, but colors.

@xzyfer
Copy link
Contributor

xzyfer commented Dec 7, 2014

I believe this is somewhat true. I think the map key is being treated as a colour rather than a string, although I'm not sure why this is a problem.

It's worth noting the reduced test case works fine on Ruby sass.

@xzyfer
Copy link
Contributor

xzyfer commented Dec 9, 2014

The issue is that we're treat purple in the map key as a string and purple in the function call as a color. When we call map-get we're asking for the value associated with the color purple, but it's stored under the string purple. Theses two things are not equivalent - http://sassmeister.com/gist/77ae29d46ebecd0823ed

I've got a fix I'll ship in a couple hours.

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

Successfully merging a pull request may close this issue.

5 participants