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

Improve sass strings #26

Closed
wants to merge 4 commits into from
Closed

Conversation

vieron
Copy link

@vieron vieron commented Mar 15, 2016

From JSON strings, output quoted Sass strings except for valid CSS colors and number/floats with valid CSS units.

See the example below, JSON/JS at left, Sass output at right:

"hex-color": "#c33",                            //→ #c33
"hsl-color": "hsl(100, 100%, 100%)",            //→ hsl(100, 100%, 100%)
"rgba-color": "rgba(0, 0, 0, 1)",               //→ rgba(0, 0, 0, 1)
"rgb-color": "rgb(10, 0, 0)",                   //→ rgb(10, 0, 0)
"css-color": "blue",                            //→ blue
"px-unit": "10px",                              //→ 10px
"em-unit": "2.3em",                             //→ 2.3em
"string": "Lorem ipsum, (\"foo\", bar)"         //→ 'Lorem ipsum, ("foo", bar)'
"string2": "Lorem ipsum, ('foo', bar)"          //→ "Lorem ipsum, ('foo', bar)"
"quoted-number": "5"                            //→ "5"

Note that quoted numbers or floats are not unquoted in the output, because you can use numbers in JSON, but they could be unquoted just adding an ? to the regex.

For the regex matching the CSS units I've taken all the units defined in mdn and in the libsass source code.

This PR is built on the top of #25 because it includes tests for JS imports too. #25 should be merged first.
It should fix problems mentioned in #5 and #16.

cc/ @pmowrer

@@ -52,7 +57,7 @@ function parseValue(value) {
} else if (_.isPlainObject(value)) {
return parseMap(value);
} else {
return value;
return parseEndValue(value);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseEndValue is the worst name in the world. I should rename it to parseValue or maybe you have a better proposal :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a parseValue function, thats why I chose parseEndValue... 😴 💤

@vieron
Copy link
Author

vieron commented Mar 15, 2016

I'll fixup the last commit with the previous, rebase with #25 and push force once you are OK with the PR, if you are :)

@pmowrer
Copy link
Owner

pmowrer commented Mar 16, 2016

So this seems to be a bit of a contentious issue. Would you mind explaining in some detail how this handles the edge cases in #5 and #16? Perhaps we can get @safareli's input on this since he unearthed many of the problems.

@safareli
Copy link

what if you want to store font family value in json?

conf.json

{ "font": "'Roboto', sans-serif" }

style.scss

@import conf.json;
body { font-family: $font; }

Desired output of that is this:

body { font-family: 'Roboto', sans-serif; }

Does your changes produce output like that ?

As I looked through your changes, I think your output would be this:

body { font-family: "'Roboto', sans-serif"; }

@vieron
Copy link
Author

vieron commented Mar 17, 2016

@safareli I think that's a different problem, since 'Roboto', sans-serif is a list in Sass, that maps to an Array in JSON, what you want is a list with some values unquoted, like here

With the conf.json in your previous comment, you need to use Sass unquote function, example here.

@vieron
Copy link
Author

vieron commented Mar 17, 2016

@pmowrer I've updated the PR description above with more details.

It solves #5 because parser do not break when the input is "Ashness Pier, Derwentwater, Lake District, Cumbria, UK", since input value is a string and doesn't match a CSS color or a number with CSS units, the output remains quoted.

And solves #16 for the same reason before. "screen and (min-width: 560px)" remains quoted in Sass so you can do that: http://www.sassmeister.com/gist/14f141bd37ee357a7405

It also solves the problem with CSS colores mentioned in the PRs.

@safareli
Copy link

json arrays do not map directly to sass lists as in sass list may have commas or spaces as separator.


if i have {"font": [ "Roboto", "serif" ]} you would get $font: "Roboto", "serif"; and it will make user of that variable to know which element it should unquote.


maybe we have conf.json like this

{ 
  "bg": "red", 
  "spacing":"2rem",
  "color" : "blue",
  "size" : "3rem",
  "content": "hi"
}

and here we are using that variables from conf.json in style.scss

@import conf.json;

.btn {
  backgorund: $bg;
  padding: $spacing;
  color: $color;
  size: $size;
}
.btn:after{
  content: $content;
}

and we get desired output

.btn {
  backgorund: red;
  padding: 2rem;
  color: blue;
  size: 3rem;
}
.btn:after{
  content: "hi";
}

but what if conf.json was changed to this:

{ 
  "bg": "url('/img.png')", 
  "spacing":"calc(2rem + 1vw)",
  "color" : "lighten(currentColor, 10%)",
  "size" : "var(--button-size)",
  "content": "'(url: ' attr(src) ')'"
}

now output will be this

.btn {
  backgorund: "url('/img.png')";
  padding: "calc(2rem + 1vw)";
  color: "lighten(currentColor, 10%)";
  size: "var(--button-size)";
}
.btn:after{
  content: "'(url: ' attr(src) ')'";
}

to get desired output we need to wrap all variables in unquote which i think is bad.

@pmowrer
Copy link
Owner

pmowrer commented May 17, 2016

Closed due to inactivity

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

Successfully merging this pull request may close these issues.

3 participants