Incompatibility with postcss-responsive-type

[petekp]

The Responsive Type postcss plugin allows you to provide a min and max font-size which dynamically changes on window resize (using viewport unit & calc() tricks—this post by @madebymike popularized the technique).

Unfortunately, it doesn't work in tandem with the modular scale plugin.

This works:

font-size: responsive 1rem 7rem;

But when using modular scale units like below, it compiles, but the output isn't expected:

font-size: responsive ms(1)rem ms(7)rem;

I thought re-ordering the plugins so that the modular scale plugin ran first (replacing the min and max with valid rem units) would do the trick, but sadly it appears to be a deeper issue. Taking a look to see what I can personally contribute to resolve this—gotta read up on the ParseCSS API first.

As a temporary workaround, you can store your min max values as separate variables (via postcss-variables or precss), then pass those as parameters to responsive type:

$ms1: ms(1)rem;
$ms2: ms(2)rem;

font-size: responsive $ms1 $ms2;

[MadeByMike]

Awesome! Thanks for describing this so thoughly. I'm happy to help you if you'd like to work on a fix.

[petekp]

@MadeByMike Took a crack at it tonight. Any feedback welcome; first time OS contributor and JS novice :)

https://github.com/petekp/postcss-modular-scale/tree/12

[MadeByMike]

@petekp I'm so sorry for taking this long to get back to you! I'm doing a lot of traveling at the moment so I hope you will forgive me. Thanks for the contribution. I think it needs a little bit more work, but it helps me understand the problem better.

I notice that the variable newValue is still being added to the messages array, but with this change we are not defining newValue. I also think that we might be reproducing some functionality of the method css.walkDecls, perhaps unnecessarily? In other words, it's not as DRY as it could be.

I hope that is helpful feedback. I had nothing to do with writing postcss-modular-scale or postcss-responsive-type so it will take me a little longer to get familiar with them and offer some alternatives. We also need to be careful that other uses of postcss-modular-scale are not impacted by any change and if they are, the fix might be better placed in postcss-responsive-type. I'd be interested to know what @seaneking thinks.

Thanks again @petekp, every OS contribution is valuable whether it is the final result or not, congratulation on you first, it's much better than mine was!

[MadeByMike]

OK, so I did some more testing and I think there is an issue with postcss-modular-scale but surprisingly not as described.

I didn't know much about how the order of execution for PostCSS plugins works so I reached out on twitter and Andrey Sitnik (creator of postCSS) got back to me. What an awesome guy:

https://twitter.com/andreysitnik/status/715219287128944640

There is more detail about plugin order here: https://github.com/postcss/postcss/issues/296 and this issue may go away in the future versions of PostCSS.

But for now I made sure the plugin order was correct and and tested it again. It also failed but with a different result.

The real issue is that postcss-modular scale cannot return more than one property value. This means you can't use it on something like a box-shadow:

.ms-shadow {
  box-shadow: ms(1)px ma(2)px 5px 0px rgba(0,0,0,0.75);
}

My feeling is that when the above works it will also work with postcss-responsive-type.

@kristoferjoseph shall we close this and create a new issue?

[petekp]

@MadeByMike Really appreciate the feedback and encouragement 😃 Yes, you're totally right that this is part of the wider issue of it only returning 1 property max. I thought I'd addressed this, but then re-ran my fork and encountered the newValue error you mentioned. Having now fixed that, it seems to be handling multi-props, albeit inefficiently:

:+1: for opening a new, more specific issue.

[kristoferjoseph]

Ah, yes! This is by design since I was discouraging my team from using short-hand because it makes the CSS harder to update and override. I had designed this to work with font-size, margin-*, padding-*, line-height, etc. I didn't think it would be useful for things like box-shadow, but I stand corrected :)

It is impossible to anticipate the syntax of future PostCSS plugins, I never would have imagined font-size taking multiple values, as it does when using the responsive type plugin. Who knew ;)

I could be open to updating this to support parsing shorthand values.

[MadeByMike]

I agree in that I don't think it would make much sense to use a modular scale for box-show it was just one example that came to mind for multiple properties. I do think it's a good idea to support multiple and mixed return values.

I also discourage shorthand in my team. Generally speaking it leads to people setting values of 0 when what they really want to do is update a single padding\margin property. There is a big difference between setting a value of 0 and not specifying a value. But my personal preference aside, shorthand margin and padding values are very common and a significant use case.

I don't think anyone would expect you to anticipate the syntax of other PostCSS plugins but if returning multiple values results in better compatibility with other plugins that's a bonus. I also think fluid modular scales are a beautiful technique and it would be nice for this it to be so easy to work with in PostCSS.

Finally calc() expressions are becoming more prevalent and important in CSS and they can have multiple values mixed with operators and other characters. It would be nice if the postcss-modular-scale plugin supported this.

I think these are the main use cases. Again I'd be happy to help out and I think @petekp is willing also. Please let us know if and how you'd like to approach this.

[kristoferjoseph]

Cool. I'm going to take a pass at removing multiple bases and ratios and will add support for this use case while I am in there. Thanks