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

@franciscolourenco franciscolourenco commented Oct 1, 2010

No description provided.

@dylan

This comment has been minimized.

Copy link

@dylan dylan commented Jan 19, 2011

I am having this problem as well.

@mishunov

This comment has been minimized.

Copy link
Contributor

@mishunov 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

This comment has been minimized.

Copy link

@vincentbernat vincentbernat commented May 23, 2011

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

This comment has been minimized.

Copy link
Contributor

@maranomynet maranomynet commented Aug 12, 2011

+1 for this issue!

@davetayls

This comment has been minimized.

Copy link

@davetayls davetayls commented Aug 28, 2011

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

@PaulAdamDavis

This comment has been minimized.

Copy link

@PaulAdamDavis PaulAdamDavis commented Sep 5, 2011

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

@dfreerksen

This comment has been minimized.

Copy link

@dfreerksen dfreerksen commented Sep 7, 2011

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

This comment has been minimized.

Copy link

@jayf 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

This comment has been minimized.

Copy link
Contributor

@ttfkam 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

This comment has been minimized.

Copy link
Contributor

@maranomynet maranomynet commented Oct 6, 2011

@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

This comment has been minimized.

Copy link

@loa loa commented Nov 21, 2011

+1 Need same thing as @vincentbernat

@matthew-dean

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Nov 22, 2011

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

This comment has been minimized.

Copy link

@razialx 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

This comment has been minimized.

Copy link

@razialx 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

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Dec 5, 2011

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

@matthew-dean

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Dec 5, 2011

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

@razialx

This comment has been minimized.

Copy link

@razialx 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

This comment has been minimized.

Copy link

@razialx 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

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Dec 6, 2011

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

This comment has been minimized.

Copy link

@razialx 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

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Dec 6, 2011

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

This comment has been minimized.

Copy link

@razialx 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

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Dec 6, 2011

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

This comment has been minimized.

Copy link

@razialx 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

This comment has been minimized.

Copy link

@razialx 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

This comment has been minimized.

Copy link

@razialx 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

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Dec 6, 2011

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

@matthew-dean

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Dec 6, 2011

@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

This comment has been minimized.

Copy link
Member

@cloudhead cloudhead commented Dec 17, 2011

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

This comment has been minimized.

Copy link

@tomdcc 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

This comment has been minimized.

Copy link
Member

@lukeapage lukeapage commented Jul 17, 2012

@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

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Jul 18, 2012

@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

This comment has been minimized.

Copy link
Member

@lukeapage lukeapage commented Jul 18, 2012

@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

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Jul 18, 2012

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

This comment has been minimized.

Copy link
Member

@lukeapage lukeapage commented Jul 18, 2012

@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

This comment has been minimized.

Copy link
Member

@lukeapage lukeapage commented Jul 18, 2012

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

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Jul 18, 2012

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

This comment has been minimized.

Copy link
Member

@matthew-dean matthew-dean commented Jul 18, 2012

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.

@calvintennant

This comment has been minimized.

Copy link

@calvintennant calvintennant commented Nov 28, 2012

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

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

Possibly related to #915

@brgrz

This comment has been minimized.

Copy link

@brgrz 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

This comment has been minimized.

Copy link
Member

@lukeapage lukeapage commented Feb 11, 2013

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

This comment has been minimized.

Copy link

@brgrz 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

This comment has been minimized.

Copy link

@brgrz 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

This comment has been minimized.

Copy link

@kamranayub kamranayub commented Feb 11, 2013

This doesn't work already @mhrovatic?

Because we're doing this in our projects already.

@brgrz

This comment has been minimized.

Copy link

@brgrz brgrz commented Feb 11, 2013

What exactly are you doing? please post a sample code

@kamranayub

This comment has been minimized.

Copy link

@kamranayub kamranayub commented Feb 11, 2013

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

This comment has been minimized.

Copy link

@brgrz 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

This comment has been minimized.

Copy link
Member

@lukeapage lukeapage commented Feb 12, 2013

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

This comment has been minimized.

Copy link
Member

@lukeapage lukeapage commented Feb 16, 2013

this is implemented in master

@lukeapage lukeapage closed this Feb 16, 2013
@nicooprat nicooprat mentioned this issue Aug 5, 2013
@adam-lynch

This comment has been minimized.

Copy link

@adam-lynch adam-lynch commented Dec 15, 2013

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

This comment has been minimized.

Copy link

@Soviut Soviut commented Dec 15, 2013

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

@adam-lynch

This comment has been minimized.

Copy link

@adam-lynch adam-lynch commented Dec 15, 2013

@Soviut 1.5.0

@Soviut

This comment has been minimized.

Copy link

@Soviut 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

This comment has been minimized.

Copy link

@adam-lynch adam-lynch commented Dec 15, 2013

@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

This comment has been minimized.

Copy link
Member

@seven-phases-max seven-phases-max commented Dec 15, 2013

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
You can’t perform that action at this time.