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

Breakpoint Version 2 #35

Closed
robwierzbowski opened this issue Feb 11, 2013 · 44 comments
Closed

Breakpoint Version 2 #35

robwierzbowski opened this issue Feb 11, 2013 · 44 comments
Milestone

Comments

@robwierzbowski
Copy link
Contributor

After some discussion and percolation I think we have the major features and workflow of Breakpoint 2.0 worked out.

Breakpoint 2

Breakpoint 2 will add full media query handling, including only, not, and or operators, along with making working with breakpoint variables simpler and more robust.

Example breakpoint variables:

// A normal example:
$mq: screen 'and' min-width 30em;
// An intense example:
$mq: only screen 'and' width 20em 35em 'and' monochrome 
        'and' max-height 20em, print;

Feature list types

  • All original breakpoint combinations:
    • 500px // default-feature-value
    • min-height 500px // feature value
    • width 20em 30em // feature min-value max-value
  • Media type, with optional not or only operators.
    • screen // media-type
    • 'only' screen // operator media-type
  • No-query is now treated like any other breakpoint feature list:
    • no-query '.nq' // no-query-flag selector

And and Or operators

In V2, and operators will use the keyword 'and', while or operators will use a comma.

Prefixed features

To be decided. Maybe keyword handling, maybe array handling, although both have unique complexity issues.

Combination of multiple breakpoint variables

The following should be possible:

$medium: 20em;
$max-medium: max-width 35em 
$medium-scoped: $medium 'and' $max-medium;
$low-color: monotone;

@include breakpoint($medium-scoped 'and' $low-color)  {
  ...
}

Variable arguments

By switching to keywords and internal handling instead of separate arguments, we can use variable arguments in the mixin and streamline ux.

Context

Now that we are allowing or media queries, context will have to return a list, sorted from smallest to largest value.

@include breakpoint(500px 200px 'and' resolution 2dppx) {
  // bp_context(min-width) == (200px, 500px).
}

Warnings

We should throw warnings on some possible user errors:

  • More than one media type in an and query
  • More than one instance of a feature in an and query

Thoughts, problems, suggestions? @Snugug, want to make a canonical v2 branch and divide some of these features to work on?

@Snugug
Copy link
Member

Snugug commented Feb 11, 2013

Don't like the syntax that you've got under context, should simply be @include breakpoint(500px, 200px, resolution 2dppx){}, and $no-query would be an or statement, not an and, so that example should be @include breakpoint($medium-scoped, $noquery-9){}. Otherwise, looks about right. I've got some ideas about prefixing, and specifically around resolution that may get added, but we'll see how those pan out.

I've already started on a 2.x.x branch, with @ChinggizKhan's full refactor merged in as a starting point. I've got work time today and tomorrow to work on it, so that's what I'm doing.

@robwierzbowski
Copy link
Contributor Author

Re: syntax: of course, my mistake. I had copied/pasted that from another example.

@Snugug
Copy link
Member

Snugug commented Feb 11, 2013

No problem!

@robwierzbowski
Copy link
Contributor Author

No-query could be an 'or' query, and maybe that's even recommendable, so updated the combination example to beter show what's going to need to be handled.

@robwierzbowski
Copy link
Contributor Author

Re-read mdn docs on not and only — there's no reason to add only to a feature list, only to the media type.

Context will have to do something with not operators. Since it's edge case-ey, and you can usually re-write with a positive query, ignoring might be the best choice.

@Snugug
Copy link
Member

Snugug commented Feb 11, 2013

Agreed, same with not; it can only be applied to a query as a whole, not to an individual feature, so we'll only accept them as part of the media string

On Monday, February 11, 2013 at 4:52 PM, Rob Wierzbowski wrote:

Re-read mdn docs on not and only — there's no reason to add only to a feature list, only to the media type.
Context will have to do something with not operators. Since it's edge case-ey, and you can usually re-write with a positive query, ignoring might be the best choice.


Reply to this email directly or view it on GitHub (https://github.com/canarymason/breakpoint/issues/35#issuecomment-13405295).

@robwierzbowski
Copy link
Contributor Author

Not can be used on a feature. From MDN:

The not keyword negates the result of the query; "all and (not color)" is true for monochrome devices regardless of media type, for example.

@Snugug
Copy link
Member

Snugug commented Feb 11, 2013

Interesting, that's not how the spec reads. http://www.w3.org/TR/css3-mediaqueries/
The only place in the spec where not/only are mentioned is in regards to negating the whole query

On Monday, February 11, 2013 at 4:55 PM, Rob Wierzbowski wrote:

Not can be used on a feature. From MDN (https://developer.mozilla.org/en-US/docs/CSS/Media_queries):

The not keyword negates the result of the query; "all and (not color)" is true for monochrome devices regardless of media type, for example.


Reply to this email directly or view it on GitHub (https://github.com/canarymason/breakpoint/issues/35#issuecomment-13405459).

@robwierzbowski
Copy link
Contributor Author

Oh cool. Let's go with the spec.

@Snugug
Copy link
Member

Snugug commented Feb 12, 2013

I've tested MDN's incorrect example and proven it false, and have subsequently updated the MDN to reflect the proper use of not (as well as a large general update to the syntax section)

@Snugug
Copy link
Member

Snugug commented Feb 12, 2013

Additionally, after testing, only and not may only be used if paired with a media type. Because of this, we must make the decision whether or not to allow only and not to be used stand alone, and if so, should we set the media type used there to $breakpoint-default-media or to all?

@robwierzbowski
Copy link
Contributor Author

Nice research. After IRC chat, agree that not/only operators can only be set with media type. Simple and sweet.

@iamcarrico
Copy link
Contributor

So, I do not think adding in 'and' is a very good idea--- for three big reasons.

  1. it will break all current functionality. No site that currently uses breakpoint will be able to "upgrade" with any simple ability. This isn't a "some sites" or "sites using some advanced pieces" will break... but all of them.
  2. it adds in extra code. It is getting to the point of I am not sure what would be easier--- to just save variables myself, and have a @media #{$query-string} {} written out myself. A good part of the reason to use breakpoint is making this code easier to use, and we are just making it more complex.
  3. It will break respond-to entirely, by having the commas within the queries. This will be very difficult, if not impossible to get around.

I think there can be far better ways of doing the "or" functionality without breaking current sites. Since the specification points out that "or" is not more complex then a list of media queries, we should do the exact same. This will keep functionality for the old breakpoint code, and keep the "simplicity" of breakpoint to stay. Examples below:

$breakpoint_one = height 300px 700px;
$breakpoint_two = print;

@include breakpoint($breakpoint1, $breakpoint2) {
}

Again, this is just first thoughts, and I will refine it more--- but lets start the conversation up again.

@Snugug
Copy link
Member

Snugug commented Feb 16, 2013

We need to be able to offer both and and or options entirely within a single variable definition and not rely upon mixin input to do it. We need to for the same reason we need to have no-query and media in variable definitions; to make a single media query totally portable.
That being said, I can understand, and tend to agree the more I think of it, that 'and' may not be the best option for and, but that we do need something different and do need to make the comma an or. There are potential other options though; we can wrap each feature pair in (), we may be able to replace 'and' with & (not tested), or maybe some other symbol. No matter what we do, though, we're going to have some non-compatible API changes.

@robwierzbowski
Copy link
Contributor Author

Had a couple hours tonight to get my version's core functionality working. I took a different direction on a lot of it, so there's plenty to discuss. Some simpler, some maybe more complex.

Works with nested queries while keeping track that you don't 'or' inside of them, context in or queries returns a list, and processes feature lists in the old (or any) order. Try it with something ridiculous like:

$bp-a: 50em 60em;
$bp-b: height 20em 10em;
$bp-c: monochrome 'and' color-index 8 'and' $bp-a;

$bp: 'not' all 'and' 22em max-height, $bp-b 'and' $bp-c, width 50em 60em 'and' 
      no-query ".ie9", color 'and' monochrome 'and' screen 'and' 1.0 resolution 2.4;

There are some issues with (variable) argument list array encapsulation, so for now you still have to parenthesize bp lists added directly to the mixin.

https://github.com/robwierzbowski/breakpoint/tree/2.x.x-robw

@robwierzbowski
Copy link
Contributor Author

Context had some omissions, fixed and working now. No-queries (even nested and multiple) too.

@codingdesigner
Copy link
Member

I did some work on the 2x branch this weekend. I found and fixed some sass syntax and refined in a few places. I also added triple parsing.

I also did some thinking about the 'and' syntax. I think the quotes are a clumsy, and I'm looking for an alternative. Unfortunately I haven't found anything suitable yet. The CSS syntax obviously uses the word and, and all of the other keywords I can think of for joining tests fall outside of both CSS and Sass conventions. At this point I think we should stick with the 'and' syntax.

I also want to add some backwards compatibility for 1.x users. I think we can add a $breakpoint-1-support configuration variable that would parse a comma separated breakpoint argument. I can work on this soon.

@robwierzbowski
Copy link
Contributor Author

Even with the quotes I think 'and' is the best option. It's as close to the css syntax as we're going to get, and most of the time people are going to be using variables anyways.

@Snugug
Copy link
Member

Snugug commented Feb 19, 2013

I've been thinking about it, and I think and may be a bit too verbose. I think wrapping features in parenthesis would work just as well

On Feb 19, 2013, at 9:25 AM, Rob Wierzbowski notifications@github.com wrote:

Even with the quotes I think 'and' is the best option. It's as close to the css syntax as we're going to get, and most of the time people are going to be using variables anyways.


Reply to this email directly or view it on GitHub.

@codingdesigner
Copy link
Member

would we still need to wrap a single feature in parens? That should be a deal-breaker.

@codingdesigner
Copy link
Member

otherwise I think I would agree with that.

@Snugug
Copy link
Member

Snugug commented Feb 19, 2013

No we would not need to wrap single features in parens. What's nice about the parens option is that it's actually the Sass way to deal with nested lists, plus it makes double features look kind of like their Sass counterparts

@Snugug
Copy link
Member

Snugug commented Feb 19, 2013

CSS MQ counterparts *

@Snugug
Copy link
Member

Snugug commented Feb 19, 2013

@ChinggizKhan To your point that it will break respond-to; it won't. What you'd need to do is wrap your respond-to queries inside () and you'll be good to go. I do agree that 'and' is getting a little bit too verbose though. As for breaking backwards compatibility, yes it will, but that's the cost we have for creating a more flexible system. I'm not opposed to having breakpoint and breakpoint-legacy, but if you're using variables for your MQs it shouldn't be too hard to transfer to the new syntax and if 90% of your MQs are default MQs or double MQs, you will not be affected by the new syntax.

@robwierzbowski
Copy link
Contributor Author

Enabled $breakpoint-legacy-support in my branch. Slightly hackey with the 'and' syntax, but will get cleaner if we switch to ().

@codingdesigner
Copy link
Member

@robwierzbowski I went to look at yours but it doesn't look like you've updated your tests to show your work. Can you do that soon to make it easier to see what you've done? thanks

@Snugug
Copy link
Member

Snugug commented Feb 22, 2013

I'd prefer not to maintain legacy support inside of Breakpoint 2.0 and would much rather have breakpoint and breakpoint-legacy gems available. Maintaining two separate mental models and syntaxes inside of one system is a maintenance nightmare.

On Friday, February 22, 2013 at 11:02 AM, Mason Wendell wrote:

@robwierzbowski (https://github.com/robwierzbowski) I went to look at yours but it doesn't look like you've updated your tests to show your work. Can you do that soon to make it easier to see what you've done? thanks


Reply to this email directly or view it on GitHub (https://github.com/canarymason/breakpoint/issues/35#issuecomment-13950526).

@robwierzbowski
Copy link
Contributor Author

Sure, can do. I'm new to this whole Ruby test everything workflow. Have we
firmly decided on the parenthesis syntax? If we have, I'll update the
parser as well, and ping again.

On Fri, Feb 22, 2013 at 11:02 AM, Mason Wendell notifications@github.comwrote:

@robwierzbowski https://github.com/robwierzbowski I went to look at
yours but it doesn't look like you've updated your tests to show your work.
Can you do that soon to make it easier to see what you've done? thanks


Reply to this email directly or view it on GitHubhttps://github.com/canarymason/breakpoint/issues/35#issuecomment-13950526.

@codingdesigner
Copy link
Member

I just reviewed and really like the () syntax. Check it out.

@codingdesigner
Copy link
Member

@Snugug You may be right, but I want to explore it. Either way we need to set a timeframe for 1.x end of support.

@robwierzbowski
Copy link
Contributor Author

The legacy syntax wasn't hard to get working. My branch works like this:

Takes breakpoint argument input ->
flattens the array (nested variables cause array encapsulation) ->
processes the flattened array into a context array, and makes available to context ->
prints the context array into a css media query.

To enable legacy support I added a single function which parses the old style breakpoint syntax into the new style flattened list. If the legacy flag is set to true BP sends the arguments to bp-flatten-legacy instead of bp-flatten, and then continues as normal. If legacy support is something that we decide we want, the maintenance investment could be minimal.

@robwierzbowski
Copy link
Contributor Author

The parenthesis handling is really hard if we want to allow any type of nested variables.

$color: color; // Can be stand alone or take values
$medium: 20em;
// Without testing against features and allowed values this:
$breakpoint: ($color) ($medium); // two value space separated list, a string and number
// looks identical to this:
$breakpoint-fv: color 8; // two value space separated list, a string and number

With and keywords and allowing only commas/ors on the top level figuring out what's a breakpoint list was comparatively easy.

@Snugug
Copy link
Member

Snugug commented Feb 22, 2013

You should take a look at my branch, I've got this covered.

On Friday, February 22, 2013 at 4:47 PM, Rob Wierzbowski wrote:

The parenthesis handling is really hard if we want to allow any type of nested variables.
$color: color; // Can be stand alone or take values $medium: 20em; // Without testing against features and allowed values this: $breakpoint: ($color) ($medium); // two value space separated list, a string and number // looks identical to this: $breakpoint-fv: color 8; // two value space separated list, a string and number

With and keywords and allowing only commas/ors on the top level figuring out what's a breakpoint list was comparatively easy.


Reply to this email directly or view it on GitHub (https://github.com/canarymason/breakpoint/issues/35#issuecomment-13972963).

@robwierzbowski
Copy link
Contributor Author

Can you point me to a line/block?

@robwierzbowski
Copy link
Contributor Author

$medium-color: (20em) (color);
$medium-color-hires: $medium-color (resolution 2.0dppx) ;

.this {
 @include breakpoint($medium-color-hires) {
    background-color: red;
  }
}

compiles to:

@media (color: 20em) and (resolution: 2dppx), (color: 20em) and (-webkit-device-pixel-ratio: 2), (color: 20em) and (-moz-device-pixel-ratio: 2), (color: 20em) and (resolution: 192dpi) {
  .this {
    background-color: red; } }

and

$medium-color: (all) (20em) (color);
$medium-color-hires: $medium-color (resolution 2.0dppx) ;

causes an error because bp thinks $medium-color is a three value bp list.

@Snugug
Copy link
Member

Snugug commented Feb 22, 2013

We probably need to include the single and double parsing inside of the triple, break it out into functions maybe. Test each piece of a triple individually.

On Friday, February 22, 2013 at 5:02 PM, Sam Richard wrote:

So we refine it. I've spent the least amount of time on parsing the triples. Singles and doubles are fine.

On Friday, February 22, 2013 at 5:01 PM, Rob Wierzbowski wrote:

$medium-color: (20em) (color); $medium-color-hires: $medium-color (resolution 2.0dppx) ; .this { @include breakpoint($medium-color-hires) { background-color: red; } }

compiles to:
@media (color: 20em) and (resolution: 2dppx), (color: 20em) and (-webkit-device-pixel-ratio: 2), (color: 20em) and (-moz-device-pixel-ratio: 2), (color: 20em) and (resolution: 192dpi) { .this { background-color: red; } }

and
$medium-color: (all) (20em) (color); $medium-color-hires: $medium-color (resolution 2.0dppx) ;

causes an error because bp thinks $medium-color is a three value bp list.


Reply to this email directly or view it on GitHub (https://github.com/canarymason/breakpoint/issues/35#issuecomment-13973526).

@robwierzbowski
Copy link
Contributor Author

It's a tough problem for sure. Right now I don't see a way around it without testing if the feature is color and the number is unitless, since a string-feature and number-value bp list, and a single string-feature bp list, single number-value bp list are all valid.

@Snugug
Copy link
Member

Snugug commented Feb 22, 2013

So we refine it. I've spent the least amount of time on parsing the triples. Singles and doubles are fine.

On Friday, February 22, 2013 at 5:01 PM, Rob Wierzbowski wrote:

$medium-color: (20em) (color); $medium-color-hires: $medium-color (resolution 2.0dppx) ; .this { @include breakpoint($medium-color-hires) { background-color: red; } }

compiles to:
@media (color: 20em) and (resolution: 2dppx), (color: 20em) and (-webkit-device-pixel-ratio: 2), (color: 20em) and (-moz-device-pixel-ratio: 2), (color: 20em) and (resolution: 192dpi) { .this { background-color: red; } }

and
$medium-color: (all) (20em) (color); $medium-color-hires: $medium-color (resolution 2.0dppx) ;

causes an error because bp thinks $medium-color is a three value bp list.


Reply to this email directly or view it on GitHub (https://github.com/canarymason/breakpoint/issues/35#issuecomment-13973526).

@Snugug
Copy link
Member

Snugug commented Feb 22, 2013

It's totally do-able, and this syntax is much more sound than 'and' so we're going to need to do it.
If you're going to work on it, please do so on a fork of 2.x.x so we can merge it back in.

On Friday, February 22, 2013 at 5:07 PM, Rob Wierzbowski wrote:

It's a tough problem for sure. Right now I don't see a way around it without testing if the feature is color and the number is unitless, since a string-feature and number-value is correct, and a single string-feature, single number-value are both valid breakpoint lists. I'll take a crack at it over the weekend.


Reply to this email directly or view it on GitHub (https://github.com/canarymason/breakpoint/issues/35#issuecomment-13973791).

@Snugug
Copy link
Member

Snugug commented Mar 5, 2013

@robwierzbowski: @canarymason and I just brainstormed the nesting issue, and we've decided it's very edge case, but more importantly, we don't need to write any code to get it to work. Let's take your example:

What you wrote:

$medium-color: (20em) (color);
$medium-color-hires: $medium-color (resolution 2.0dppx) ;

Gets interpreted by Sass as ((20em) (color)) (resolution 2.0dppx) which is a 2 items list. While there should be error checking in our double to make sure the number value of color is unitless, that otherwise is a totally valid media query. The correct way to append two queries together is with Sass's append function, as follows:

$medium-color: (20em) (color);
$medium-color-hires: append($medium-color, (resolution 2.0dppx)) ;

This now is interpreted by Sass as 20em color (resolution: 2.0dppx) and works as expected. This needs to be documented, but this is the correct way to do it. As such, we do not believe we need advanced nesting support in Breakpoint, provided we write the documentation to explain append

@robwierzbowski
Copy link
Contributor Author

That would work, but I don't think it's the most end user friendly pattern. I solved the nesting on my branch, but I'm not sure the code is applicable to this version.

If you have some time while you two are in NYC I think it would help to set up a hangout or irc chat. Everything is functioning in my parens branch with tests, commented code, etc -- if we can talk over both and see what should be adapted where I think it we'd be very close to getting a nice and shiny BP 2.0 released.

@Snugug
Copy link
Member

Snugug commented Mar 11, 2013

@ChinggizKhan An update on Legacy Support:
Our most recent update to 2.x.x adds the $breakpoint-legacy-syntax variable that can be set by the user to trigger legacy syntax support. Legacy syntax support has full feature parity with the new syntax with the exception of supporting or queries (this means support for media and no--query in variable form) as well as being able to use our new parser and, as currently architected, any updates to our new parser.

@Snugug
Copy link
Member

Snugug commented Apr 10, 2013

Launched

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

No branches or pull requests

4 participants