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

Variables not being replaced in media queries declaration #122

Closed
franciscolourenco opened this issue Oct 1, 2010 · 64 comments
Closed

Variables not being replaced in media queries declaration #122

franciscolourenco opened this issue Oct 1, 2010 · 64 comments

Comments

@franciscolourenco
Copy link

No description provided.

@dylan
Copy link

dylan commented Jan 19, 2011

I am having this problem as well.

@mishunov
Copy link
Contributor

mishunov commented Apr 2, 2011

This is still the issue to me. Has there been any action on this?
Both variables and @import rules within @media queries are not converted and are just output as they are not converted at all. Let's asume I have core.less file that has some variables and CSS in it. Then I have another .less file, that dispatches different stylesheets based on @media queries. So to serve core.less to a device I do the following:

@media only screen and (min-device-width : 769px) {
    @import "core";    
}

In resulting CSS it turns into

@media only screen and (min-device-width : 769px) {

}

If I want to work around this and instead of importing core.less will import compiled core.css file like:

@media only screen and (min-device-width : 769px) {
    @import "core.css";    
}    

it compiles without any change to:

@media only screen and (min-device-width : 769px) {
  @import "core.css";
}

@vincentbernat
Copy link

I also would like to get variables replaced inside media query. Something like this :

@media screen and (max-width: @page-width) {
 ...
}

The CSS generated from this still contains @page-width instead of its value.

@maranomynet
Copy link
Contributor

+1 for this issue!

@davetayls
Copy link

Is this being looked at? I would like to use it in the same way as @vincentbernat

@PaulAdamDavis
Copy link

+1 on this too. I want to use it the same way as @mishunov

@dfreerksen
Copy link

The only solutions I have found to this is to do something like this:

  1. Create a file named 768.less
  2. In this file create a parametric mixin with no parameters such as:
    .max-width-768() {
    body {
    background-color: #FF00FF;
    }
    }
  3. In your styles.less (or whatever you are calling it)
    @import "768.less";
    @media (max-width: 768px) {
    .max-width-768();
    }

The other option I have found is:

  1. Create a file named 768.less
  2. In your styles.less (or whatever you are calling it)
    @import "768.less";
  3. In your 768.less file you do something like:
    @media (max-width: 768px) {
    body {
    background-color: #FF00FF;
    }
    }

@jayf
Copy link

jayf commented Oct 5, 2011

Likewise, I'd would like to use it in the same way as @vincentbernat, e.g.,

@media screen and (min-width: @bigass) {

I see in parser.js where it grabs the media rule, but I'm still too new to the code to see the exact changes needed...

@ttfkam
Copy link
Contributor

ttfkam commented Oct 5, 2011

With regard to everything I've seen from the draft CSS 3 specs, @import statements are not allowed within @media directives. This, however, is allowed by the spec:

 @import "foo.css" screen and (min-width: 768px);

I haven't tried this with Less, but if there is going to be compatibility with CSS, perhaps this should be the target instead of nesting import statements. http://www.w3.org/TR/css3-mediaqueries/

@maranomynet
Copy link
Contributor

@importing LESS files should be possible inside @media blocks IMO, even though CSS @import (linking) is invalid.

However, the original issue being reported is that LESS currently doesn't allow variables or other such niceties following @media and @import

@loa
Copy link

loa commented Nov 21, 2011

+1 Need same thing as @vincentbernat

@matthew-dean
Copy link
Member

Ah, looks like I stumbled upon the same bug. My statement:

@import "desktop/nav";

This imports perfectly well OUTSIDE a media query, but vanishes with no error inside a media query.

@razialx
Copy link

razialx commented Dec 1, 2011

I have a patch for this I am getting ready to submit. Haven't looked at the importing, just the variable replacement.

@razialx
Copy link

razialx commented Dec 1, 2011

So, I messed up my pull request apparently. The patch for this is in this request: #481

Should have been in its own. It is the second commit. Can find the original commit on my branch here:
razialx@1f97c27

razialx added a commit to razialx/less.js that referenced this issue Dec 2, 2011
@matthew-dean
Copy link
Member

Would be awesome to get this in the next release. And then it would be awesomer to have that release be today.

@matthew-dean
Copy link
Member

@razialx - Is there a JS file that includes the parser changes?

@razialx
Copy link

razialx commented Dec 5, 2011

Sure. Give me a moment to boot my laptop up

Sent from my iPhone

On Dec 5, 2011, at 5:30 PM, matthewdl
reply@reply.github.com
wrote:

@razialx - Is there JS file that includes the parser changes?


Reply to this email directly or view it on GitHub:
#122 (comment)

@razialx
Copy link

razialx commented Dec 5, 2011

@MatthewDL Here you are: https://github.com/downloads/razialx/less.js/less-mediaquery-1.1.5.zip

The un-minified version has been tested. I just ran it through an online uglifyjs tool to minify it, so I haven'ted tested that at all.

@matthew-dean
Copy link
Member

Thanks! I've already incorporated it into the latest version of
Crunch. Http://crunchapp.net/ :-)

On 2011-12-05, at 2:51 PM, razialx
reply@reply.github.com
wrote:

@MatthewDL Here you are: https://github.com/downloads/razialx/less.js/less-mediaquery-1.1.5.zip

The un-minified version has been tested. I just ran it through an online uglifyjs tool to minify it, so I haven'ted tested that at all.


Reply to this email directly or view it on GitHub:
#122 (comment)

@razialx
Copy link

razialx commented Dec 6, 2011

@MatthewDL Oh, no, thank you. I have Crunch on my machine right now. This is a great example of when open source just works, heh.

@matthew-dean
Copy link
Member

Yep, creating or even contributing to open source (other than filing issues / asking questions) is something new for me, and it's been fun to see who gets involved. I'm sure if you were willing to figure out @imports inside @media querys, that would make bunches of people happy too! ;-)

@razialx
Copy link

razialx commented Dec 6, 2011

Working on it. I have imports on the query line working in that it
imports but then it fails to parse. Example:
Style.less:
@media screen and (@import "Import.less"){...}
Import.less:
min-width: 1000px

Fails because the less parser tries to parse Import.less as a complete
legal less file. It isn't, only being a fragment. I would propose a
new token perhaps? @Fragment "filename" but I would need to really
think about the impact and honestly it isn't my call at all. I would
need to discuss it with the less devs.

Sent from my iPhone

On Dec 6, 2011, at 9:52 AM, matthewdl
reply@reply.github.com
wrote:

Yep, creating or even contributing to open source (other than filing issues / asking questions) is something new for me, and it's been fun to see who gets involved. I'm sure if you were willing to figure out @imports inside @media querys, that would make bunches of people happy too! ;-)


Reply to this email directly or view it on GitHub:
#122 (comment)

@matthew-dean
Copy link
Member

Hmmm..... I disagree with your example. I think it's perfectly valid for the parser to reject import.less, because otherwise I (or someone else) can't do something like build syntax checking into that particular file. So, while it make sense to, say, start a curly brace in style.less, and finish it in import.less (different from your example, but would still follow the model of a fragment) it's much harder to form an object tree.

Instead, what would seem the better case would be to import the file, and then call a mixin which outputs the min-width declaration. That way, all my LESS files follow the spec, and everyone is happy.

I think if you put in the @Fragment piece or importing of fragments, it probably won't be merged, but I couldn't say for sure. Maybe you can get @cloudhead 's thoughts here or on Twitter.

In the meantime, if you have a build that otherwise fully supports @import, I'd love to see it. :-)

@razialx
Copy link

razialx commented Dec 6, 2011

@MatthewDL

Could you post an example of what you would like to see with the import
statement then?

Are you looking for:
style.less:
@media screen and (min-width:1024px){
@import "import.less";
.page { ... }
.aside { ... }
}

import.less:
.header { ... }

? That is something I can definitely look at doing. The current problem is,
when a directive is found it scans until it gets an open curly brace, then
reads a 'block' and then a closing curly brace.
I changed it to tokenize the contents after the @media directive with a few
modifications elsewhere. To get the above example working I would need to
change the 'block' reading code to watch for imports, which should be easy
enough.

Tim

On Tue, Dec 6, 2011 at 10:22 AM, matthewdl <
reply@reply.github.com

wrote:

Hmmm..... I disagree with your example. I think it's perfectly valid for
the parser to reject import.less, because otherwise I (or someone else)
can't do something like build syntax checking into that particular file.
So, while it make sense to, say, start a curly brace in style.less, and
finish it in import.less (different from your example, but would still
follow the model of a fragment) it's much harder to form an object tree.

Instead, what would seem the better case would be to import the file, and
then call a mixin which outputs the min-width declaration. That way, all
my LESS files follow the spec, and everyone is happy.

I think if you put in the @Fragment piece or importing of fragments, it
probably won't be merged, but I couldn't say for sure. Maybe you can get
@cloudhead 's thoughts here or on Twitter.

In the meantime, if you have a build that otherwise fully supports
@import, I'd love to see it. :-)


Reply to this email directly or view it on GitHub:
#122 (comment)

Tim Reynolds

@razialx
Copy link

razialx commented Dec 6, 2011

@MatthewDL Ok, I have imports working inside of a media query. Was pretty simple, though I have no idea if there are further reaching implications of to this. Will make sure unit tests pass and commit it up shortly.

@razialx
Copy link

razialx commented Dec 6, 2011

@MatthewDL Here is an updated zip: https://github.com/downloads/razialx/less.js/less-mediaquery-import-1.1.5.zip

It was a one-line change.

In the code imports are only evaluated if the context is the root of the document. I added code to flag a media query block as a 'root.' This could of course cause problems, so use with caution. Unit tests do pass, however.

Tim

@matthew-dean
Copy link
Member

Awesome. I'll test it out today with my projects.

@matthew-dean
Copy link
Member

@razialx - Btw, to answer how I'd like to be using @import, there's an example of almost the exact scenario that I attempted yesterday. This blog post popped up today: http://alwaystwisted.com/post.php?s=2011-12-01-a-less-approach-to-mobile-first-css-supporting-older-ie-browsers

The first example (which wouldn't work except for your hotfix) would be the cleanest way to set up imports.

@cloudhead
Copy link
Member

Just pushed a fix to this, it's in the master branch. Let me know if it solves the problems.

https://github.com/cloudhead/less.js/blob/master/test/less/media.less#L27

@tomdcc
Copy link

tomdcc commented Jun 15, 2012

We find that (with 1.3.0) direct variable substitution is working, but expressions based on them is not.

E.g.
@desktop-site-width: 980px;
@media all and (max-width: @desktop-site-width - 1px) {
// stuff
}

fails with a syntax error, but

@desktop-site-width-minus-1: @desktop-site-width - 1px;
@media all and (max-width: @desktop-site-width-minus-1) {
// stuff
}

works as expected

@lukeapage
Copy link
Member

@tomdcc the issue is with conflicts over whether less should calculate or take the value literally, e.g. "3/4" as a ratio.

I think the issue should be considered closed what do you think?

@MatthewDL is there anything stopping this being closed in your opinion?

@matthew-dean
Copy link
Member

@agatronic - I'm not sure if the problem is the 3/4 ratio issue. If you look at @tomdcc's example, it evaluates correctly when declared as the variable value, but not within the media declaration.

In contrast, this works:

border: 1px solid @baseColor - #FFF

So, expressions are allowed everywhere else in LESS, so I would say that this behavior in the media query is inconsistent and therefore still qualifies as a not-quite-fixed bug.

@lukeapage
Copy link
Member

@MatthewDL yes expressions are allowed everywhere - but there is a bug about how that causes problems for border-radius... thats still unresolved with people having to change valid css to less to get it to compile.

This is an area of css currently expanding so I think it makes sense to keep it as it is - which isn't a bug, but it very specifically allows only variables or an expression.

Still, lets keep this open as a low priority feature request.

@matthew-dean
Copy link
Member

Right, you mean the slash "/" operator? That's going to take some serious thought. We may have to want to discuss with @cloudhead at some point how to reduce / eliminate parser ambiguities. As in, make the parser less greedy if it's possible a value might not be a math expression (so as to not ruin any existing / imported CSS), and then have a syntax for declaring that it IS indeed an expression. (Maybe just enclosing in parentheses.)

Regardless, I don't understand why that bug would be causing this error, since the minus sign isn't valid in a media query, is it? It just seems like LESS isn't evaluating expressions in this instance, but you may be right.

@lukeapage
Copy link
Member

@MatthewDL you're right and I'm just guessing the intention - some things in code look like bugs, others look intentional - but even if it is intentional, it doesn't mean it can't change one day :)

@lukeapage
Copy link
Member

and feel free to change the priority label, I'm just trying to make sensible guesses on what the community wants vs what makes sense to add

@matthew-dean
Copy link
Member

Let's mark as a bug but I agree it's a low-priority bug, since there's a very simple workaround, and it's not a bug that will break imported CSS.

@matthew-dean
Copy link
Member

And also, there's a pile of issues related to evaluation of expressions, so let's mark as duplicates and maybe there can be an effort to address expression syntax bugs across the board.

@calvinte
Copy link

Getting Cannot call method 'charAt' of null when I do:

@media only screen and (max-width: @width/2) {}

Possibly related to #915

@brgrz
Copy link

brgrz commented Feb 11, 2013

Was this issue closed or solved another way? For instance, I'd like to do similar to @mishunov, this:

@media only screen and (min-width: 1140px) {
@import "1136.gs.less";
}
@media only screen and (min-width: 768px) and (max-width: 1139px) {
@import "978.gs.less";
}

If not possible, what's the alternative?

@lukeapage
Copy link
Member

It will be fixed in 1.4.0 - it is probably fixed already in 1.4.0

@mhrovatic Your issue is imports in media statements - that is different.. ?

@brgrz
Copy link

brgrz commented Feb 11, 2013

My issue is imports inside the media blocks, the same as mishunov and matthewdl. I guess that's a different issue than the variables replacements? but it seems related to this issue as other's talked about it here..or is there another issue opened on that?

@brgrz
Copy link

brgrz commented Feb 11, 2013

Maybe it will be easier to understand what I'd like with this workaround that I use now:

`<link href="@Url.Content("~/public/less/styles.978.gs.less")" rel="stylesheet" type="text/css" media="only screen and (min-width: 768px) and (max-width: 1139px)"/>

`

I have different grids that are computed off different values for variables and I use those grids for different resolutions. The grids are based on Semantic.gs where I have a couple of mixins like .column(number-of-columns).

@kamranayub
Copy link

This doesn't work already @mhrovatic?

Because we're doing this in our projects already.

@brgrz
Copy link

brgrz commented Feb 11, 2013

What exactly are you doing? please post a sample code

@kamranayub
Copy link

We're doing more than you're asking ;)

@media (max-width: @breakpointLarge) {
  @import "modules/_header";
  @import "modules/_footer";
  ...
}

@media only screen and (min-width: 700px), only screen and (max-width: 800px) {
 ...
}

No issues here using Mindscape.

@brgrz
Copy link

brgrz commented Feb 11, 2013

Is the "modules/_header" an uncompiled LESS file that includes variable declarations?

For instance can you do this
'@import "978grid_vars"; // defines and initializes some variables
@import "grid"; // uses those variables'

This doesn't seem to work for me with dotless, says variable is undefined.

@lukeapage
Copy link
Member

if it is dotless you are using, raise the issue with https://github.com/dotless/dotless - it is a port, it doesn't directly use less.js

@lukeapage
Copy link
Member

this is implemented in master

@adam-lynch
Copy link

This is a frustrating one :/. Really limits what I want to do with variables but I need to compile on the fly in the browser too.

Wanted to do stuff like:

@media(min-width: @feedMaxWidth + @gutter_base){
    //...
}

Or even:

@extraRoomAtThisWidth: @feedMaxWidth + @gutter_base;
@media(min-width: @extraRoomAtThisWidth){
    //...
}

@Soviut
Copy link

Soviut commented Dec 15, 2013

@adam-lynch Both examples should work fine by now. What version of LESS are you using?

@adam-lynch
Copy link

@Soviut 1.5.0

@Soviut
Copy link

Soviut commented Dec 15, 2013

If you have the strictMath option turned on, you'll need to wrap your math operations in parenthesis (@feedMaxWidth + @gutter_base)

@adam-lynch
Copy link

@Soviut, parenthesis sorted it.

Strangely though, I don't have strictMath set to true. Even when I set it to false I was still getting it.

My config:

{
        env: "production", // or "production"
        async: false,       // load imports async
        fileAsync: false,   // load imports async when in a page under
        // a file protocol
        poll: 1000,         // when in watch mode, time in ms between polls
        functions: {},      // user functions, keyed by name
        dumpLineNumbers: "comments", // or "mediaQuery" or "all"
        relativeUrls: false,
        strictMath: false// whether to adjust url's to be relative
        // if false, url's are already relative to the
        // entry less file
        //rootpath: ":/a.com/"// a path to add on to the start of every url
        //resource
    }

@seven-phases-max
Copy link
Member

The trick is that math operations are not evaluated without parenthesis inside media feature expressions regardless of strictMath settings. E.g.:

@feedMaxWidth: 11;
@gutter_base:  22;

@extraRoomAtThisWidth: @feedMaxWidth + @gutter_base;
@media (min-width: @extraRoomAtThisWidth) {
    - {value: @extraRoomAtThisWidth}
}

@media (min-width: @feedMaxWidth + @gutter_base) {
    - {value: @feedMaxWidth + @gutter_base}
}

@media (min-width: (@feedMaxWidth + @gutter_base)) {
    - {value: (@feedMaxWidth + @gutter_base)}
}

result (strictMath: on):

@media (min-width: 11 + 22) {
  - {
    value: 11 + 22;
  }
}
@media (min-width: 11 + 22) {
  - {
    value: 11 + 22;
  }
}
@media (min-width: 33) {
  - {
    value: 33;
  }
}

result (strictMath: off):

@media (min-width: 11 + 22) {
  - {
    value: 33;
  }
}
@media (min-width: 11 + 22) {
  - {
    value: 33;
  }
}
@media (min-width: 33) {
  - {
    value: 33;
  }
}

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

No branches or pull requests