Skip to content
This repository has been archived by the owner on Aug 30, 2018. It is now read-only.

Make the at-query more intuitive for min-max calls #359

Closed
wants to merge 1 commit into from

Conversation

jtgrenz
Copy link
Contributor

@jtgrenz jtgrenz commented Mar 16, 2015

I modified the at-query mixin to make calling values of in-between a little more intuitive.
Currently if you want to define a media query between two ranges, say small and medium, you'd write:
@include at-query(null, $small, $medium) { ... }

I don't like passing in null values and sass is smart enough to figure out whats what. So with this change,
@include at-query(null, $small, $medium) { ... } and @include at-query( $small, $medium) { ... } are identical.

if you were used to adding null to do an in-between, it works, but if you wanted a short hand, that works too.

@carolineschnapp

@carolineschnapp
Copy link
Contributor

This adds some welcomed smarts to a function that isn't as intuitive as it could be.

I can't comment on the actual code edit, I am not an experienced Sassy coder, but I like that we can call the function with a min and max values. 👍

@carolineschnapp
Copy link
Contributor

@cshold what do you think?

@cshold
Copy link
Contributor

cshold commented Mar 16, 2015

Looks like a smart way to call the function, and I like that it still works with the long syntax. Let me test it out a bit tomorrow and get back to you.

@cshold
Copy link
Contributor

cshold commented Mar 16, 2015

@Shopify/fed Thoughts on this approach to make a more intuitive at-query mixin?

@snookca
Copy link

snookca commented Mar 17, 2015

I'm a bigger fan of using named arguments.

@mpiotrowicz
Copy link
Contributor

I as well, but this is an improvement I think

@AWaselnuk
Copy link

The type-of check seems brittle to me. What if someone does at-query('100px')?

@jtgrenz
Copy link
Contributor Author

jtgrenz commented Mar 17, 2015

@AWaselnuk at-query('100px') wouldn't work in either case. Existing code using strings for the values should work the same since they would be defined with long form of the function or they would be passing in $min or $max to the constraint. Anyone trying to use the short form with a string would trip an error though, but if you forget to add null right now, you also get an error.

There is an article on casting string values to numbers but its definitely overkill.

@AWaselnuk
Copy link

Agreed that casting would be a bad idea.
As long as an error is thrown for invalid argument type then it should be fine.

@cshold
Copy link
Contributor

cshold commented Mar 18, 2015

Opened #367 to include link to docs and slight syntax update.

@cshold cshold closed this Mar 18, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants