peterramsing / lost

LostGrid is a powerful grid system built in PostCSS that works with any preprocessor and even vanilla CSS.
http://lostgrid.org
MIT License
4.5k stars 161 forks source link

Add parameter no-height to lost-waffle #411

Closed elgandoz closed 6 years ago

elgandoz commented 6 years ago

What kind of change is this? (Bug Fix, Feature...) Feature

What is the current behavior (You can also link to an issue) See #410 and #183

What is the new behavior this introduces (if any) New parameter to skip completely the height calculation in lost-waffle. Ex:

div {
  lost-waffle: 1/3 no-height;
}

which will print:

div {
  width: calc(99.99999% * 1/3 - (20px - 20px * 1/3));
}

Does this introduce any breaking changes? It's extremely simple, it shouldn't break anything.

Does the PR fulfill these requirements?

Other Comments It's my first PR, I hope I'm doing it right...

codecov[bot] commented 6 years ago

Codecov Report

Merging #411 into master will decrease coverage by 0.14%. The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #411      +/-   ##
==========================================
- Coverage     100%   99.85%   -0.15%     
==========================================
  Files          19       19              
  Lines         701      704       +3     
==========================================
+ Hits          701      703       +2     
- Misses          0        1       +1
Impacted Files Coverage Δ
lib/lost-waffle.js 98.87% <66.66%> (-1.13%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7681e79...2107185. Read the comment docs.

peterramsing commented 6 years ago

@elgandoz, this is pretty sweet.

Here's what I'm thinking right now:

  1. What is your browser compatibility requirements? Waffles are something I've been not touching too much as you can do Waffles with CSS Grid extremely easily. (See this CodePen)
  2. Of course, CSS Grid isn't in all browsers...we need fallbacks. That's one reason why LostGrid is so powerful. I totally agree that this is a solution that could work well.
  3. This needs a few things, like docs, and tests. What you would want to do is, at the very least, have coverage for what is added ensuring regressions don't occur.
  4. Docs should have this API updated to include this no-height param.

Let me glance through the code a bit more flush out my thoughts on this.

peterramsing commented 6 years ago

For now, it looks like we're adapting this away from this strategy. Closing for now.