Closed BenTangible closed 10 months ago
Thanks! I'll look into it. Good to know it's happening when a Sass variable is a URL, that helps me narrow it down.
You're right, I'm working on fixing a couple of issues that users found, and preparing to release the next version soon. I expected there will be some bumps with v4, since the entire codebase was reorganized - pretty much every file was moved or changed. I added many tests but it's never enough, haha.
@nicolas-jaussaud I wanted to ask your opinion on this, since I saw that you wrote a utility function in Tangible Blocks dealing with Sass variables.
Looking at the test page that reproduces this issue, I see the generated style shows a parser error.
<style data-name="tangible-template-inline-style">/**
* Error in "image-url-in-style-tab" - Post type: tangible_template, ID: 397
* parse error: failed at `$variable_name: https://loops-test-forum.instawp.xyz/image-url-in-style-tab/;` on line 1, at column 0
*/
</style>
I'm glad I added a feature to output any parser error as a CSS comment. Anyway, this line is invalid Sass syntax:
$variable_name: https://loops-test-forum.instawp.xyz/image-url-in-style-tab/;
The solution would be to either quote the string, or escape special characters.
I seem to remember there was a reason why values are passed unquoted, because automatically quoting a Sass (and JS) variable's value was problematic, as some value types like numbers need to be unquoted to be valid. We used to automatically quote all values, which made some use cases impossible - so we changed it. That was a breaking change which affected some users unexpectedly.
Maybe we need to document this as expected behavior, that Sass and JS variables are always unquoted - so that users are expected to quote values as needed.
Thanks for the clarification @eliot-akira, that makes sense, I just wasn't thinking that through. I've made a note to add it to the docs so I can handle that unless someone else gets to it first. Feel free to close this then.
Sure - I can see why this would be surprising though, and the fact that the parsing error is not visible on the page makes it hard to understand why the style is not working.
Maybe one way to solve that is to make it required to specify the value type.. The existing documentation seems pretty clear, but maybe it's not emphasized enough.
Use the
type
attribute to define the value type:number
,color
,list
,map
, andstring
.
string
wraps the value in "quotes" while all other types work the same as the default by passing an unquoted value.
(From https://docs.loopsandlogic.com/dynamic-tags/set-get#sass-variable)
Previously, string
was the default, but it turned out that the more common use case was to use the value directly in Sass. For example, passing a color (#fff
) or unit (2rem
). And users were surprised that it didn't work when the values got quoted. I think there was an even more compelling reason to change the default - but I also remember one user was quite upset by the breaking change. It reminds me of: Every change breaks someone's workflow.
@nicolas-jaussaud I wanted to ask your opinion on this, since I saw that you wrote a utility function in Tangible Blocks dealing with Sass variables.
@eliot-akira The main reason we needed utility functions in blocks was to be able to deal with quoted/unquoted sub values of sass maps and lists, but it still relies on template-system to handle it for regular values
I didn't find a way to automatically determine if a value must be quoted or not, so the way we deal with it for control values is to always explicitly define the needed type per control (and quotes will only be added when type is string
, regardless of if the quote is added by template-system or blocks)
For example, this is the definition for the dimensions
control type
This definition is then used during the render with the control value to generate a sass variable
I tried to add some examples in this comment regarding how it can be used, if needed I can move the associated functions and tests to template-system
I don't know if it's possible already or if it would make sense, but we could use those functions to convert L&L map and list to sass variables easily:
<Set scss=width type=map>
<Map name=value>
<Key value>10</Key>
<Key type>number</Key>
</Map>
<Map name=unit>
<Key value>px</Key>
<Key type>unit</Key>
</Map>
</Set>
Ooh, interesting, so that function is recursively un/quoting values to be able to pass lists and maps to a Sass variable. Sounds useful.
There was a recent discussion about how to pass values from a repeater field to Sass variables. The difficulty was that template features like Loop
and Field
cannot be used within Sass. (Ben, your answer to them was right that this limitation is due to HTML and Sass syntax being incompatible. I tried before to run both compilers on the Style field but it didn't work well.) The person did find a workaround solution, but it made me wonder if we can make it simpler somehow.
One idea I had was to provide a Sass function that returns a field value, including "simple" (like number, string) and "complex" (lists, maps). But now that I think about it, it might not be easy because we don't know what value types would be contained in them.
I remember seeing a function in the Sass compiler itself, that was added in a recent version, to convert PHP values to what Sass uses internally. Ah, here I made a comment about it: modules/sass/index.php.
ScssPhp 1.x no longer accepts unprocessed values. Convert known types manually and pass as Sass variable declarations.
The way Sass variables are passed currently: if it's a string (or other types already converted to string), it's passed as is. If it's an array, it's converted into a string with a questionable function (that I wrote) which applies json_encode()
and just replaces the first and last characters to make it a Sass list/map. From how it looks, I'm guessing it doesn't work for nested lists or maps. For other value types, it calls Tangible\ScssPhp\ValueConverter::fromPhp()
. ..But I see that function only handles simple values and throws an error for complex values.
I didn't find a way to automatically determine if a value must be quoted or not
I think there are cases where it's not possible to distinguish value types. For example, numbers, colors (#fff
, rgba(..)
), number with unit (2rem
) - they need to be unquoted. We might be able to parse the value and recognize such patterns, except the user may still want to use it as a string for some reason.
OK, so the conclusion is that the user must specify the value type of the variable, so we can reliably pass it to Sass or JS. That probably means if the user wants to pass a list or map, they need to somehow define the type of values it contains - just like your code example at the end there.
we needed utility functions in blocks to be able to deal with quoted/unquoted sub values of sass maps and lists
This does sound like something that the template system could need eventually, for some purposes. I'll keep that in mind, maybe we can move them over here at some point.
I'll close this issue for now, but we'll probably come back around to similar topics another time.
Hi @eliot-akira, I imagine you'll be having lots of fun solving 4.0.0 bugs, so to help balance the load I've just found a bug that existed prior to 4.0.0. You're welcome! Hahah 😜
It's a pretty weird one but is easy to replicate. I have this barebones template:
Template
Style
And it's displaying as expected on this page. However, if I change the variable to
Set sass=variable_name
, my style breaks. This only seems to happen when the contents of a sass variable are a URL, since if I use something likeField title
instead ofField url
, it works fine.