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

When using lost-move, have it retain the gutter if specified by a previous lost-column #195

Closed
sidonaldson opened this issue Dec 10, 2015 · 12 comments

Comments

@sidonaldson
Copy link

sidonaldson commented Dec 10, 2015

Hi Cory, I think I've found a little bug with the lost-move where it seems to default to the global gutter value.
In the test case I have two columns (1/3 and 2/3) and I want to swap their positions and without any gutter.

div1{
  lost-column: 2/3 0 0 no-flex;
  lost-move: 1/3;
}
div2{
  lost-column: 1/3 0 0 no-flex;
  lost-move: -2/3;
}

The problem is that lost-move ignores the gutter override and you still get a gap and the offsets are out.
I can get around it by setting @lost gutter 0; but that's not ideal.

Demo: http://codepen.io/sidonaldson/pen/rxVvpm/

What do you think?

@peterramsing
Copy link
Owner

Looking at line 30 of the lib/lost-move.js file (lost-move.js), it looks like it it pulls the gutter from the default settings, not the previous lost-column: declaration.

That's not really an answer, I suppose.

@peterramsing
Copy link
Owner

Maybe the gutter could be passed on to lost-move if set by lost-column...

@corysimmons
Copy link
Contributor

We can probably do something where we look through that current block of definitions for lost-column and detect if it has a gutter set, then set it for the lost-move declaration.

Thanks for pointing this out.

@peterramsing peterramsing added this to the 6.6.x - Bug Fixing milestone Dec 30, 2015
@peterramsing peterramsing changed the title Move / Gutter issue When using lost-move, have it retain the gutter if specified by a previous lost-column Dec 30, 2015
@peterramsing peterramsing self-assigned this Jan 2, 2016
@peterramsing
Copy link
Owner

@sidonaldson Here's a codepen with this "fixed". http://codepen.io/peterramsing/pen/dGNXmK

You can specify the gutter within the lost-move rule. (see the codepen)
You can also use lost-move-gutter: 0;.

I'm working on getting the lost-move to read the gutter from the lost-column rule right now, though. Sorry for the delay on this. When I took over this repo I really wanted to get a handle on everything needed before diving into the code. This is slated to be done by next week, though. It's in the 6.6.4 milestone.

@peterramsing
Copy link
Owner

@corysimmons Was there any reason that lost-move.js doesn't use "newBlock()"?

@peterramsing
Copy link
Owner

Closing to migrate discussion to the PR #219

@peterramsing
Copy link
Owner

@corysimmons I think I answered my own question about new-block.js.

@corysimmons
Copy link
Contributor

👍 tbh, I'm not sure. I might have just overlooked it. If it works with newBlock, newBlock should probably be added so the property is uniform with the rest of them.

@peterramsing
Copy link
Owner

I'm noticing a few things that are inconsistent, but not breaking. Any refactor will have to wait. I'd love to get some of these bugs/issues deployed before doing any refactor work. Maybe when I have another long weekend I'll spend sometime doing that and maybe getting some linting in place.

@corysimmons
Copy link
Contributor

Sounds good. 👍

@peterramsing peterramsing added this to the v8.1 milestone Nov 14, 2016
@peterramsing
Copy link
Owner

With the work that I did with the $lost-gutter variable I think this should be pretty simple now (or at least more simple?). As I'm getting close to finishing all the needs for version 8 early I'm adding this to the version 8 milestone for the December release.

@peterramsing
Copy link
Owner

Closing for #350

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

No branches or pull requests

3 participants