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

WIP: Fixes issue where lost-move would not retain local gutter #219

Closed
wants to merge 2 commits into from

Conversation

peterramsing
Copy link
Owner

From #195

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

The lost-move rule overrides the 0 gutter from lost-column. This is addressed with this PR

* fixes a typo
* reflects the lost-move adding its rules to the lost-column rules
@peterramsing peterramsing mentioned this pull request Jan 2, 2016
@peterramsing
Copy link
Owner Author

This is looking a lot more complex as I'm diving in further. It's doable but having lost-move reference something from lost-column is a bit tricky as the property and value are removed as they are processed. This is so that lost-column: 2/3 doesn't end up in the compiled CSS. I'm looking into storing the value of the gutter in the selector, but I'm going to have to do some more PostCSS API reading and experimenting.

There are three options I can see right now:

  1. Have lost-move process after lost-column and remove the lost-column. I don't like this for several reasons.
  2. Store the selector's gutter as I said above.
  3. Don't remove any of the lost-* properties until the end when a cleanup task removes them. similar concerns to option 1.

I do have a working concept of option 3 working right now nicely. It might be a suitable patch.

@peterramsing
Copy link
Owner Author

Another thought: I am realizing that this change could have some breaking affects for users. I'm concerned with shipping this in 6.6.4. @sidonaldson You should be able to make this work with the example codepen but I understand having to declare the gutter again isn't idea.

@corysimmons I don't think this is not suitable as a patch. I think there could be a backwards compatibility solution that could be lost-move: 1/3 gutter-retain, or something like that. That would mean nothing would change for any code that has used this before. But if you're doing that, why not just hand-code what the gutter should be?

With that thought, I feel this might be an addition for 7.0.0. Thoughts on this "breaking" update?

@corysimmons
Copy link
Contributor

I wouldn't worry about making backwards compatible API changes. gutter-retain? I mean... come on.

I'd just do this in 7.

So far as making this work, you could move https://github.com/peterramsing/lost/blob/master/lost.js#L16 above https://github.com/peterramsing/lost/blob/master/lost.js#L12

Then when lost-move is parsed, lost-column calls would still be in the same declaration block.

@peterramsing
Copy link
Owner Author

Wait-is it really as simple as moving that order?

I promised I'd finish editing a photoshoot for the Mrs. but will look further into this tomorrow. That being said...I think this is something that goes in 7.0.0–especially if you agree. Thanks.

@peterramsing peterramsing removed this from the 6.6.4 - Bug Fixing milestone Jan 3, 2016
@peterramsing
Copy link
Owner Author

Closing but will re-open on the 7.0.0 branch.

@corysimmons
Copy link
Contributor

Are there other properties as well? Like would lost-offset benefit from this kind of treatment as well? What else?

Maybe it'd be best if all of these were wrapped up into 7 with some kind of global rule for them turned on by default. @lost inherit-gutters true (default in 7)?

@peterramsing
Copy link
Owner Author

There are probably more that would benefit from this. Sounds like a good start to the scope of Lost 7

@peterramsing
Copy link
Owner Author

After thinking through this on my run this morning a bit more, I think that option 3 (3. Don't remove any of the lost-* properties until the end when a cleanup task removes them.) might be a good long-term option. I want to do some testing to see if there is a performance hit in regards to how long it takes to compile, but I think that it would allow for greater flexibility in the long run. I'll test re-ordering the files but that doesn't seem like the long-term solution that this needs. What if lost-column needs to retain the gutters from lost-move, for example. I'll let this simmer some more but certainly, certainly worth testing out. It would be amazing if all rules were exposed to what the other rules had set.

@peterramsing peterramsing deleted the lost-move-retain-lost-column-gutter branch July 6, 2016 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants