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

+between() seems to be broken #65

Open
sidonaldson opened this issue Feb 4, 2016 · 3 comments
Open

+between() seems to be broken #65

sidonaldson opened this issue Feb 4, 2016 · 3 comments

Comments

@sidonaldson
Copy link

I've been playing with rapture for a while now and I think I've found an error...

http://codepen.io/sidonaldson/pen/YwOeZB/

The above codepen example has two divs. You can see the breakpoints in action by resizing the panel. The CSS code is at the bottom of the CSS panel after I pasted in the rapture mixin.

The top div contains the between mixin and the bottom div shows how it should look it used with standard +above(). As you can see they are different.

I then set about playing with the tests.

This test is pretty basic

.hello
  color: red

  +between(0, 1)
    color lime

  +between(1, 2)
    color white

  +between(2, 3)
    color white

  +between(2, 4)
    color white

But produces the following code to pass the test

.hello {
  color: #f00;
}
@media only screen and (max-width: 400px) {
  .hello {
    color: #0f0;
  }
}
@media only screen and (max-width: 600px)  {
  .hello {
    color: #fff;
  }
}
@media only screen and (min-width: 400px) and (max-width: 800px) {
  .hello {
    color: #fff;
  }
}
@media only screen and (min-width: 400px) and (max-width: 1050px) {
  .hello {
    color: #fff;
  }
}

I would expect the following

.hello {
  color: #f00;
}
@media only screen and (max-width: 400px) {
  .hello {
    color: #0f0;
  }
}
@media only screen and (min-width: 400px) and (max-width: 600px)  {
  .hello {
    color: #fff;
  }
}
@media only screen and (min-width: 600px) and (max-width: 800px) {
  .hello {
    color: #fff;
  }
}
@media only screen and (min-width: 600px) and (max-width: 1050px) {
  .hello {
    color: #fff;
  }
}

I think the error is the way Rapture handles a index of '0'. I'll have look at doing a PR when I get sometime but I'm knee deep in production code at the mo so can only report it.

Hope this helps & thanks for the code :)

@jescalan
Copy link
Owner

jescalan commented Feb 8, 2016

Ping @declandewet on this one!

@zspecza
Copy link
Collaborator

zspecza commented Feb 9, 2016

@Jenius I'll have a look into this later, when I get a chance - I have to prioritise some other work for the moment. But this is strange - If it is a genuine bug, then very nice spot @sidonaldson. I just find it super weird because, well, Rupture is entirely built on the foundation of the between mixin and Rupture has been battle tested in production countless times 😕

@denizdogan
Copy link

The unexpected output looks like some sort of optimization, doesn't it? I would expect the CSS to be functionally equivalent, but maybe I'm missing something.

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