Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Added media query matches for more browsers #39

Merged
merged 4 commits into from Aug 20, 2012

Conversation

Projects
None yet
2 participants
Contributor

feelepxyz commented Aug 10, 2012

Hey, this project was just what I was looking for. Awesome stuff.

Added relevant browser matches for the retina media query.
The min--moz query is used by FF up to version 15. FF >= 16 has the new pimping dppx unit.
http://www.w3.org/TR/css3-values/#dppx
http://www.w3.org/blog/CSS/2012/06/14/unprefix-webkit-device-pixel-ratio/
This is supposedly the way other browsers will implement it too.

Had to escape the media matches, the less parser was chocking on the dppx unit.

Also made the path codes a bit more readable.

Cheers

Contributor

caseyohara commented Aug 13, 2012

@harrison Your method of using .replace() for @2x paths instead of a million splits is great. Much, much cleaner and easier to read. I'd like to use it in the JS code as well.

Unfortunately, we've had trouble with multiple media queries in the past. The less compiler doesn't seem to like comma-separated media queries. See issues #13 and #18. I'll try it out and circle back to this.

Contributor

feelepxyz commented Aug 13, 2012

Cool! Yeah go for it.

Had the same issues. Seems one liner is the way to go. Less freaks out if you have line breaks to separate media queries.
Also, 'all and' media type before each media query is the default when not setting any type.

Found a weird quirk in less just now. Seems you have to pass in a single quoted string to ~"@{path}" - otherwise it comes out as a double double quoted string! Might be worth putting this in the docs?

@caseyohara caseyohara pushed a commit that referenced this pull request Aug 18, 2012

Casey O'Hara Use better @2x path method from based on pull #39 c1f2d04
Contributor

caseyohara commented Aug 18, 2012

@harrison I just tested your changes to retina.less. The Less compiler compiles them no problem (good tip on the one-liner, btw), but background-image properties nested inside the media query in the output don't get the '@2x' injected in the filename.

I just pushed up a new unit test for comparing actual output from the Less compiler to a known desired output. It would be awesome if you'd rebase my new changes into your branch and update test/fixtures/desired_output.css to what your updates above should render out and then push that up here so I can take a look.

(I added less to the dev dependencies, so be sure to npm install after you pull down my latest commit. Then just run npm test to run the suite.)

Thanks man.

feelepxyz added some commits Aug 20, 2012

@feelepxyz feelepxyz Merge branch 'master' of git://github.com/imulus/retinajs 796a656
@feelepxyz feelepxyz Stop double quoting path string in javascript eval, add spec
@path was being double quoted despite already being a string.
'string' became "'string'"
"string" became ""string"" which is a invalid javascript string.

Explicitly double quote the background image url.
Seems to be the convention generally.
0a44042
Contributor

feelepxyz commented Aug 20, 2012

Fixed the replace not working properly. The path string was being quoted twice by the variable inclusion.
Seems it works fine to do @{variable} directly in the javascript eval. No need to wrap it in a string.
Added a test for double quoted urls as a regression test.

@caseyohara caseyohara merged commit 0a44042 into strues:master Aug 20, 2012

Contributor

caseyohara commented Aug 20, 2012

@harrison Works great. Thanks.

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